Attachment 348641 Details for Bug 189193 – Patch

[patch] Patch

bug-189193-20180831091318.patch (text/plain), 8.70 KB, created by zalan on 2018-08-31 09:13:19 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: zalan

Created: 2018-08-31 09:13:19 PDT

Size: 8.70 KB

patch

obsolete

>Subversion Revision: 235509
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index f950ca9559df3119b9988010803f549dbedbb551..3face9dd27a9456dc1f739821942fdfc03a73773 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,23 @@
>+2018-08-31  Zalan Bujtas  <zalan@apple.com>
>+
>+        [LFC] Add margin box verification back now that Display::Box has non-computed horizontal margin.
>+        https://bugs.webkit.org/show_bug.cgi?id=189193
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Use the non-computed margin values to verify correctness.
>+        This patch also fixes a margin collapsing issue when the inflow box has a float sibling. The float
>+        sibling does not prevent collapsing with the parent's top/bottom margin.
>+
>+        Test: fast/block/block-only/floating-and-next-previous-inflow-with-margin-with-no-border.html
>+
>+        * layout/Verification.cpp:
>+        (WebCore::Layout::outputMismatchingBlockBoxInformationIfNeeded):
>+        (WebCore::Layout::LayoutContext::verifyAndOutputMismatchingLayoutTree const):
>+        * layout/blockformatting/BlockMarginCollapse.cpp:
>+        (WebCore::Layout::isMarginTopCollapsedWithParent):
>+        (WebCore::Layout::BlockFormattingContext::MarginCollapse::isMarginBottomCollapsedWithParent):
>+
> 2018-08-30  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC][Floating] Block formatting context roots avoid floats.
>diff --git a/Source/WebCore/layout/Verification.cpp b/Source/WebCore/layout/Verification.cpp
>index ef11025840b5d0c1746cec64cb0e8bbf16342117..6b5fe7b6294790ab501f8241078bb2a3c595ca71 100644
>--- a/Source/WebCore/layout/Verification.cpp
>+++ b/Source/WebCore/layout/Verification.cpp
>@@ -121,6 +121,18 @@ static bool outputMismatchingBlockBoxInformationIfNeeded(TextStream& stream, con
>         stream.nextLine();
>     };
> 
>+    auto renderBoxLikeMarginBox = [](auto& displayBox) {
>+        // Produce a RenderBox matching margin box. 
>+        auto borderBox = displayBox.borderBox();
>+
>+        return Display::Box::Rect { 
>+            borderBox.top() - displayBox.nonCollapsedMarginTop()
>+            , borderBox.left() - displayBox.nonComputedMarginLeft()
>+            , displayBox.nonComputedMarginLeft() + borderBox.width() + displayBox.nonComputedMarginRight()
>+            , displayBox.nonCollapsedMarginTop() + borderBox.height() + displayBox.nonCollapsedMarginBottom() 
>+        };
>+    };
>+
>     auto* displayBox = context.displayBoxForLayoutBox(layoutBox);
>     ASSERT(displayBox);
> 
>@@ -134,6 +146,11 @@ static bool outputMismatchingBlockBoxInformationIfNeeded(TextStream& stream, con
>         return true;
>     }
> 
>+    if (renderer.marginBoxRect() != renderBoxLikeMarginBox(*displayBox)) {
>+        outputRect("marginBox", renderer.marginBoxRect(), renderBoxLikeMarginBox(*displayBox));
>+        return true;
>+    }
>+
>     if (renderer.borderBoxRect() != displayBox->borderBox()) {
>         outputRect("borderBox", renderer.borderBoxRect(), displayBox->borderBox());
>         return true;
>@@ -149,7 +166,6 @@ static bool outputMismatchingBlockBoxInformationIfNeeded(TextStream& stream, con
>         return true;
>     }
> 
>-    // TODO: The RenderBox::marginBox() does not follow the spec and ignores certain constraints. Skip them for now.
>     return false;
> }
> 
>diff --git a/Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp b/Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp
>index 7e54af7e6536f0f7ebec880644a7339e8719f38b..ed3b3c96146b12e2d497ccffd7a683d1ca635173 100644
>--- a/Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp
>+++ b/Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp
>@@ -97,8 +97,8 @@ static bool isMarginTopCollapsedWithParent(const LayoutContext& layoutContext, c
>     // We never margin collapse the initial containing block.
>     ASSERT(layoutBox.parent());
>     auto& parent = *layoutBox.parent();
>-    // Only the first inlflow child collapses with parent (floating sibling also prevents collapsing). 
>-    if (layoutBox.previousInFlowOrFloatingSibling())
>+    // Only the first inlflow child collapses with parent. 
>+    if (layoutBox.previousInFlowSibling())
>         return false;
> 
>     if (parent.establishesBlockFormattingContext())
>@@ -274,8 +274,8 @@ bool BlockFormattingContext::MarginCollapse::isMarginBottomCollapsedWithParent(c
>     // We never margin collapse the initial containing block.
>     ASSERT(layoutBox.parent());
>     auto& parent = *layoutBox.parent();
>-    // Only the last inlflow child collapses with parent (floating sibling also prevents collapsing). 
>-    if (layoutBox.nextInFlowOrFloatingSibling())
>+    // Only the last inlflow child collapses with parent. 
>+    if (layoutBox.nextInFlowSibling())
>         return false;
> 
>     if (parent.establishesBlockFormattingContext())
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 7cec6187fe5500d5bf80f52d8061660ba32b4e7d..cc2daf9c6bca245eca6b4524153abf70d2fb83c1 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,12 @@
>+2018-08-31  Zalan Bujtas  <zalan@apple.com>
>+
>+        [LFC] Add margin box verification back now that Display::Box has non-computed horizontal margin.
>+        https://bugs.webkit.org/show_bug.cgi?id=189193
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * LayoutReloaded/misc/LFC-passing-tests.txt:
>+
> 2018-08-30  Sihui Liu  <sihui_liu@apple.com>
> 
>         WKNavigation.ProcessCrashDuringCallback is failing on iOS
>diff --git a/Tools/LayoutReloaded/misc/LFC-passing-tests.txt b/Tools/LayoutReloaded/misc/LFC-passing-tests.txt
>index 506cf8a6dac3f4df08aaf395810ab55dfecad2f3..640ef446ead19651986cab06f3e0083e1b0e77b0 100644
>--- a/Tools/LayoutReloaded/misc/LFC-passing-tests.txt
>+++ b/Tools/LayoutReloaded/misc/LFC-passing-tests.txt
>@@ -47,6 +47,7 @@ fast/block/block-only/floating-multiple-lefts-in-body.html
> 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-and-next-previous-inflow-with-margin-with-no-border.html
> fast/block/block-only/floating-left-and-right-with-clearance.html
> fast/block/block-only/float-and-siblings-with-margins.html
> fast/block/block-only/margin-collapse-with-clearance.html
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 6553ed1c58209c8dfe9d410adf1b0303db74a3ac..5e3d9ce5fd1044d80c7dbcfc6783f8a5b6396718 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,13 @@
>+2018-08-31  Zalan Bujtas  <zalan@apple.com>
>+
>+        [LFC] Add margin box verification back now that Display::Box has non-computed horizontal margin.
>+        https://bugs.webkit.org/show_bug.cgi?id=189193
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * fast/block/block-only/floating-and-next-previous-inflow-with-margin-with-no-border-expected.txt: Added.
>+        * fast/block/block-only/floating-and-next-previous-inflow-with-margin-with-no-border.html: Added.
>+
> 2018-08-30  Youenn Fablet  <youenn@apple.com>
> 
>         fast/mediastream/RTCPeerConnection-overloaded-operations.html is flaky after r235484
>diff --git a/LayoutTests/fast/block/block-only/floating-and-next-previous-inflow-with-margin-with-no-border-expected.txt b/LayoutTests/fast/block/block-only/floating-and-next-previous-inflow-with-margin-with-no-border-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..d413691e0ab4644d981570500fc60ec29e91b8e2
>--- /dev/null
>+++ b/LayoutTests/fast/block/block-only/floating-and-next-previous-inflow-with-margin-with-no-border-expected.txt
>@@ -0,0 +1,10 @@
>+layer at (0,0) size 800x600
>+  RenderView at (0,0) size 800x600
>+layer at (0,0) size 800x128
>+  RenderBlock {HTML} at (0,0) size 800x128
>+    RenderBody {BODY} at (8,20) size 784x100
>+      RenderBlock {DIV} at (0,0) size 100x100
>+        RenderBlock (floating) {DIV} at (0,0) size 10x10
>+        RenderBlock {DIV} at (0,0) size 10x10
>+        RenderBlock {DIV} at (0,10) size 10x10
>+        RenderBlock (floating) {DIV} at (0,40) size 10x10
>diff --git a/LayoutTests/fast/block/block-only/floating-and-next-previous-inflow-with-margin-with-no-border.html b/LayoutTests/fast/block/block-only/floating-and-next-previous-inflow-with-margin-with-no-border.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..e9c2ccab557aac4ec10b722d03dbf000c48e0356
>--- /dev/null
>+++ b/LayoutTests/fast/block/block-only/floating-and-next-previous-inflow-with-margin-with-no-border.html
>@@ -0,0 +1,20 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+<style>
>+div {
>+	outline: 1px solid black;
>+	width: 10px;
>+	height: 10px;
>+}
>+</style>
>+</head>
>+<body>
>+<div style="width: 100px; height: 100px;">
>+    <div style="float: left;"></div>
>+    <div style="margin-top: 20px;"></div>
>+    <div style="margin-bottom: 20px;"></div>
>+    <div style="float: left;"></div>
>+</div>
>+</body>
>+</html>

Flags:

koivisto: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 189193: 348641