[LFC][BFC][MarginCollapsing] Adjust vertical position when box margin collapses through.
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 11 Jan 2019 16:19:41 +0000 (16:19 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 11 Jan 2019 16:19:41 +0000 (16:19 +0000)
https://bugs.webkit.org/show_bug.cgi?id=193346

Reviewed by Antti Koivisto.

Source/WebCore:

If the top and bottom margins of a box are adjoining, then it is possible for margins to collapse through it.
In this case, the position of the element depends on its relationship with the other elements whose margins are being collapsed.

1. If the element's margins are collapsed with its parent's top margin, the top border edge of the box is defined to be the same as the parent's.
2. Otherwise, either the element's parent is not taking part in the margin collapsing, or only the parent's bottom margin is involved.
   The position of the element's top border edge is the same as it would have been if the element had a non-zero bottom border.

Test: fast/block/block-only/collapsed-through-with-parent.html

* layout/MarginTypes.h:
(WebCore::Layout::EstimatedMarginBefore::usedValue const):
* layout/blockformatting/BlockFormattingContext.cpp:
(WebCore::Layout::BlockFormattingContext::computeEstimatedMarginBefore const):
(WebCore::Layout::BlockFormattingContext::computeHeightAndMargin const):
(WebCore::Layout::BlockFormattingContext::adjustedVerticalPositionAfterMarginCollapsing const):
* layout/blockformatting/BlockFormattingContext.h:
* layout/blockformatting/BlockMarginCollapse.cpp:
(WebCore::Layout::BlockFormattingContext::MarginCollapse::estimatedMarginBefore):
(WebCore::Layout::BlockFormattingContext::MarginCollapse::marginBeforeIgnoringCollapsingThrough):

Tools:

* LayoutReloaded/misc/LFC-passing-tests.txt:

LayoutTests:

* fast/block/block-only/collapsed-through-with-parent-expected.txt: Added.
* fast/block/block-only/collapsed-through-with-parent.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/block/block-only/collapsed-through-with-parent-expected.txt [new file with mode: 0644]
LayoutTests/fast/block/block-only/collapsed-through-with-parent.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/layout/MarginTypes.h
Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp
Source/WebCore/layout/blockformatting/BlockFormattingContext.h
Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp
Tools/ChangeLog
Tools/LayoutReloaded/misc/LFC-passing-tests.txt

index a6e591f..97628de 100644 (file)
@@ -1,3 +1,13 @@
+2019-01-11  Zalan Bujtas  <zalan@apple.com>
+
+        [LFC][BFC][MarginCollapsing] Adjust vertical position when box margin collapses through.
+        https://bugs.webkit.org/show_bug.cgi?id=193346
+
+        Reviewed by Antti Koivisto.
+
+        * fast/block/block-only/collapsed-through-with-parent-expected.txt: Added.
+        * fast/block/block-only/collapsed-through-with-parent.html: Added.
+
 2019-01-11  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         Unreviewed GTK gardening. Rebaseline several tests after r239822.
diff --git a/LayoutTests/fast/block/block-only/collapsed-through-with-parent-expected.txt b/LayoutTests/fast/block/block-only/collapsed-through-with-parent-expected.txt
new file mode 100644 (file)
index 0000000..61050a5
--- /dev/null
@@ -0,0 +1,8 @@
+layer at (0,0) size 800x600
+  RenderView at (0,0) size 800x600
+layer at (0,0) size 800x600
+  RenderBlock {HTML} at (0,0) size 800x600
+    RenderBody {BODY} at (8,8) size 784x584
+      RenderBlock {DIV} at (0,0) size 402x402 [border: (1px solid #FF0000)]
+        RenderBlock {DIV} at (1,11) size 100x0
+        RenderBlock {DIV} at (1,81) size 100x0
diff --git a/LayoutTests/fast/block/block-only/collapsed-through-with-parent.html b/LayoutTests/fast/block/block-only/collapsed-through-with-parent.html
new file mode 100644 (file)
index 0000000..0a5d4d8
--- /dev/null
@@ -0,0 +1,4 @@
+<div style="border: 1px solid red; width: 400px; height: 400px">
+       <div style="outline: 2px solid red; width: 100px; margin-top: 10px; margin-bottom: 70px"></div>
+       <div style="outline: 1px solid green; width: 100px; margin-top: 80px"></div>
+</div>
index 76b6276..da837e7 100644 (file)
@@ -1,3 +1,30 @@
+2019-01-11  Zalan Bujtas  <zalan@apple.com>
+
+        [LFC][BFC][MarginCollapsing] Adjust vertical position when box margin collapses through.
+        https://bugs.webkit.org/show_bug.cgi?id=193346
+
+        Reviewed by Antti Koivisto.
+
+        If the top and bottom margins of a box are adjoining, then it is possible for margins to collapse through it.
+        In this case, the position of the element depends on its relationship with the other elements whose margins are being collapsed.
+
+        1. If the element's margins are collapsed with its parent's top margin, the top border edge of the box is defined to be the same as the parent's.
+        2. Otherwise, either the element's parent is not taking part in the margin collapsing, or only the parent's bottom margin is involved.
+           The position of the element's top border edge is the same as it would have been if the element had a non-zero bottom border.
+
+        Test: fast/block/block-only/collapsed-through-with-parent.html
+
+        * layout/MarginTypes.h:
+        (WebCore::Layout::EstimatedMarginBefore::usedValue const):
+        * layout/blockformatting/BlockFormattingContext.cpp:
+        (WebCore::Layout::BlockFormattingContext::computeEstimatedMarginBefore const):
+        (WebCore::Layout::BlockFormattingContext::computeHeightAndMargin const):
+        (WebCore::Layout::BlockFormattingContext::adjustedVerticalPositionAfterMarginCollapsing const):
+        * layout/blockformatting/BlockFormattingContext.h:
+        * layout/blockformatting/BlockMarginCollapse.cpp:
+        (WebCore::Layout::BlockFormattingContext::MarginCollapse::estimatedMarginBefore):
+        (WebCore::Layout::BlockFormattingContext::MarginCollapse::marginBeforeIgnoringCollapsingThrough):
+
 2019-01-10  Myles C. Maxfield  <mmaxfield@apple.com>
 
         [WHLSL] Include the standard library
index ac4aa8b..67b68ab 100644 (file)
@@ -75,7 +75,10 @@ struct UsedHorizontalMargin {
 };
 
 struct EstimatedMarginBefore {
-    LayoutUnit usedValue;
+    LayoutUnit usedValue() const { return collapsedValue.valueOr(nonCollapsedValue); }
+    LayoutUnit nonCollapsedValue;
+    Optional<LayoutUnit> collapsedValue;
+    bool isCollapsedThrough { false };
 };
 
 struct PositiveAndNegativeVerticalMargin {
index ecd8f87..c3b1d82 100644 (file)
@@ -191,7 +191,10 @@ void BlockFormattingContext::computeEstimatedMarginBefore(const Box& layoutBox)
     blockFormattingState().setHasEstimatedMarginBefore(layoutBox);
 
     auto& displayBox = layoutState.displayBoxForLayoutBox(layoutBox);
-    displayBox.setTop(adjustedVerticalPositionAfterMarginCollapsing(layoutBox, estimatedMarginBefore.usedValue));
+    auto nonCollapsedValues = UsedVerticalMargin::NonCollapsedValues { estimatedMarginBefore.nonCollapsedValue, { } };
+    auto collapsedValues = UsedVerticalMargin::CollapsedValues { estimatedMarginBefore.collapsedValue, { }, estimatedMarginBefore.isCollapsedThrough };
+    auto verticalMargin = UsedVerticalMargin { nonCollapsedValues, collapsedValues };
+    displayBox.setTop(adjustedVerticalPositionAfterMarginCollapsing(layoutBox, verticalMargin));
 #if !ASSERT_DISABLED
     displayBox.setEstimatedMarginBefore(estimatedMarginBefore);
 #endif
@@ -372,21 +375,33 @@ void BlockFormattingContext::computeHeightAndMargin(const Box& layoutBox) const
         }
     }
 
+    // 1. Compute collapsed margins.
+    // 2. Adjust vertical position using the collaped values
+    // 3. Adjust previous in-flow sibling margin after using this margin.
     auto collapsedMargin = MarginCollapse::collapsedVerticalValues(layoutState, layoutBox, heightAndMargin.nonCollapsedMargin);
     auto verticalMargin = UsedVerticalMargin { heightAndMargin.nonCollapsedMargin, collapsedMargin };
     auto& displayBox = layoutState.displayBoxForLayoutBox(layoutBox);
+
+    // Out of flow boxes don't need vertical adjustment after margin collapsing.
+    if (layoutBox.isOutOfFlowPositioned()) {
+        displayBox.setContentBoxHeight(heightAndMargin.height);
+        displayBox.setVerticalMargin(verticalMargin);
+        return;
+    }
+
     auto& formattingState = this->blockFormattingState();
     if (formattingState.hasEstimatedMarginBefore(layoutBox)) {
         formattingState.clearHasEstimatedMarginBefore(layoutBox);
-        ASSERT(displayBox.estimatedMarginBefore()->usedValue == verticalMargin.before());
+        ASSERT(displayBox.estimatedMarginBefore()->usedValue() == verticalMargin.before());
     } else
-        displayBox.setTop(adjustedVerticalPositionAfterMarginCollapsing(layoutBox, verticalMargin.before()));
+        displayBox.setTop(adjustedVerticalPositionAfterMarginCollapsing(layoutBox, verticalMargin));
 
-    displayBox.setContentBoxHeight(heightAndMargin.height);
-    displayBox.setVerticalMargin(verticalMargin);
     // Adjust the previous sibling's margin bottom now that this box's vertical margin is computed.
     if (MarginCollapse::marginBeforeCollapsesWithPreviousSiblingMarginAfter(layoutState, layoutBox))
         MarginCollapse::updateCollapsedMarginAfter(layoutState, *layoutBox.previousInFlowSibling(), verticalMargin);
+
+    displayBox.setContentBoxHeight(heightAndMargin.height);
+    displayBox.setVerticalMargin(verticalMargin);
 }
 
 FormattingContext::InstrinsicWidthConstraints BlockFormattingContext::instrinsicWidthConstraints() const
@@ -453,8 +468,9 @@ FormattingContext::InstrinsicWidthConstraints BlockFormattingContext::instrinsic
     return instrinsicWidthConstraints;
 }
 
