Fix overflow and clipping issues with 'overflow: auto' and 'overflow:…

… hidden' on nested elements. See #116. Some additional tests added.
mikke89 · Aug 5, 2020 · c864264 · viciious · Aug 6, 2020 · mikke89
1 parent 23f7a42
commit c864264
Show file tree

Hide file tree

Showing 8 changed files with 160 additions and 7 deletions.
diff --git a/Source/Core/LayoutBlockBox.cpp b/Source/Core/LayoutBlockBox.cpp
@@ -39,7 +39,7 @@
 namespace Rml {
 
 // Creates a new block box for rendering a block element.
-LayoutBlockBox::LayoutBlockBox(LayoutEngine* _layout_engine, LayoutBlockBox* _parent, Element* _element) : position(0, 0)
+LayoutBlockBox::LayoutBlockBox(LayoutEngine* _layout_engine, LayoutBlockBox* _parent, Element* _element) : position(0), visible_outer_width(0)
 {
 	RMLUI_ZoneScoped;
 
@@ -211,10 +211,13 @@ LayoutBlockBox::CloseResult LayoutBlockBox::Close()
 		{
 			// Calculate the dimensions of the box's *internal* content; this is the tightest-fitting box around all of the
 			// internal elements, plus this element's padding.
-			Vector2f content_box(0, 0);
+			Vector2f content_box = Vector2f(0, 0);
 
 			for (size_t i = 0; i < block_boxes.size(); i++)
-				content_box.x = Math::Max(content_box.x, block_boxes[i]->GetBox().GetSize(Box::MARGIN).x);
+			{
+				// TODO: Only if the containing block is not an ancestor of us (ie. we are the containing block?).
+				content_box.x = Math::Max(content_box.x, block_boxes[i]->visible_outer_width);
+			}
 
 			// Check how big our floated area is.
 			Vector2f space_box = space->GetDimensions();
@@ -246,6 +249,16 @@ LayoutBlockBox::CloseResult LayoutBlockBox::Close()
 			element->SetBox(box);
 			element->SetContentBox(space->GetOffset(), content_box);
 
+			const float margin_width = GetBox().GetSize(Box::MARGIN).x;
+
+			// Set the visible outer width so that ancestors can catch any overflow produced by us. That is, hiding it or providing a scrolling mechanism.
+			// If we catch our own overflow here, then just use the normal margin box as that will effectively remove the overflow from our ancestor's perspective.
+			if (overflow_x_property != Style::Overflow::Visible)
+				visible_outer_width = margin_width;
+			else
+				visible_outer_width = Math::Max(margin_width, space->GetOffset().x + content_box.x + box.GetEdge(Box::MARGIN, Box::LEFT) + box.GetEdge(Box::MARGIN, Box::RIGHT));
+
+
 			// Format any scrollbars which were enabled on this element.
 			element->GetElementScroll()->FormatScrollbars();
 		}

diff --git a/Source/Core/LayoutBlockBox.h b/Source/Core/LayoutBlockBox.h
@@ -208,6 +208,7 @@ class LayoutBlockBox
 	Box box;
 	float min_height;
 	float max_height;
+
 	// Used by inline contexts only; set to true if the block box's line boxes should stretch to fit their inline content instead of wrapping.
 	bool wrap_content;
 
@@ -224,6 +225,9 @@ class LayoutBlockBox
 	// Used by block contexts only; stores the value of the overflow property for the element.
 	Style::Overflow overflow_x_property;
 	Style::Overflow overflow_y_property;
+	//  Used by block contexts only; the content width as visible from the parent. Similar to scroll width, but shrinked if overflow is caught here. 
+	//   This can be wider than the box if we are overflowing. Only available after the box has been closed. 
+	float visible_outer_width;
 	// Used by block contexts only; if true, we've enabled our vertical scrollbar.
 	bool vertical_overflow;
 

diff --git a/Tests/Data/VisualTests/float_basic.rml b/Tests/Data/VisualTests/float_basic.rml
@@ -0,0 +1,52 @@
+<rml>
+<head>
+    <title>Floats, block formatting contexts</title>
+    <link type="text/rcss" href="../style.rcss"/>
+	<link rel="help" href="https://www.w3.org/TR/CSS21/visufx.html#propdef-overflow" />
+	<meta name="Description" content="Nesting divs should still hide overflow. Elements whose containing block is located above the 'overflow: hidden' element should be visible." />
+	<meta name="See also" content="CSS 2.1 'clipping-' and 'overflow-' tests." />
+	<style>
+		body {
+			background: #ddd;
+			color: #444;
+		}
+		#content {
+			width: 200px;
+			margin: 0 auto;
+		}
+		div.float {
+			float: left;
+			background-color: #bbb;
+			border: 1px #333;
+			width: 100px;
+			height: 100px;
+			margin: 5px;
+		}
+		.red {
+			color: red;
+		}
+	</style>
+</head>
+
+<body>
+<p>This test makes assumptions about the font properties, that is, it depends on the size of the layed-out text.</p>
+<div id="content">
+	<p>Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod.
+	<div class="float">float: left</div>
+	<span class="red">This</span> is the first word after the float and should flow next to the float.</p>
+	<p>This paragraph should flow next to the float.</p>
+
+	<hr/>
+	<p>Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod.
+	<div class="float">float: left</div>
+	<span class="red">This</span> is the first word after the float and should flow next to the float.</p>
+	<p style="clear: left;">This paragraph should be below the float as it clears the float.</p>
+
+	<hr/>
+	<p>Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod.
+	<div class="float">float: left</div>
+	<span class="red">This</span> is the first word after the float and should flow next to the float.</p>
+	<p style="overflow: auto;">This paragraph should establish a new block formatting context. This element's size and position should not overlap with the float, but still be located next to it.</p>
+</div>
+</body>
+</rml>
diff --git a/Tests/Data/VisualTests/float_overflow.rml b/Tests/Data/VisualTests/float_overflow.rml
@@ -0,0 +1,43 @@
+<rml>
+<head>
+    <title>Floats: overflow</title>
+    <link type="text/rcss" href="../style.rcss"/>
+	<link rel="help" href="http://www.w3.org/TR/CSS21/visuren.html#floats" />
+	<meta name="Description" content="Floating boxes" />
+	<style>
+		body {
+			background: #ddd;
+			color: #444;
+		}
+		#content {
+			width: 200px;
+			margin: 0 auto;
+		}
+		.box {
+			background-color: #cce;
+			border: 5px #77b;
+		}
+		.float {
+			float: left;
+			background-color: #ddda;
+			border: 1px #333;
+			width: 200px;
+			height: 130px;
+			margin: 5px;
+		}
+
+	</style>
+</head>
+
+<body>
+<div class="box">
+	<div class="float">float: left</div>
+	<p>The float to the left should extend past the background of the containing '.box' element.</p>
+</div>
+<hr/>
+<div class="box" style="overflow:auto">
+	<div class="float">float: left</div>
+	<p>Using 'overflow: auto' on the containing '.box' element. This should establish a new block formatting context (thereby resolving all floats) so that the background wraps around the entire float.</p>
+</div>
+</body>
+</rml>
diff --git a/Tests/Data/VisualTests/overflow_hidden.rml b/Tests/Data/VisualTests/overflow_hidden.rml
@@ -4,6 +4,7 @@
     <link type="text/rcss" href="../style.rcss"/>
     <link rel="match" href="reference/overflow_hidden-ref.rml"/>
 	<link rel="help" href="https://www.w3.org/TR/CSS21/visufx.html#propdef-overflow" />
