Attachment 357529 Details for Bug 192798 – Patch

[patch] Patch

Patch.txt (text/plain), 7.00 KB, created by zalan on 2018-12-17 21:05:03 PST

(hide)

Description:

Filename:

MIME Type:

Creator: zalan

Created: 2018-12-17 21:05:03 PST

Size: 7.00 KB

patch

obsolete

>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index e82bb28393c..76cee112623 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,19 @@
>+2018-12-17  Zalan Bujtas  <zalan@apple.com>
>+
>+        [LFC][BFC][MarginCollapsing] Implement marginAfterCollapsesWithParentMarginBefore
>+        https://bugs.webkit.org/show_bug.cgi?id=192798
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * layout/blockformatting/BlockFormattingContext.h:
>+        * layout/blockformatting/BlockFormattingContextGeometry.cpp:
>+        (WebCore::Layout::BlockFormattingContext::Geometry::inFlowNonReplacedHeightAndMargin):
>+        * layout/blockformatting/BlockMarginCollapse.cpp:
>+        (WebCore::Layout::BlockFormattingContext::Geometry::MarginCollapse::collapsedMarginAfterFromLastChild):
>+        (WebCore::Layout::BlockFormattingContext::Geometry::MarginCollapse::marginAfterCollapsesWithParentMarginBefore):
>+        (WebCore::Layout::BlockFormattingContext::Geometry::MarginCollapse::marginAfterCollapsesWithParentMarginAfter):
>+        (WebCore::Layout::BlockFormattingContext::Geometry::MarginCollapse::marginAfter):
>+
> 2018-12-17  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC][BFC][MarginCollapsing] Expand marginsCollapseThrough collapsing logic
>diff --git a/Source/WebCore/layout/blockformatting/BlockFormattingContext.h b/Source/WebCore/layout/blockformatting/BlockFormattingContext.h
>index 1067c4b9ca3..9d24c103b39 100644
>--- a/Source/WebCore/layout/blockformatting/BlockFormattingContext.h
>+++ b/Source/WebCore/layout/blockformatting/BlockFormattingContext.h
>@@ -90,7 +90,7 @@ private:
>             static LayoutUnit marginAfter(const LayoutState&, const Box&);
> 
>             static bool marginBeforeCollapsesWithParentMarginAfter(const Box&);
>-            static bool marginAfterCollapsesWithParentMarginAfter(const Box&);
>+            static bool marginAfterCollapsesWithParentMarginAfter(const LayoutState&, const Box&);
> 
>         private:
>             static LayoutUnit collapsedMarginAfterFromLastChild(const LayoutState&, const Box&);
>@@ -106,7 +106,7 @@ private:
>             static bool marginBeforeCollapsesWithPreviousSibling(const Box&);
>             static bool marginAfterCollapsesWithNextSibling(const Box&);
>             static bool marginAfterCollapsesWithSiblingMarginBeforeWithClearance(const Box&);
>-            static bool marginAfterCollapsesWithParentMarginBefore(const Box&);
>+            static bool marginAfterCollapsesWithParentMarginBefore(const LayoutState&, const Box&);
>             static bool marginsCollapseThrough(const LayoutState&, const Box&);
>         };
> 
>diff --git a/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp b/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp
>index d5158a0964a..dfbd839b21d 100644
>--- a/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp
>+++ b/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp
>@@ -84,7 +84,7 @@ HeightAndMargin BlockFormattingContext::Geometry::inFlowNonReplacedHeightAndMarg
>         // 2. the bottom edge of the bottom (possibly collapsed) margin of its last in-flow child, if the child's bottom margin...
>         auto* lastInFlowChild = downcast<Container>(layoutBox).lastInFlowChild();
>         ASSERT(lastInFlowChild);
>-        if (!MarginCollapse::marginAfterCollapsesWithParentMarginAfter(*lastInFlowChild)) {
>+        if (!MarginCollapse::marginAfterCollapsesWithParentMarginAfter(layoutState, *lastInFlowChild)) {
>             auto& lastInFlowDisplayBox = layoutState.displayBoxForLayoutBox(*lastInFlowChild);
>             return { lastInFlowDisplayBox.bottom() + lastInFlowDisplayBox.marginAfter() - borderAndPaddingTop, { nonCollapsedMargin, collapsedMargin } };
>         }
>diff --git a/Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp b/Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp
>index 619d2a2f299..29cb4c88028 100644
>--- a/Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp
>+++ b/Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp
>@@ -167,7 +167,7 @@ LayoutUnit BlockFormattingContext::Geometry::MarginCollapse::collapsedMarginAfte
> 
>     // FIXME: Check for collapsed through margin.
>     auto& lastInFlowChild = *downcast<Container>(layoutBox).lastInFlowChild();
>-    if (!marginAfterCollapsesWithParentMarginAfter(lastInFlowChild))
>+    if (!marginAfterCollapsesWithParentMarginAfter(layoutState, lastInFlowChild))
>         return 0;
> 
>     // Collect collapsed margin bottom recursively.
>@@ -230,12 +230,25 @@ bool BlockFormattingContext::Geometry::MarginCollapse::marginAfterCollapsesWithS
>     return false;
> }
> 
>-bool BlockFormattingContext::Geometry::MarginCollapse::marginAfterCollapsesWithParentMarginBefore(const Box&)
>+bool BlockFormattingContext::Geometry::MarginCollapse::marginAfterCollapsesWithParentMarginBefore(const LayoutState& layoutState, const Box& layoutBox)
> {
>+    // 1. This is the first in-flow child and its margins collapse through and the margin before collapses with parent's margin before or
>+    // 2. This box's margin before collapses with the previous sibling's margin after and that sibling collapses through and
>+    // we can get to the first in-flow child like that.
>+    auto* firstInFlowChild = layoutBox.parent()->firstInFlowChild();
>+    for (auto* currentBox = &layoutBox; currentBox; currentBox = currentBox->previousInFlowSibling()) {
>+        if (!marginsCollapseThrough(layoutState, *currentBox))
>+            return false;
>+        if (currentBox == firstInFlowChild)
>+            return marginBeforeCollapsesWithParentMarginBefore(layoutState, *currentBox); 
>+        if (!marginBeforeCollapsesWithPreviousSibling(*currentBox))
>+            return false;
>+    }
>+    ASSERT_NOT_REACHED();
>     return false;
> }
> 
>-bool BlockFormattingContext::Geometry::MarginCollapse::marginAfterCollapsesWithParentMarginAfter(const Box& layoutBox)
>+bool BlockFormattingContext::Geometry::MarginCollapse::marginAfterCollapsesWithParentMarginAfter(const LayoutState& layoutState, const Box& layoutBox)
> {
>     if (layoutBox.isAnonymous())
>         return false;
>@@ -281,7 +294,7 @@ bool BlockFormattingContext::Geometry::MarginCollapse::marginAfterCollapsesWithP
> 
>     // nor (if the box's min-height is non-zero) with the box's top margin.
>     auto computedMinHeight = parent.style().logicalMinHeight();
>-    if (!computedMinHeight.isAuto() && computedMinHeight.value() && marginAfterCollapsesWithParentMarginBefore(layoutBox))
>+    if (!computedMinHeight.isAuto() && computedMinHeight.value() && marginAfterCollapsesWithParentMarginBefore(layoutState, layoutBox))
>         return false;
> 
>     return true;
>@@ -406,7 +419,7 @@ LayoutUnit BlockFormattingContext::Geometry::MarginCollapse::marginAfter(const L
>     ASSERT(layoutBox.isBlockLevelBox());
> 
>     // TODO: take _hasAdjoiningMarginBeforeAndBottom() into account.
>-    if (marginAfterCollapsesWithParentMarginAfter(layoutBox))
>+    if (marginAfterCollapsesWithParentMarginAfter(layoutState, layoutBox))
>         return 0;
> 
>     if (marginsCollapseThrough(layoutState, layoutBox))

Flags:

koivisto: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 192798: 357529