-LayoutUnit BlockFormattingContext::adjustedVerticalPositionAfterMarginCollapsing(const Box& layoutBox, LayoutUnit marginBefore) const
+LayoutUnit BlockFormattingContext::adjustedVerticalPositionAfterMarginCollapsing(const Box& layoutBox, const UsedVerticalMargin& verticalMargin) const
 {
+    ASSERT(!layoutBox.isOutOfFlowPositioned());
     // Now that we've computed the final margin before, let's shift the box's vertical position.
     // 1. Check if the margin before collapses with the previous box's margin after. if not -> return previous box's bottom inlcuding margin after + marginBefore
     // 2. Check if the previous box's margins collapse through. If not -> return previous box' bottom excluding margin after + marginBefore (they are supposed to be equal)
@@ -467,26 +483,45 @@ LayoutUnit BlockFormattingContext::adjustedVerticalPositionAfterMarginCollapsing
         auto& previousInFlowSibling = *currentLayoutBox->previousInFlowSibling();
         if (!MarginCollapse::marginBeforeCollapsesWithPreviousSiblingMarginAfter(layoutState, *currentLayoutBox)) {
             auto& previousDisplayBox = layoutState.displayBoxForLayoutBox(previousInFlowSibling);
-            return previousDisplayBox.rectWithMargin().bottom() + marginBefore;
+            return previousDisplayBox.rectWithMargin().bottom() + verticalMargin.before();
         }
 
         if (!MarginCollapse::marginsCollapseThrough(layoutState, previousInFlowSibling)) {
             auto& previousDisplayBox = layoutState.displayBoxForLayoutBox(previousInFlowSibling);
-            return previousDisplayBox.bottom() + marginBefore;
+            return previousDisplayBox.bottom() + verticalMargin.before();
         }
         currentLayoutBox = &previousInFlowSibling;
     }
 
     auto& containingBlock = *layoutBox.containingBlock();
     auto containingBlockContentBoxTop = layoutState.displayBoxForLayoutBox(containingBlock).contentBoxTop();
