Attachment 347064 Details for Bug 188543 – Patch

[patch] Patch

bug-188543-20180813213137.patch (text/plain), 9.50 KB, created by zalan on 2018-08-13 21:31:38 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: zalan

Created: 2018-08-13 21:31:38 PDT

Size: 9.50 KB

patch

obsolete

>Subversion Revision: 234834
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index c7699006ca36dd2598dc7c8b8a43c7fe2c5a4f98..78225baea98e21b26113b4699b7e6b0309c3fcc5 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,29 @@
>+2018-08-13  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 NOBODY (OOPS!).
>+
>+        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-13  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC][Floating] Do not confuse clear with clearance.
>diff --git a/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp b/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp
>index 0eeab9f22f36c43a2bd213b8ab031a310afdca82..c61e55482bccc02c5ab0d3d3a8a173b08049ea87 100644
>--- a/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp
>+++ b/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp
>@@ -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);
> }
>diff --git a/Source/WebCore/layout/blockformatting/BlockFormattingContext.h b/Source/WebCore/layout/blockformatting/BlockFormattingContext.h
>index 255f4dba552c1164c1feab989da8204ff83205e2..3247ed568132f9f9642a738acfeddb65064b2e48 100644
>--- a/Source/WebCore/layout/blockformatting/BlockFormattingContext.h
>+++ b/Source/WebCore/layout/blockformatting/BlockFormattingContext.h
>@@ -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;
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 7512a15c957a3717b55d6d6c5cbae6cb5d064ebf..8599f1c418cdd6d9bd5d3e0972cb004129cbfca8 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,12 @@
>+2018-08-13  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 NOBODY (OOPS!).
>+
>+        * LayoutReloaded/misc/LFC-passing-tests.txt:
>+
> 2018-08-13  Thomas Denney  <tdenney@apple.com>
> 
>         Allow the substring 'me' in contributor names and email addresses
>diff --git a/Tools/LayoutReloaded/misc/LFC-passing-tests.txt b/Tools/LayoutReloaded/misc/LFC-passing-tests.txt
>index aba76f45f0f153644a27304b5d4f90135db0b6ab..51d16c24c0f39814d6ee24b54bdf8450d9f6efe1 100644
>--- a/Tools/LayoutReloaded/misc/LFC-passing-tests.txt
>+++ b/Tools/LayoutReloaded/misc/LFC-passing-tests.txt
>@@ -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
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index e8bcc0d00861cccbb329c5e2fe69f5209c719940..4c3dd4fc1c6f1e33e6d5b86be340fa026f051de2 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,13 @@
>+2018-08-13  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 NOBODY (OOPS!).
>+
>+        * 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-13  Joseph Pecoraro  <pecoraro@apple.com>
> 
>         [macOS debug] LayoutTest inspector/worker/resources-in-worker.html is a flaky timeout
>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
>index 0000000000000000000000000000000000000000..da99d480cd542bdd99fe85817ec68d158babff7c
>--- /dev/null
>+++ b/LayoutTests/fast/block/block-only/float-and-siblings-with-margins-expected.txt
>@@ -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
>index 0000000000000000000000000000000000000000..54aeabf454101890980dfa0e4c3cd6ba5842cd33
>--- /dev/null
>+++ b/LayoutTests/fast/block/block-only/float-and-siblings-with-margins.html
>@@ -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>

Flags:

koivisto: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 188543: 347064