Clear floats added dynamically to previous siblings

Clear floats added dynamically to previous siblings https://bugs.webkit.org/show_bug.cgi?id=247237 Reviewed by Alan Baradlay. This patch is to align Webkit behavior with Blink / Chrome and Gecko / Firefox. Merge - https://chromium.googlesource.com/chromium/src.git/+/c75a8e71e60527400e95d0834f4a07a018087130 If margin collapsing moves a child up into a previous sibling, we shouldn't add any floats that it may have cleared and prompt another layout so that the child can also clear them if necessary. Confining the need for a layout to cases where the self-collapsing block has margin excludes the more obvious case where a sibling has a lot of negative margin and moves up into a sibling with nested floats inside. * Source/WebCore/rendering/RenderBlockFlow: (RenderBlockFlow::collapseMarginsWithChildInfo): Remove "clearanceForSelfCollapsingBlock" and conditions and update comment as needed * LayoutTests/fast/block/margin-collapse/clear-dynamically-added-float.html: Added Test Case * LayoutTests/fast/block/margin-collapse/clear-dynamically-added-float-expected.txt: Added Test Case Expectations Canonical link: https://commits.webkit.org/256238@main
WebKit · Nov 2, 2022 · 6c2570b4a4d2472445444323e37741fc45e90904 · 6c2570b
1 parent 6f821cc
commit 6c2570b4a4d2472445444323e37741fc45e90904
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 9 deletions.
diff --git a/LayoutTests/fast/block/margin-collapse/clear-dynamically-added-float-expected.txt b/LayoutTests/fast/block/margin-collapse/clear-dynamically-added-float-expected.txt
@@ -0,0 +1,2 @@
+PASS
+Clear floats added dynamically to previous siblings. There should be a blue bar below the green bar and both should be 50px high.
diff --git a/LayoutTests/fast/block/margin-collapse/clear-dynamically-added-float.html b/LayoutTests/fast/block/margin-collapse/clear-dynamically-added-float.html
@@ -0,0 +1,33 @@
+<!DOCTYPE html>
+<style>
+body {
+    margin: 0px;
+}
+.inner{
+    float: left;
+    display: none;
+    width: 100px;
+    height: 50px;
+    background-color: green;
+}
+.clear {
+    clear: both;
+}
+</style>
+<div style="width: 100px;">
+    <div>
+        <div class="inner"></div>
+        <div class="clear"></div>
+    </div>
+    <div style="margin-top: -20px;">
+        <div class="clear" id="clear" style="width: 100px; height:50px; background-color: blue" data-offset-y=50></div>
+    </div>
+</div>
+<div id="test-output"></div>
+<script src="../../../resources/check-layout.js"></script>
+<script>
+    document.body.offsetTop;
+    document.querySelector('.inner').style.display = 'block';
+    window.checkLayout("#clear", document.getElementById("test-output"));
+</script>
+<p>Clear floats added dynamically to previous siblings. There should be a blue bar below the green bar and both should be 50px high.</p>
diff --git a/Source/WebCore/rendering/RenderBlockFlow.cpp b/Source/WebCore/rendering/RenderBlockFlow.cpp
@@ -1160,16 +1160,11 @@ LayoutUnit RenderBlockFlow::collapseMarginsWithChildInfo(RenderBox* child, Rende
 
     LayoutUnit beforeCollapseLogicalTop = logicalHeight();
     LayoutUnit logicalTop = beforeCollapseLogicalTop;
-
-    LayoutUnit clearanceForSelfCollapsingBlock;
-
     // If the child's previous sibling is a self-collapsing block that cleared a float then its top border edge has been set at the bottom border edge
     // of the float. Since we want to collapse the child's top margin with the self-collapsing block's top and bottom margins we need to adjust our parent's height to match the 
     // margin top of the self-collapsing block. If the resulting collapsed margin leaves the child still intruding into the float then we will want to clear it.
-    if (!marginInfo.canCollapseWithMarginBefore() && is<RenderBlockFlow>(prevSibling) && downcast<RenderBlockFlow>(*prevSibling).isSelfCollapsingBlock()) {
-        clearanceForSelfCollapsingBlock = downcast<RenderBlockFlow>(*prevSibling).marginOffsetForSelfCollapsingBlock();
-        setLogicalHeight(logicalHeight() - clearanceForSelfCollapsingBlock);
-    }
+    if (!marginInfo.canCollapseWithMarginBefore() && is<RenderBlockFlow>(prevSibling) && downcast<RenderBlockFlow>(*prevSibling).isSelfCollapsingBlock())
+        setLogicalHeight(logicalHeight() - downcast<RenderBlockFlow>(*prevSibling).marginOffsetForSelfCollapsingBlock());
 
     if (childIsSelfCollapsing) {
         // This child has no height. We need to compute our
@@ -1227,10 +1222,10 @@ LayoutUnit RenderBlockFlow::collapseMarginsWithChildInfo(RenderBox* child, Rende
             addOverhangingFloats(block, false);
         setLogicalHeight(oldLogicalHeight);
 
-        // If |child|'s previous sibling is a self-collapsing block that cleared a float and margin collapsing resulted in |child| moving up
+        // If |child|'s previous sibling is or contains a self-collapsing block that cleared a float and margin collapsing resulted in |child| moving up
         // into the margin area of the self-collapsing block then the float it clears is now intruding into |child|. Layout again so that we can look for
         // floats in the parent that overhang |child|'s new logical top.
-        bool logicalTopIntrudesIntoFloat = clearanceForSelfCollapsingBlock > 0 && logicalTop < beforeCollapseLogicalTop;
+        bool logicalTopIntrudesIntoFloat = logicalTop < beforeCollapseLogicalTop;
         if (child && logicalTopIntrudesIntoFloat && containsFloats() && !child->avoidsFloats() && lowestFloatLogicalBottom() > logicalTop)
             child->setNeedsLayout();
     }