-    // If we reached the parent through collapsed sibling(s), this box (may) collapses the before margin indirectly with the parent.  
-    auto* collapsingCandidate = layoutBox.previousInFlowSibling() ? containingBlock.firstInFlowChild() : &layoutBox;
-    auto marginBeforeCollapsesWithParent = MarginCollapse::marginBeforeCollapsesWithParentMarginBefore(layoutState, *collapsingCandidate) 
-        || MarginCollapse::marginBeforeCollapsesWithParentMarginAfter(layoutState, *collapsingCandidate);
-    if (marginBeforeCollapsesWithParent)
+    // Adjust vertical position depending whether this box directly or indirectly adjoins with its parent.
+    auto directlyAdjoinsParent = !layoutBox.previousInFlowSibling();
+    if (directlyAdjoinsParent) {
+        // If the top and bottom margins of a box are adjoining, then it is possible for margins to collapse through it.
+        // In this case, the position of the element depends on its relationship with the other elements whose margins are being collapsed.
+        if (verticalMargin.collapsedValues().isCollapsedThrough) {
+            // If the element's margins are collapsed with its parent's top margin, the top border edge of the box is defined to be the same as the parent's.
+            if (MarginCollapse::marginBeforeCollapsesWithParentMarginBefore(layoutState, layoutBox))
+                return containingBlockContentBoxTop;
+            // Otherwise, either the element's parent is not taking part in the margin collapsing, or only the parent's bottom margin is involved.
+            // The position of the element's top border edge is the same as it would have been if the element had a non-zero bottom border.
+            auto beforeMarginWithBottomBorder = MarginCollapse::marginBeforeIgnoringCollapsingThrough(layoutState, layoutBox, verticalMargin.nonCollapsedValues());
+            return containingBlockContentBoxTop + beforeMarginWithBottomBorder;
+        }
+        // Non-collapsed through box vertical position depending whether the margin collapses.
+        if (MarginCollapse::marginBeforeCollapsesWithParentMarginBefore(layoutState, layoutBox))
+            return containingBlockContentBoxTop;
+
+        return containingBlockContentBoxTop + verticalMargin.before();
+    }
+    // At this point this box indirectly (via collapsed through previous in-flow siblings) adjoins the parent. Let's check if it margin collapses with the parent.
+    ASSERT(containingBlock.firstInFlowChild());
+    ASSERT(containingBlock.firstInFlowChild() != &layoutBox);
+    if (MarginCollapse::marginBeforeCollapsesWithParentMarginBefore(layoutState, *containingBlock.firstInFlowChild()))
         return containingBlockContentBoxTop;
 
