Attachment 347016 Details for Bug 188522 – Patch

[patch] Patch

bug-188522-20180813104101.patch (text/plain), 10.46 KB, created by zalan on 2018-08-13 10:41:02 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: zalan

Created: 2018-08-13 10:41:02 PDT

Size: 10.46 KB

patch

obsolete

>Subversion Revision: 234800
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 53b6825c468dc7d339856a618733fcc8d71af123..df4ba8637ee68590470287eb9e55bb0da235a3d2 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,21 @@
>+2018-08-13  Zalan Bujtas  <zalan@apple.com>
>+
>+        [LFC][BFC] Add clearance rule to margin collapsing.
>+        https://bugs.webkit.org/show_bug.cgi?id=188522
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        8.3.1 Collapsing margins
>+
>+        "..The bottom margin of an in-flow block-level element always collapses with the top margin of its
>+        next in-flow block-level sibling, unless that sibling has clearance..."
>+
>+        Test: fast/block/block-only/margin-collapse-with-clearance.html
>+
>+        * layout/blockformatting/BlockMarginCollapse.cpp:
>+        (WebCore::Layout::clearancePreventsBottomMarginCollapsing):
>+        (WebCore::Layout::BlockFormattingContext::MarginCollapse::marginBottom):
>+
> 2018-08-13  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC][Floating] Add basic clearance support
>diff --git a/Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp b/Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp
>index 8610f1bb2ee36cbc66e756cc9a1b769eab45f1d3..a80f42acee2d21ab8d15c2208b49afc1af86435a 100644
>--- a/Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp
>+++ b/Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp
>@@ -144,6 +144,42 @@ static bool isMarginBottomCollapsedThrough(const LayoutContext& layoutContext, c
>     return true;
> }
> 
>+static bool clearancePreventsBottomMarginCollapsing(const Box& nextInflowOrFloatingSibling)
>+{
>+    // Check if the next inflow sibling has clearance and whether it actually prevents margin collapsing.
>+    //
>+    // 1. Next inflow sibling has to have the clear property set (left/right/both).
>+    // 2. There has to be a clearance matching floating box inbetween.
>+
>+    auto* nextSibling = &nextInflowOrFloatingSibling;
>+    if (!nextSibling->isFloatingPositioned())
>+        return false;
>+
>+    // Skip the floating positioned siblings until we find the next inflow box and check if it has matching clearance.
>+    auto forClearance = nextSibling->isLeftFloatingPositioned() ? Clear::Left : Clear::Right;
>+    while (true) {
>+        nextSibling = nextSibling->nextInFlowOrFloatingSibling();
>+        if (!nextSibling)
>+            return false;
>+
>+        if (nextSibling->isFloatingPositioned()) {
>+            if ((nextSibling->isLeftFloatingPositioned() && forClearance == Clear::Right) || (nextSibling->isRightFloatingPositioned() && forClearance == Clear::Left))
>+                forClearance = Clear::Both;
>+            continue;
>+        }
>+
>+        // This is the first (next) inflow sibling.
>+        ASSERT(nextSibling->isInFlow());
>+        if (!nextSibling->hasClearance())
>+            return false;
>+
>+        auto clear = nextSibling->style().clear();
>+        return clear == Clear::Both || clear == forClearance;
>+    }
>+
>+    return false;
>+}
>+
> LayoutUnit BlockFormattingContext::MarginCollapse::collapsedMarginTopFromFirstChild(const LayoutContext& layoutContext, const Box& layoutBox)
> {
>     ASSERT(layoutBox.isBlockLevelBox());
>@@ -249,11 +285,15 @@ LayoutUnit BlockFormattingContext::MarginCollapse::marginBottom(const LayoutCont
>     if (!isMarginBottomCollapsedWithSibling(layoutBox))
>         return nonCollapsedMarginBottom(layoutContext, layoutBox);
> 
>+    if (!layoutBox.nextInFlowSibling())
>+        return nonCollapsedMarginBottom(layoutContext, layoutBox);
>+
>     // The bottom margin of an in-flow block-level element always collapses with the top margin of its next in-flow block-level sibling,
>     // unless that sibling has clearance.
>-    if (layoutBox.nextInFlowSibling())
>-        return 0;
>-    return nonCollapsedMarginBottom(layoutContext, layoutBox);
>+    if (clearancePreventsBottomMarginCollapsing(*layoutBox.nextInFlowOrFloatingSibling()))
>+        return nonCollapsedMarginBottom(layoutContext, layoutBox);
>+
>+    return 0;
> }
> 
> bool BlockFormattingContext::MarginCollapse::isMarginBottomCollapsedWithParent(const LayoutContext& layoutContext, const Box& layoutBox)
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index c49660591afac7b56667f5278e5fbe2f15e40c1d..6eea551bda8a85970d72a6c981e0bb0e83ea8315 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,12 @@
>+2018-08-13  Zalan Bujtas  <zalan@apple.com>
>+
>+        [LFC][BFC] Add clearance rule to margin collapsing.
>+        https://bugs.webkit.org/show_bug.cgi?id=188522
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * LayoutReloaded/misc/LFC-passing-tests.txt:
>+
> 2018-08-13  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC][Floating] Add basic clearance support
>diff --git a/Tools/LayoutReloaded/misc/LFC-passing-tests.txt b/Tools/LayoutReloaded/misc/LFC-passing-tests.txt
>index aba76f45f0f153644a27304b5d4f90135db0b6ab..dd861a652c3b30dedaa0129aab923d8358a2ef8c 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/margin-collapse-with-clearance.html
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 1fa9e388149ef143435806518d2e9d92ad53efdd..1f4e6d3f2bcc745ab4996cbb99db4d63aaf04cb3 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,13 @@
>+2018-08-13  Zalan Bujtas  <zalan@apple.com>
>+
>+        [LFC][BFC] Add clearance rule to margin collapsing.
>+        https://bugs.webkit.org/show_bug.cgi?id=188522
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * fast/block/block-only/margin-collapse-with-clearance-expected.txt: Added.
>+        * fast/block/block-only/margin-collapse-with-clearance.html: Added.
>+
> 2018-08-13  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC][Floating] Add basic clearance support
>diff --git a/LayoutTests/fast/block/block-only/margin-collapse-with-clearance-expected.txt b/LayoutTests/fast/block/block-only/margin-collapse-with-clearance-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..90214244b5920dc1ff587608902de0756c7953c3
>--- /dev/null
>+++ b/LayoutTests/fast/block/block-only/margin-collapse-with-clearance-expected.txt
>@@ -0,0 +1,29 @@
>+layer at (0,0) size 785x628
>+  RenderView at (0,0) size 785x600
>+layer at (0,0) size 785x628
>+  RenderBlock {HTML} at (0,0) size 785x628
>+    RenderBody {BODY} at (8,8) size 769x612
>+      RenderBlock {DIV} at (0,0) size 502x102 [border: (1px solid #000000)]
>+        RenderBlock {DIV} at (1,1) size 12x12 [border: (1px solid #000000)]
>+        RenderBlock (floating) {DIV} at (1,23) size 52x22 [border: (1px solid #000000)]
>+        RenderBlock {DIV} at (1,45) size 12x12 [border: (1px solid #000000)]
>+      RenderBlock {DIV} at (0,102) size 502x102 [border: (1px solid #000000)]
>+        RenderBlock {DIV} at (1,1) size 12x12 [border: (1px solid #000000)]
>+        RenderBlock (floating) {DIV} at (449,23) size 52x22 [border: (1px solid #000000)]
>+        RenderBlock {DIV} at (1,45) size 12x12 [border: (1px solid #000000)]
>+      RenderBlock {DIV} at (0,204) size 502x102 [border: (1px solid #000000)]
>+        RenderBlock {DIV} at (1,1) size 12x12 [border: (1px solid #000000)]
>+        RenderBlock {DIV} at (1,43) size 12x12 [border: (1px solid #000000)]
>+      RenderBlock {DIV} at (0,306) size 502x102 [border: (1px solid #000000)]
>+        RenderBlock {DIV} at (1,1) size 12x12 [border: (1px solid #000000)]
>+        RenderBlock (floating) {DIV} at (1,23) size 52x22 [border: (1px solid #000000)]
>+        RenderBlock (floating) {DIV} at (449,23) size 52x22 [border: (1px solid #000000)]
>+        RenderBlock {DIV} at (1,45) size 12x12 [border: (1px solid #000000)]
>+      RenderBlock {DIV} at (0,408) size 502x102 [border: (1px solid #000000)]
>+        RenderBlock {DIV} at (1,1) size 12x12 [border: (1px solid #000000)]
>+        RenderBlock (floating) {DIV} at (1,23) size 52x22 [border: (1px solid #000000)]
>+        RenderBlock {DIV} at (1,45) size 12x12 [border: (1px solid #000000)]
>+      RenderBlock {DIV} at (0,510) size 502x102 [border: (1px solid #000000)]
>+        RenderBlock {DIV} at (1,1) size 12x12 [border: (1px solid #000000)]
>+        RenderBlock (floating) {DIV} at (449,23) size 52x22 [border: (1px solid #000000)]
>+        RenderBlock {DIV} at (1,45) size 12x12 [border: (1px solid #000000)]
>diff --git a/LayoutTests/fast/block/block-only/margin-collapse-with-clearance.html b/LayoutTests/fast/block/block-only/margin-collapse-with-clearance.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..67942e3727096482744d132bda2a4c54fa3c989f
>--- /dev/null
>+++ b/LayoutTests/fast/block/block-only/margin-collapse-with-clearance.html
>@@ -0,0 +1,42 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+<style>
>+div {
>+	border: 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: 20px;"></div>
>+	<div style="clear: left; width: 10px; height: 10px; margin-top: 30px"></div>
>+</div>
>+<div style="width: 500px; height: 100px;">
>+	<div style="width: 10px; height: 10px; margin-bottom: 10px"></div>
>+	<div style="float: right; width: 50px; height: 20px;"></div>
>+	<div style="clear: right; width: 10px; height: 10px; margin-top: 30px"></div>
>+</div>
>+<div style="width: 500px; height: 100px;">
>+	<div style="width: 10px; height: 10px; margin-bottom: 10px"></div>
>+	<div style="clear: both; width: 10px; height: 10px; margin-top: 30px"></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: 20px;"></div>
>+	<div style="float: right; width: 50px; height: 20px;"></div>
>+	<div style="clear: both; width: 10px; height: 10px; margin-top: 30px"></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: 20px;"></div>
>+	<div style="clear: both; width: 10px; height: 10px; margin-top: 30px"></div>
>+</div>
>+<div style="width: 500px; height: 100px;">
>+	<div style="width: 10px; height: 10px; margin-bottom: 10px"></div>
>+	<div style="float: right; width: 50px; height: 20px;"></div>
>+	<div style="clear: both; width: 10px; height: 10px; margin-top: 30px"></div>
>+</div>
>+</body>
>+</html>

Flags:

koivisto: review+
ews-watchlist: commit-queue-

Actions: View | Formatted Diff | Diff

Attachments on bug 188522: 347016 | 347025