Attachment 358876 Details for Bug 193346 – Patch

[patch] Patch

bug-193346-20190110215341.patch (text/plain), 18.09 KB, created by zalan on 2019-01-10 21:53:52 PST

(hide)

Description:

Filename:

MIME Type:

Creator: zalan

Created: 2019-01-10 21:53:52 PST

Size: 18.09 KB

patch

obsolete

>Subversion Revision: 239857
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index f336428be9c596c01b2108d23295ca352c0efa2a..1d0075172cd0764a9ceeff9f34ddb0f85366226f 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,30 @@
>+2019-01-10  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 NOBODY (OOPS!).
>+
>+        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  Jer Noble  <jer.noble@apple.com>
> 
>         <video> elements do not enter 'paused' state when playing to end over AirPlay
>diff --git a/Source/WebCore/layout/MarginTypes.h b/Source/WebCore/layout/MarginTypes.h
>index ac4aa8bc5eedb9e571e1adae0ab3c6cdfbc1075d..67b68ab074ac6f8b29a405083e8a079467869249 100644
>--- a/Source/WebCore/layout/MarginTypes.h
>+++ b/Source/WebCore/layout/MarginTypes.h
>@@ -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 {
>diff --git a/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp b/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp
>index ecd8f87437b4804e9e407dbfdde1b19bc16e5d8f..eab7d627eb7a3709006db934449dfe5fa9df49f3 100644
>--- a/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp
>+++ b/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp
>@@ -191,7 +191,8 @@ void BlockFormattingContext::computeEstimatedMarginBefore(const Box& layoutBox)
>     blockFormattingState().setHasEstimatedMarginBefore(layoutBox);
> 
>     auto& displayBox = layoutState.displayBoxForLayoutBox(layoutBox);
>-    displayBox.setTop(adjustedVerticalPositionAfterMarginCollapsing(layoutBox, estimatedMarginBefore.usedValue));
>+    auto verticalMargin = UsedVerticalMargin { { estimatedMarginBefore.nonCollapsedValue, { } }, { estimatedMarginBefore.collapsedValue, { }, estimatedMarginBefore.isCollapsedThrough } };
>+    displayBox.setTop(adjustedVerticalPositionAfterMarginCollapsing(layoutBox, verticalMargin));
> #if !ASSERT_DISABLED
>     displayBox.setEstimatedMarginBefore(estimatedMarginBefore);
> #endif
>@@ -375,18 +376,27 @@ void BlockFormattingContext::computeHeightAndMargin(const Box& layoutBox) const
>     auto collapsedMargin = MarginCollapse::collapsedVerticalValues(layoutState, layoutBox, heightAndMargin.nonCollapsedMargin);
>     auto verticalMargin = UsedVerticalMargin { heightAndMargin.nonCollapsedMargin, collapsedMargin };
>     auto& displayBox = layoutState.displayBoxForLayoutBox(layoutBox);
>+    
>+    // In flow/floating boxes need vertical adjustment using the collapsed margin values.
>+    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.setContentBoxHeight(heightAndMargin.height);
>-    displayBox.setVerticalMargin(verticalMargin);
>+        displayBox.setTop(adjustedVerticalPositionAfterMarginCollapsing(layoutBox, 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 +463,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 +478,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;
>+        }
>+        
>+        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 margin collapsing is on 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();
> }
> 
> }
>diff --git a/Source/WebCore/layout/blockformatting/BlockFormattingContext.h b/Source/WebCore/layout/blockformatting/BlockFormattingContext.h
>index 500d0649c25e237ec00de72fbdeb92b05e758ed5..a2cfdad2017fae77103e8081c311e6f556608e10 100644
>--- a/Source/WebCore/layout/blockformatting/BlockFormattingContext.h
>+++ b/Source/WebCore/layout/blockformatting/BlockFormattingContext.h
>@@ -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&);
>diff --git a/Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp b/Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp
>index f9ab127f9aa48c20d0a7f4fa1e7b4c445ad44c1f..a3b41c2ee3a862bedfc7d0c6738b5f65cc48bd29 100644
>--- a/Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp
>+++ b/Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp
>@@ -593,13 +593,24 @@ 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);
>+    Optional<LayoutUnit> collapsedMarginBefore;
> 
>-    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) };
>+    if (marginsCollapseThrough) {
>+        collapsedMarginBefore = marginValue(computedPositiveAndNegativeMargin(positiveNegativeMarginBefore(layoutState, layoutBox, nonCollapsedMargin),
>+            positiveNegativeMarginAfter(layoutState, layoutBox, nonCollapsedMargin)));
>+    } else
>+        collapsedMarginBefore = marginValue(positiveNegativeMarginBefore(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)
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index dcadc79259a1ba96594f5321aa551d687ab4ce56..a5f9b248b0976268cee70d49abe09130faf23e6d 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,12 @@
>+2019-01-10  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 NOBODY (OOPS!).
>+
>+        * LayoutReloaded/misc/LFC-passing-tests.txt:
>+
> 2019-01-10  Joseph Pecoraro  <pecoraro@apple.com>
> 
>         MiniBrowser should be able to navigate to about:blank
>diff --git a/Tools/LayoutReloaded/misc/LFC-passing-tests.txt b/Tools/LayoutReloaded/misc/LFC-passing-tests.txt
>index e3c8518416d32bde445b4d2ab9ecd97016906d2b..d7d55a646428beb28fb214787e7b4ca4c12fe842 100644
>--- a/Tools/LayoutReloaded/misc/LFC-passing-tests.txt
>+++ b/Tools/LayoutReloaded/misc/LFC-passing-tests.txt
>@@ -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
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 9dc92ef5965983287f65fe63a67c2993036e730a..a7190ebc875d5494af1b128ce03a1697b99b7ece 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,13 @@
>+2019-01-10  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 NOBODY (OOPS!).
>+
>+        * fast/block/block-only/collapsed-through-with-parent-expected.txt: Added.
>+        * fast/block/block-only/collapsed-through-with-parent.html: Added.
>+
> 2019-01-10  Justin Fan  <justin_fan@apple.com>
> 
>         [WebGPU] WebGPUBindGroup and device::createBindGroup prototype
>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
>index 0000000000000000000000000000000000000000..61050a58e40aa6b46ba6c4e2d71e39eeced08842
>--- /dev/null
>+++ b/LayoutTests/fast/block/block-only/collapsed-through-with-parent-expected.txt
>@@ -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
>index 0000000000000000000000000000000000000000..0a5d4d89133de7dd170266ff79b3af359463c06a
>--- /dev/null
>+++ b/LayoutTests/fast/block/block-only/collapsed-through-with-parent.html
>@@ -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>

Actions: View | Formatted Diff | Diff

Attachments on bug 193346: 358876 | 358895