Show warning on nested flexboxes and other unsupported elements in a …

…top-level formatting context, see #320
mikke89 · May 30, 2022 · 7686779 · 7686779
1 parent 57352b9
commit 7686779
Show file tree

Hide file tree

Showing 3 changed files with 122 additions and 0 deletions.
diff --git a/Source/Core/LayoutEngine.cpp b/Source/Core/LayoutEngine.cpp
@@ -57,6 +57,32 @@ static Pool< LayoutChunk<ChunkSizeBig> > layout_chunk_pool_big(50, true);
 static Pool< LayoutChunk<ChunkSizeMedium> > layout_chunk_pool_medium(50, true);
 static Pool< LayoutChunk<ChunkSizeSmall> > layout_chunk_pool_small(50, true);
 
+static inline bool ValidateTopLevelElement(Element* element)
+{
+	const Style::Display display = element->GetDisplay();
+
+	// Currently we don't support flexboxes or tables in a top-level formatting context. This includes on the <body> element, table cells, the
+	// children of flex containers, and possibly elements with custom formatting such as <select>. See also the related
+	// 'uses_unsupported_display_position_float_combination' below.
+	if (display == Style::Display::Flex || display == Style::Display::Table)
+	{
+		const char* error_msg = "located in a top-level formatting context";
+		if (Element* parent = element->GetParentNode())
+		{
+			if (parent->GetDisplay() == Style::Display::Flex)
+				error_msg = "nested inside a flex container";
+		}
+		const Property* display_property = element->GetProperty(PropertyId::Display);
+		Log::Message(Log::LT_WARNING,
+			"Element with display type '%s' cannot be %s. Instead, wrap it within a parent block element such as a <div>. Element will not be "
+		    "formatted: %s",
+			display_property ? display_property->ToString().c_str() : "*unknown*", error_msg, element->GetAddress().c_str());
+
+		return false;
+	}
+
+	return true;
+}
 
 // Formats the contents for a root-level element (usually a document or floating element).
 void LayoutEngine::FormatElement(Element* element, Vector2f containing_block, const Box* override_initial_box, Vector2f* out_visible_overflow_size)
@@ -68,6 +94,9 @@ void LayoutEngine::FormatElement(Element* element, Vector2f containing_block, co
 	RMLUI_ZoneName(name.c_str(), name.size());
 #endif
 
+	if (!ValidateTopLevelElement(element))
+		return;
+
 	auto containing_block_box = MakeUnique<LayoutBlockBox>(nullptr, nullptr, Box(containing_block), 0.0f, FLT_MAX);
 
 	Box box;

diff --git a/Tests/Data/VisualTests/flex_nested.rml b/Tests/Data/VisualTests/flex_nested.rml
@@ -0,0 +1,46 @@
+<rml>
+<head>
+	<title>Flex - Nested flex boxes</title>
+	<link type="text/rcss" href="../style.rcss"/>
+    <link rel="match" href="reference/flex_nested-ref.rml"/>
+	<link rel="GitHub issue #320" href="https://github.com/mikke89/RmlUi/issues/320" />
+	<meta name="Description" content="Flex items should support flex layout." />
+	<meta name="Assert" content="All items should be located on a single line, and there should be no holes between them." />
+	<style>
+		#window  
+		{
+			display: flex;
+			width: 100%;
+			height: 100%;
+			overflow: hidden auto;
+			flex-direction: row;
+			justify-content: center;
+			flex-wrap: nowrap;
+		}
+		.content
+		{
+			display: flex;
+			height: 128px;
+			width: 40%;
+			text-align: center;
+			flex-direction: row;
+		}
+	</style>
+</head>
+
+<body>
+<div id="window">
+	<div class="content">
+		<div style="width: 20%; background: red;">1</div>
+		<div style="width: 60%; background: green;">2</div>
+		<div style="width: 20%; background: blue;">3</div>
+	</div>
+	<div class="content">
+		<div style="width: 20%; background: red;">1</div>
+		<div style="width: 60%; background: green;">2</div>
+		<div style="width: 20%; background: blue;">3</div>
+	</div>
+</div>
+<handle size_target="#document"/>
+</body>
+</rml>
diff --git a/Tests/Data/VisualTests/reference/flex_nested-ref.rml b/Tests/Data/VisualTests/reference/flex_nested-ref.rml
@@ -0,0 +1,47 @@
+<rml>
+<head>
+    <title>Flex 05 ref</title>
+    <link type="text/rcss" href="../../style.rcss"/>
+	<meta name="Reference" content="Reference implemented by wrapping the nested flex box in an extra div." />
+	<style>
+		#window {
+			display: flex;
+			width: 100%;
+			height: 100%;
+			overflow: hidden auto;
+			flex-direction: row;
+			justify-content: center;
+			flex-wrap: nowrap;
+		}
+		.column {
+			width: 40%;
+		}
+		.content {
+			display: flex;
+			height: 128px;
+			text-align: center;
+			flex-direction: row;
+		}
+	</style>
+</head>
+
+<body>
+<div id="window">
+	<div class="column">
+		<div class="content">
+			<div style="width: 20%; background: red;">1</div>
+			<div style="width: 60%; background: green;">2</div>
+			<div style="width: 20%; background: blue;">3</div>
+		</div>
+	</div>
+	<div class="column">
+		<div class="content">
+			<div style="width: 20%; background: red;">1</div>
+			<div style="width: 60%; background: green;">2</div>
+			<div style="width: 20%; background: blue;">3</div>
+		</div>
+	</div>
+</div>
+<handle size_target="#document"/>
+</body>
+</rml>