+	<link rel="GitHub issue" href="https://github.com/mikke89/RmlUi/issues/116" />
 	<meta name="Description" content="Nesting divs should still hide overflow. Elements whose containing block is located above the 'overflow: hidden' element should be visible." />
 	<meta name="See also" content="CSS 2.1 'clipping-' and 'overflow-' tests." />
 	<style>
@@ -43,7 +44,6 @@
 <body>
 <p>There should be a green box, no red visible, and the word 'fail' should not appear.</p>
 <div class="overflow">
-<!-- Nesting an extra div appears to break overflow in RmlUi. Setting it to 'display: inline' appears to make it work correctly. -->
 	<div>
 		<div class="wide">FAIL</div>
 		LONG_WOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOORD&nbsp;FAIL

diff --git a/Tests/Data/VisualTests/overflow_nested.rml b/Tests/Data/VisualTests/overflow_nested.rml
@@ -0,0 +1,42 @@
+<rml>
+<head>
+    <title>Nested overflow</title>
+    <link type="text/rcss" href="../style.rcss"/>
+	<meta name="Description" content="The deepest element in the tree should catch the overflow." />
+	<style>
+		body {
+			display: block;
+			background: #ddd;
+			color: #444;
+		}
+		div.outer {
+			overflow: auto;
+			width: 200px;
+			height: 200px;
+		}
+		div.overflow {
+			border: 1px black;
+			overflow: auto;
+			width: 150px;
+			height: 150px;
+		}
+		div.wide {
+			width: 300px;
+			height: 20px;
+			text-align: center;
+			border: 1px #0a0;
+			margin: 5px;
+			background-color: #eee;
+		}
+	</style>
+</head>
+
+<body>
+<p>There should should only be one scroll bar visible, inside the black border.</p>
+<div class="outer">
+	<div class="overflow">
+		<div class="wide">Wide element</div>
+	</div>
+</div>
+</body>
+</rml>
diff --git a/Tests/Data/style.rcss b/Tests/Data/style.rcss
@@ -27,6 +27,7 @@ body {
 	padding: 8px;
 	width: 484px;
 	min-height: 484px;
+	max-height: 100%;
 	color: black;
 	background: #ccc;
 	overflow: auto;
@@ -52,12 +53,10 @@ a:active {
 scrollbarvertical
 {
 	width: 16dp;
-	scrollbar-margin: 16px;
 }
 scrollbarhorizontal
 {
 	height: 16dp;
-	scrollbar-margin: 16px;
 }
 scrollbarvertical slidertrack,
 scrollbarhorizontal slidertrack

diff --git a/Tests/Source/VisualTests/XmlNodeHandlers.cpp b/Tests/Source/VisualTests/XmlNodeHandlers.cpp
@@ -83,8 +83,8 @@ Element* XMLNodeHandlerLink::ElementStart(XMLParser* parser, const String& name,
 {
 	RMLUI_ASSERT(name == "link");
 
-	const String rel = StringUtilities::ToLower(Get<String>(attributes, "rel", ""));
 	const String type = StringUtilities::ToLower(Get<String>(attributes, "type", ""));
+	const String rel = Get<String>(attributes, "rel", "");
 	const String href = Get<String>(attributes, "href", "");
 
 	if (!type.empty() && !href.empty())