Source/WebCore:
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 14 Aug 2018 14:22:45 +0000 (14:22 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 14 Aug 2018 14:22:45 +0000 (14:22 +0000)
[LFC][Floating] Adjust vertical position with non-collapsed previous sibling margin.
https://bugs.webkit.org/show_bug.cgi?id=188543

Reviewed by Antti Koivisto.

This patch ensures that the inital vertical position for a float is adjusted with the non-collapsed sibling margin.

<div id=A style="margin-bottom: 20px;"></div>
<div id=B style='float: left'></div>
<div id=C style="margin-top: 10px;"></div>

While computing the static position for element "B", we simply call marginBottom() on A.
In the case above, A's margin bottom is collapsed with C's margin top and the value is 0 (C.marginTop() is 20px).
However CSS spec says that in block formatting context, the non-collapsed margin should be used instead to offset the float box.
(The reason why this should not be part of the BlockMarginCollapse::marginBottom() logic is because it can not differentiate the context of
sibling float/sibling inflow. When we margin collapse, we always do it in the context of inflow boxes.)

Test: fast/block/block-only/float-and-siblings-with-margins.html

* layout/blockformatting/BlockFormattingContext.cpp:
(WebCore::Layout::BlockFormattingContext::layoutFormattingContextRoot const):
(WebCore::Layout::BlockFormattingContext::computeFloatingPosition const):
* layout/blockformatting/BlockFormattingContext.h:

Tools:
[LFC][Floating] Adjust vertical position with non-collapsing previous sibling margin.
https://bugs.webkit.org/show_bug.cgi?id=188543

Reviewed by Antti Koivisto.

* LayoutReloaded/misc/LFC-passing-tests.txt:

LayoutTests:
[LFC][Floating] Adjust vertical position with non-collapsing previous sibling margin.
https://bugs.webkit.org/show_bug.cgi?id=188543

Reviewed by Antti Koivisto.

* fast/block/block-only/float-and-siblings-with-margins-expected.txt: Added.
* fast/block/block-only/float-and-siblings-with-margins.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@234847 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/block/block-only/float-and-siblings-with-margins-expected.txt [new file with mode: 0644]
LayoutTests/fast/block/block-only/float-and-siblings-with-margins.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp
Source/WebCore/layout/blockformatting/BlockFormattingContext.h
Tools/ChangeLog
Tools/LayoutReloaded/misc/LFC-passing-tests.txt

index 85872cb..e563eb3 100644 (file)
@@ -1,3 +1,13 @@
+2018-08-14  Zalan Bujtas  <zalan@apple.com>
+
+        [LFC][Floating] Adjust vertical position with non-collapsing previous sibling margin.
+        https://bugs.webkit.org/show_bug.cgi?id=188543
+
+        Reviewed by Antti Koivisto.
+
+        * fast/block/block-only/float-and-siblings-with-margins-expected.txt: Added.
+        * fast/block/block-only/float-and-siblings-with-margins.html: Added.
+
 2018-08-14  Yusuke Suzuki  <yusukesuzuki@slowstart.org>
 
         Worker should support unhandled promise rejections
diff --git a/LayoutTests/fast/block/block-only/float-and-siblings-with-margins-expected.txt b/LayoutTests/fast/block/block-only/float-and-siblings-with-margins-expected.txt
new file mode 100644 (file)
index 0000000..da99d48
--- /dev/null
@@ -0,0 +1,17 @@
+layer at (0,0) size 800x600
+  RenderView at (0,0) size 800x600
+layer at (0,0) size 800x316
+  RenderBlock {HTML} at (0,0) size 800x316
+    RenderBody {BODY} at (8,8) size 784x300
+      RenderBlock {DIV} at (0,0) size 500x100
+        RenderBlock {DIV} at (0,0) size 10x10
+        RenderBlock (floating) {DIV} at (0,20) size 50x10
+        RenderBlock {DIV} at (0,20) size 10x10
+      RenderBlock {DIV} at (0,100) size 500x100
+        RenderBlock {DIV} at (0,0) size 10x10
+        RenderBlock (floating) {DIV} at (0,30) size 50x10
+        RenderBlock {DIV} at (0,30) size 10x10
+      RenderBlock {DIV} at (0,200) size 500x100
+        RenderBlock {DIV} at (0,0) size 10x10
+        RenderBlock (floating) {DIV} at (0,20) size 50x10
+        RenderBlock {DIV} at (0,30) size 10x10
diff --git a/LayoutTests/fast/block/block-only/float-and-siblings-with-margins.html b/LayoutTests/fast/block/block-only/float-and-siblings-with-margins.html
new file mode 100644 (file)
index 0000000..54aeabf
--- /dev/null
@@ -0,0 +1,27 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+div {
+       outline: 1px solid black;
+}
+</style>
+</head>
+<body>
+<div style="width: 500px; height: 100px;">
+       <div style="width: 10px; height: 10px; margin-bottom: 10px"></div>
+       <div style="float: left; width: 50px; height: 10px;"></div>
+       <div style="width: 10px; height: 10px; margin-top: 10px"></div>
+</div>
+<div style="width: 500px; height: 100px;">
+       <div style="width: 10px; height: 10px; margin-bottom: 20px"></div>
+       <div style="float: left; width: 50px; height: 10px;"></div>
+       <div style="width: 10px; height: 10px; margin-top: 10px"></div>
+</div>
+<div style="width: 500px; height: 100px;">
+       <div style="width: 10px; height: 10px; margin-bottom: 10px"></div>
+       <div style="float: left; width: 50px; height: 10px;"></div>
+       <div style="width: 10px; height: 10px; margin-top: 20px"></div>
+</div>
+</body>
+</html>
index cf872ff..3d95895 100644 (file)
@@ -1,3 +1,29 @@
+2018-08-14  Zalan Bujtas  <zalan@apple.com>
+
+        [LFC][Floating] Adjust vertical position with non-collapsed previous sibling margin.
+        https://bugs.webkit.org/show_bug.cgi?id=188543
+
+        Reviewed by Antti Koivisto.
+
+        This patch ensures that the inital vertical position for a float is adjusted with the non-collapsed sibling margin.
+
+        <div id=A style="margin-bottom: 20px;"></div>
+        <div id=B style='float: left'></div>
+        <div id=C style="margin-top: 10px;"></div>
+
+        While computing the static position for element "B", we simply call marginBottom() on A.
+        In the case above, A's margin bottom is collapsed with C's margin top and the value is 0 (C.marginTop() is 20px).
+        However CSS spec says that in block formatting context, the non-collapsed margin should be used instead to offset the float box.
+        (The reason why this should not be part of the BlockMarginCollapse::marginBottom() logic is because it can not differentiate the context of
+        sibling float/sibling inflow. When we margin collapse, we always do it in the context of inflow boxes.)
+
+        Test: fast/block/block-only/float-and-siblings-with-margins.html
+
+        * layout/blockformatting/BlockFormattingContext.cpp:
+        (WebCore::Layout::BlockFormattingContext::layoutFormattingContextRoot const):
+        (WebCore::Layout::BlockFormattingContext::computeFloatingPosition const):
+        * layout/blockformatting/BlockFormattingContext.h:
+
 2018-08-14  Yusuke Suzuki  <yusukesuzuki@slowstart.org>
 
         Worker should support unhandled promise rejections
index 0eeab9f..c61e554 100644 (file)
@@ -146,7 +146,7 @@ void BlockFormattingContext::layoutFormattingContextRoot(LayoutContext& layoutCo
     LOG_WITH_STREAM(FormattingContextLayout, stream << "[Compute] -> [Height][Margin] -> for layoutBox(" << &layoutBox << ")");
     computeHeightAndMargin(layoutContext, layoutBox, displayBox);
     if (layoutBox.isFloatingPositioned())
-        computeFloatingPosition(floatingContext, layoutBox, displayBox);
+        computeFloatingPosition(layoutContext, floatingContext, layoutBox, displayBox);
     // Now that we computed the root's height, we can go back and layout the out-of-flow descedants (if any).
     formattingContext->layoutOutOfFlowDescendants(layoutContext, layoutBox);
 }
@@ -156,9 +156,16 @@ void BlockFormattingContext::computeStaticPosition(LayoutContext& layoutContext,
     displayBox.setTopLeft(Geometry::staticPosition(layoutContext, layoutBox));
 }
 
-void BlockFormattingContext::computeFloatingPosition(FloatingContext& floatingContext, const Box& layoutBox, Display::Box& displayBox) const
+void BlockFormattingContext::computeFloatingPosition(LayoutContext& layoutContext, FloatingContext& floatingContext, const Box& layoutBox, Display::Box& displayBox) const
 {
     ASSERT(layoutBox.isFloatingPositioned());
+    // 8.3.1 Collapsing margins
+    // In block formatting context margins between a floated box and any other box do not collapse.
+    // Adjust the static position by using the previous inflow box's non-collapsed margin.
+    if (auto* previousInFlowBox = layoutBox.previousInFlowSibling()) {
+        auto& previousDisplayBox = *layoutContext.displayBoxForLayoutBox(*previousInFlowBox);
+        displayBox.moveVertically(previousDisplayBox.nonCollapsedMarginBottom() - previousDisplayBox.marginBottom());
+    }
     displayBox.setTopLeft(floatingContext.positionForFloat(layoutBox));
     floatingContext.floatingState().append(layoutBox);
 }
index 255f4db..3247ed5 100644 (file)
@@ -56,7 +56,7 @@ private:
     void computeHeightAndMargin(LayoutContext&, const Box&, Display::Box&) const;
 
     void computeStaticPosition(LayoutContext&, const Box&, Display::Box&) const override;
-    void computeFloatingPosition(FloatingContext&, const Box&, Display::Box&) const;
+    void computeFloatingPosition(LayoutContext&, FloatingContext&, const Box&, Display::Box&) const;
     void computeVerticalPositionForFloatClear(const FloatingContext&, const Box&, Display::Box&) const;
     void computeInFlowPositionedPosition(LayoutContext&, const Box&, Display::Box&) const override;
     void computeInFlowWidthAndMargin(LayoutContext&, const Box&, Display::Box&) const;
index bbd7e6e..049918b 100644 (file)
@@ -1,3 +1,12 @@
+2018-08-14  Zalan Bujtas  <zalan@apple.com>
+
+        [LFC][Floating] Adjust vertical position with non-collapsing previous sibling margin.
+        https://bugs.webkit.org/show_bug.cgi?id=188543
+
+        Reviewed by Antti Koivisto.
+
+        * LayoutReloaded/misc/LFC-passing-tests.txt:
+
 2018-08-14  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         Unreviewed. Fix WebDriver tests after r234839.
index aba76f4..51d16c2 100644 (file)
@@ -48,3 +48,4 @@ fast/block/block-only/floating-multiple-lefts-multiple-lines.html
 fast/block/block-only/floating-multiple-lefts.html
 fast/block/block-only/floating-and-next-previous-inflow-with-margin.html
 fast/block/block-only/floating-left-and-right-with-clearance.html
+fast/block/block-only/float-and-siblings-with-margins.html