[LFC][BFC] Do not collapse top/bottom margin with first/last inflow child from a...
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 21 Jul 2018 15:00:30 +0000 (15:00 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 21 Jul 2018 15:00:30 +0000 (15:00 +0000)
https://bugs.webkit.org/show_bug.cgi?id=187867

Reviewed by Antti Koivisto.

The box's top/bottom margin never collapses with a non-block inflow child.

* layout/blockformatting/BlockMarginCollapse.cpp:
(WebCore::Layout::isMarginTopCollapsedWithSibling):
(WebCore::Layout::isMarginBottomCollapsedWithSibling):
(WebCore::Layout::isMarginTopCollapsedWithParent):
(WebCore::Layout::isMarginBottomCollapsedThrough):
(WebCore::Layout::BlockFormattingContext::MarginCollapse::collapsedMarginTopFromFirstChild):
(WebCore::Layout::BlockFormattingContext::MarginCollapse::nonCollapsedMarginTop):
(WebCore::Layout::BlockFormattingContext::MarginCollapse::computedNonCollapsedMarginTop):
(WebCore::Layout::BlockFormattingContext::MarginCollapse::computedNonCollapsedMarginBottom):
(WebCore::Layout::BlockFormattingContext::MarginCollapse::marginTop):
(WebCore::Layout::BlockFormattingContext::MarginCollapse::marginBottom):
(WebCore::Layout::BlockFormattingContext::MarginCollapse::isMarginBottomCollapsedWithParent):
(WebCore::Layout::BlockFormattingContext::MarginCollapse::collapsedMarginBottomFromLastChild):
(WebCore::Layout::BlockFormattingContext::MarginCollapse::nonCollapsedMarginBottom):
* layout/layouttree/LayoutBox.cpp:
(WebCore::Layout::Box::establishesBlockFormattingContextOnly const): <div style="overflow: hidden">foobar</div> establishes both inline and block formatting context (inline wins though).
* layout/layouttree/LayoutBox.h: establishesBlockFormattingContext() does not need to be virtual since we can determine it by looking at the box's style. -while in case
of inline formatting context, it is about the content.

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

Source/WebCore/ChangeLog
Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp
Source/WebCore/layout/layouttree/LayoutBox.cpp
Source/WebCore/layout/layouttree/LayoutBox.h

index 802e5dd..37e1380 100644 (file)
@@ -1,3 +1,31 @@
+2018-07-21  Zalan Bujtas  <zalan@apple.com>
+
+        [LFC][BFC] Do not collapse top/bottom margin with first/last inflow child from a non-block formatting context.
+        https://bugs.webkit.org/show_bug.cgi?id=187867
+
+        Reviewed by Antti Koivisto.
+
+        The box's top/bottom margin never collapses with a non-block inflow child.
+
+        * layout/blockformatting/BlockMarginCollapse.cpp:
+        (WebCore::Layout::isMarginTopCollapsedWithSibling):
+        (WebCore::Layout::isMarginBottomCollapsedWithSibling):
+        (WebCore::Layout::isMarginTopCollapsedWithParent):
+        (WebCore::Layout::isMarginBottomCollapsedThrough):
+        (WebCore::Layout::BlockFormattingContext::MarginCollapse::collapsedMarginTopFromFirstChild):
+        (WebCore::Layout::BlockFormattingContext::MarginCollapse::nonCollapsedMarginTop):
+        (WebCore::Layout::BlockFormattingContext::MarginCollapse::computedNonCollapsedMarginTop):
+        (WebCore::Layout::BlockFormattingContext::MarginCollapse::computedNonCollapsedMarginBottom):
+        (WebCore::Layout::BlockFormattingContext::MarginCollapse::marginTop):
+        (WebCore::Layout::BlockFormattingContext::MarginCollapse::marginBottom):
+        (WebCore::Layout::BlockFormattingContext::MarginCollapse::isMarginBottomCollapsedWithParent):
+        (WebCore::Layout::BlockFormattingContext::MarginCollapse::collapsedMarginBottomFromLastChild):
+        (WebCore::Layout::BlockFormattingContext::MarginCollapse::nonCollapsedMarginBottom):
+        * layout/layouttree/LayoutBox.cpp:
+        (WebCore::Layout::Box::establishesBlockFormattingContextOnly const): <div style="overflow: hidden">foobar</div> establishes both inline and block formatting context (inline wins though).
+        * layout/layouttree/LayoutBox.h: establishesBlockFormattingContext() does not need to be virtual since we can determine it by looking at the box's style. -while in case
+        of inline formatting context, it is about the content.
+
 2018-07-20  Jer Noble  <jer.noble@apple.com>
 
         REGRESSION (r233974): Cannot close pip'd video; pops back into PiP.
index 870c4d6..d7c0a83 100644 (file)
@@ -54,6 +54,8 @@ static LayoutUnit marginValue(LayoutUnit currentMarginValue, LayoutUnit candidat
 
 static bool isMarginTopCollapsedWithSibling(const Box& layoutBox)
 {
+    ASSERT(layoutBox.isBlockLevelBox());
+
     if (layoutBox.isFloatingPositioned())
         return false;
 
@@ -67,6 +69,8 @@ static bool isMarginTopCollapsedWithSibling(const Box& layoutBox)
 
 static bool isMarginBottomCollapsedWithSibling(const Box& layoutBox)
 {
+    ASSERT(layoutBox.isBlockLevelBox());
+
     if (layoutBox.isFloatingPositioned())
         return false;
 
@@ -85,6 +89,8 @@ static bool isMarginTopCollapsedWithParent(const LayoutContext& layoutContext, c
     if (layoutBox.isAnonymous())
         return false;
 
+    ASSERT(layoutBox.isBlockLevelBox());
+
     if (layoutBox.isFloatingOrOutOfFlowPositioned())
         return false;
 
@@ -114,6 +120,8 @@ static bool isMarginTopCollapsedWithParent(const LayoutContext& layoutContext, c
 
 static bool isMarginBottomCollapsedThrough(const LayoutContext& layoutContext, const Box& layoutBox)
 {
+    ASSERT(layoutBox.isBlockLevelBox());
+
     // If the top and bottom margins of a box are adjoining, then it is possible for margins to collapse through it.
     auto& displayBox = *layoutContext.displayBoxForLayoutBox(layoutBox);
 
@@ -138,10 +146,16 @@ static bool isMarginBottomCollapsedThrough(const LayoutContext& layoutContext, c
 
 LayoutUnit BlockFormattingContext::MarginCollapse::collapsedMarginTopFromFirstChild(const LayoutContext& layoutContext, const Box& layoutBox)
 {
+    ASSERT(layoutBox.isBlockLevelBox());
+
     // Check if the first child collapses its margin top.
     if (!is<Container>(layoutBox) || !downcast<Container>(layoutBox).hasInFlowChild())
         return 0;
 
+    // Do not collapse margin with a box from a non-block formatting context <div><span>foobar</span></div>.
+    if (layoutBox.establishesFormattingContext() && !layoutBox.establishesBlockFormattingContextOnly())
+        return 0;
+
     // FIXME: Take collapsed through margin into account.
     auto& firstInFlowChild = *downcast<Container>(layoutBox).firstInFlowChild();
     if (!isMarginTopCollapsedWithParent(layoutContext, firstInFlowChild))
@@ -152,6 +166,8 @@ LayoutUnit BlockFormattingContext::MarginCollapse::collapsedMarginTopFromFirstCh
 
 LayoutUnit BlockFormattingContext::MarginCollapse::nonCollapsedMarginTop(const LayoutContext& layoutContext, const Box& layoutBox)
 {
+    ASSERT(layoutBox.isBlockLevelBox());
+
     // Non collapsed margin top includes collapsed margin from inflow first child.
     return marginValue(computedNonCollapsedMarginTop(layoutContext, layoutBox), collapsedMarginTopFromFirstChild(layoutContext, layoutBox));
 }
@@ -172,11 +188,15 @@ LayoutUnit BlockFormattingContext::MarginCollapse::nonCollapsedMarginTop(const L
 }*/
 LayoutUnit BlockFormattingContext::MarginCollapse::computedNonCollapsedMarginTop(const LayoutContext& layoutContext, const Box& layoutBox)
 {
+    ASSERT(layoutBox.isBlockLevelBox());
+
     return FormattingContext::Geometry::computedNonCollapsedVerticalMarginValue(layoutContext, layoutBox).top;
 }
 
 LayoutUnit BlockFormattingContext::MarginCollapse::computedNonCollapsedMarginBottom(const LayoutContext& layoutContext, const Box& layoutBox)
 {
+    ASSERT(layoutBox.isBlockLevelBox());
+
     return FormattingContext::Geometry::computedNonCollapsedVerticalMarginValue(layoutContext, layoutBox).bottom;
 }
 
@@ -185,6 +205,8 @@ LayoutUnit BlockFormattingContext::MarginCollapse::marginTop(const LayoutContext
     if (layoutBox.isAnonymous())
         return 0;
 
+    ASSERT(layoutBox.isBlockLevelBox());
+
     // TODO: take _hasAdjoiningMarginTopAndBottom() into account.
     if (isMarginTopCollapsedWithParent(layoutContext, layoutBox))
         return 0;
@@ -214,6 +236,8 @@ LayoutUnit BlockFormattingContext::MarginCollapse::marginBottom(const LayoutCont
     if (layoutBox.isAnonymous())
         return 0;
 
+    ASSERT(layoutBox.isBlockLevelBox());
+
     // TODO: take _hasAdjoiningMarginTopAndBottom() into account.
     if (isMarginBottomCollapsedWithParent(layoutContext, layoutBox))
         return 0;
@@ -239,6 +263,8 @@ bool BlockFormattingContext::MarginCollapse::isMarginBottomCollapsedWithParent(c
     if (layoutBox.isAnonymous())
         return false;
 
+    ASSERT(layoutBox.isBlockLevelBox());
+
     if (layoutBox.isFloatingOrOutOfFlowPositioned())
         return false;
 
@@ -279,10 +305,16 @@ bool BlockFormattingContext::MarginCollapse::isMarginTopCollapsedWithParentMargi
 
 LayoutUnit BlockFormattingContext::MarginCollapse::collapsedMarginBottomFromLastChild(const LayoutContext& layoutContext, const Box& layoutBox)
 {
+    ASSERT(layoutBox.isBlockLevelBox());
+
     // Check if the last child propagates its margin bottom.
     if (!is<Container>(layoutBox) || !downcast<Container>(layoutBox).hasInFlowChild())
         return 0;
 
+    // Do not collapse margin with a box from a non-block formatting context <div><span>foobar</span></div>.
+    if (layoutBox.establishesFormattingContext() && !layoutBox.establishesBlockFormattingContextOnly())
+        return 0;
+
     // FIXME: Check for collapsed through margin.
     auto& lastInFlowChild = *downcast<Container>(layoutBox).lastInFlowChild();
     if (!isMarginBottomCollapsedWithParent(layoutContext, lastInFlowChild))
@@ -294,6 +326,8 @@ LayoutUnit BlockFormattingContext::MarginCollapse::collapsedMarginBottomFromLast
 
 LayoutUnit BlockFormattingContext::MarginCollapse::nonCollapsedMarginBottom(const LayoutContext& layoutContext, const Box& layoutBox)
 {
+    ASSERT(layoutBox.isBlockLevelBox());
+
     // Non collapsed margin bottom includes collapsed margin from inflow last child.
     return marginValue(computedNonCollapsedMarginBottom(layoutContext, layoutBox), collapsedMarginBottomFromLastChild(layoutContext, layoutBox));
 }
index 24bcc06..df765e5 100644 (file)
@@ -70,6 +70,11 @@ bool Box::establishesBlockFormattingContext() const
     return false;
 }
 
+bool Box::establishesBlockFormattingContextOnly() const
+{
+    return establishesBlockFormattingContext() && !establishesInlineFormattingContext();
+}
+
 bool Box::isRelativelyPositioned() const
 {
     return m_style.position() == PositionType::Relative;
index fe2c1ea..893729e 100644 (file)
@@ -47,7 +47,8 @@ public:
     virtual ~Box();
 
     bool establishesFormattingContext() const;
-    virtual bool establishesBlockFormattingContext() const;
+    bool establishesBlockFormattingContext() const;
+    bool establishesBlockFormattingContextOnly() const;
     virtual bool establishesInlineFormattingContext() const { return false; }
 
     bool isInFlow() const { return !isFloatingOrOutOfFlowPositioned(); }