-    return containingBlockContentBoxTop + marginBefore;
+    return containingBlockContentBoxTop + verticalMargin.before();
 }
 
 }
index 500d064..a2cfdad 100644 (file)
@@ -69,7 +69,7 @@ private:
     void precomputeVerticalPositionForFormattingRootIfNeeded(const Box&) const;
 
     InstrinsicWidthConstraints instrinsicWidthConstraints() const override;
-    LayoutUnit adjustedVerticalPositionAfterMarginCollapsing(const Box&, LayoutUnit marginBefore) const;
+    LayoutUnit adjustedVerticalPositionAfterMarginCollapsing(const Box&, const UsedVerticalMargin&) const;
 
     // This class implements positioning and sizing for boxes participating in a block formatting context.
     class Geometry : public FormattingContext::Geometry {
@@ -95,6 +95,7 @@ private:
         static UsedVerticalMargin::CollapsedValues collapsedVerticalValues(const LayoutState&, const Box&, const UsedVerticalMargin::NonCollapsedValues&);
 
         static EstimatedMarginBefore estimatedMarginBefore(const LayoutState&, const Box&);
+        static LayoutUnit marginBeforeIgnoringCollapsingThrough(const LayoutState&, const Box&, const UsedVerticalMargin::NonCollapsedValues&);
         static void updateCollapsedMarginAfter(const LayoutState&, const Box&, const UsedVerticalMargin& nextSiblingVerticalMargin);
 
         static bool marginBeforeCollapsesWithParentMarginBefore(const LayoutState&, const Box&);
index f9ab127..08de2a0 100644 (file)
@@ -593,13 +593,21 @@ EstimatedMarginBefore BlockFormattingContext::MarginCollapse::estimatedMarginBef
     ASSERT(!layoutBox.establishesBlockFormattingContext());
     
     auto computedVerticalMargin = Geometry::computedVerticalMargin(layoutState, layoutBox);
-    auto nonCollapsedMargin = UsedVerticalMargin::NonCollapsedValues { computedVerticalMargin.before.valueOr(0), computedVerticalMargin.after.valueOr(0) }; 
+    auto nonCollapsedMargin = UsedVerticalMargin::NonCollapsedValues { computedVerticalMargin.before.valueOr(0), computedVerticalMargin.after.valueOr(0) };
+    auto marginsCollapseThrough = MarginCollapse::marginsCollapseThrough(layoutState, layoutBox);
+    auto positiveNegativeMarginBefore = MarginCollapse::positiveNegativeMarginBefore(layoutState, layoutBox, nonCollapsedMargin);
 
-    if (!marginsCollapseThrough(layoutState, layoutBox))
-        return { marginValue(positiveNegativeMarginBefore(layoutState, layoutBox, nonCollapsedMargin)).valueOr(0) };
-    
-    return { marginValue(computedPositiveAndNegativeMargin(positiveNegativeMarginBefore(layoutState, layoutBox, nonCollapsedMargin),
-        positiveNegativeMarginAfter(layoutState, layoutBox, nonCollapsedMargin))).valueOr(0) };
+    auto collapsedMarginBefore = marginValue(!marginsCollapseThrough ? positiveNegativeMarginBefore
+        : computedPositiveAndNegativeMargin(positiveNegativeMarginBefore, positiveNegativeMarginAfter(layoutState, layoutBox, nonCollapsedMargin)));
+
+    return { nonCollapsedMargin.before, collapsedMarginBefore, marginsCollapseThrough };
+}
+
+LayoutUnit BlockFormattingContext::MarginCollapse::marginBeforeIgnoringCollapsingThrough(const LayoutState& layoutState, const Box& layoutBox, const UsedVerticalMargin::NonCollapsedValues& nonCollapsedValues)
+{
+    ASSERT(!layoutBox.isAnonymous());
+    ASSERT(layoutBox.isBlockLevelBox());
+    return marginValue(positiveNegativeMarginBefore(layoutState, layoutBox, nonCollapsedValues)).valueOr(nonCollapsedValues.before);
 }
 
 UsedVerticalMargin::CollapsedValues BlockFormattingContext::MarginCollapse::collapsedVerticalValues(const LayoutState& layoutState, const Box& layoutBox, const UsedVerticalMargin::NonCollapsedValues& nonCollapsedValues)
index 2e3a51c..4f8ca3e 100644 (file)
@@ -1,3 +1,12 @@
+2019-01-11  Zalan Bujtas  <zalan@apple.com>
+
+        [LFC][BFC][MarginCollapsing] Adjust vertical position when box margin collapses through.
+        https://bugs.webkit.org/show_bug.cgi?id=193346
+
+        Reviewed by Antti Koivisto.
+
+        * LayoutReloaded/misc/LFC-passing-tests.txt:
+
 2019-01-10  Ryan Haddad  <ryanhaddad@apple.com>
 
         Unreviewed, fix typo that breaks dashboard link.
index e3c8518..d7d55a6 100644 (file)
@@ -64,6 +64,7 @@ fast/block/block-only/margin-propagation-simple-content-height.html
 fast/block/block-only/margin-sibling-collapse-propagated.html
 fast/block/block-only/margin-simple.html
 fast/block/block-only/collapsed-through-siblings.html
+fast/block/block-only/collapsed-through-with-parent.html
 fast/block/block-only/min-max-height-percentage.html
 fast/block/block-only/negative-margin-simple.html
 fast/block/block-only/padding-nested.html