[margin-trim][block-layout] Content at block-start edge should have t…

…rimmed margins reflected in computed style https://bugs.webkit.org/show_bug.cgi?id=253606 rdar://106452955 Reviewed by Alan Baradlay. When we trim margins at the block-start edge, we can indicate that the renderer has a trimmed margin by replacing the calls to setMarginBefore/After with setTrimmedMarginForChild. This will perform the same behavior of trimming the renderer's margins by updating its m_marginBox and also setting the margin-trim rare data bit to indicate that the margin has been trimmed. In order to apply this behavior for nested block content that is at the block-start edge of the outer block container we need to keep track of some additional state. If a block container has block-start margin-trim set then it will push some new state onto m_blockStartTrimming to indicate that block-start trimming should occur and propagate this information to its children in order to determine whether they should trim. This new structure acts as a stack that will help nested block containers determine if they should trim the margins at their block-start edge based upon its containing block's trimming state and its own MarginInfo state. A block container will push new state onto this stack in the following scenarios: - If a block-container has block-start margin trim set, then it will push some new state onto the stack (true) indicating trimming should occur - A nested block container will check to see if its containing block has trimming state set by checking the value at the top of the stack along with whether or not its margins can collapse through to its containing block - If the containing block has trimming state and the nested child's margins can collapse through to the top, then the nested child will push its own state onto the stack to use later on as it lays out its children. The state will be the same as its containing block's so that the nested block container will only trim if it is at the block-start edge of the containing block that has margin-trim set. - If the containing block has trimming state and the nested child's margins *cannot* collapse through to the top, then it will push state onto the stack (false) to indicate that it should not perform any sort of trimming. This will also indicate to nested block containers that they should also not trim. <div id="container" style="margin-trim: block"> <div id="outer" style="margin-block-start: 10px; border: 1px solid black; width: 50px; height: 50px; background-color: green;"> <div id="inner" style="margin-block-start: 10px; border: 1px solid black; width: 50px; height: 50px; background-color: blue;"></div> </div> </div> Here "container," will push some state onto the margin trimming stack to indicate that it should trim margins at the block-start edge. When "outer," goes through layout, it will see that its containing block had set some trimming state so it will do the same. Since it has a border that means its children's margins cannot collapse through, so it should not trim those margins and set the state appropriately. "Inner," will see that its containing block had set some margin-trim state and will need to do the same. Since its children's margins could collapse through, it will use the same state as its containing block. However, since the containing block did not perform trimming, it will also not trim when it attempts to use this information. In ComputedStyleExtractor, we will check to see if the renderer has its top margin trimmed by checking to see if the rare data bit was set during layout by using RenderBox::hasTrimmedMargin. * LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/computed-margin-values/block-container-block-start-child-with-border-expected.txt: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/computed-margin-values/block-container-block-start-child-with-border.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/computed-margin-values/block-container-block-start-expected.txt: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/computed-margin-values/block-container-block-start-self-collapsing-nested-expected.txt: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/computed-margin-values/block-container-block-start-self-collapsing-nested.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/computed-margin-values/block-container-block-start.html: Added. * Source/WebCore/css/ComputedStyleExtractor.cpp: (WebCore::rendererCanHaveTrimmedMargin): (WebCore::isLayoutDependent): * Source/WebCore/rendering/RenderBlockFlow.cpp: (WebCore::RenderBlockFlow::layoutBlockChildren): (WebCore::RenderBlockFlow::layoutBlockChild): (WebCore::RenderBlockFlow::collapseMarginsWithChildInfo): * Source/WebCore/rendering/RenderBox.cpp: (WebCore::RenderBox::physicalToFlowRelativeDirectionMapping const): (WebCore::RenderBox::hasTrimmedMargin const): * Source/WebCore/rendering/RenderBox.h: (WebCore::RenderBox::isBlockLevelBox const): * Source/WebCore/rendering/RenderLayoutState.cpp: (WebCore::RenderLayoutState::RenderLayoutState): * Source/WebCore/rendering/RenderLayoutState.h: (WebCore::RenderLayoutState::RenderLayoutState): (WebCore::RenderLayoutState::pushBlockStartTrimming): (WebCore::RenderLayoutState::peekBlockStartTrimming): (WebCore::RenderLayoutState::popBlockStartTrimming): Canonical link: https://commits.webkit.org/263412@main
WebKit · Apr 26, 2023 · 99e30b0 · 99e30b0
1 parent bf6ce95
commit 99e30b0
Show file tree

Hide file tree

Showing 11 changed files with 180 additions and 14 deletions.
diff --git a/...in-trim/computed-margin-values/block-container-block-start-child-with-border-expected.txt b/...in-trim/computed-margin-values/block-container-block-start-child-with-border-expected.txt
@@ -0,0 +1,6 @@
+
+PASS item 1
+PASS item 2
+PASS item 3
+PASS item 4
+
diff --git a/...box/margin-trim/computed-margin-values/block-container-block-start-child-with-border.html b/...box/margin-trim/computed-margin-values/block-container-block-start-child-with-border.html
@@ -0,0 +1,43 @@
+<!DOCTYPE html>
+<html>
+<head>
+<link rel="author" title="Sammy Gill" href="mailto:sammy.gill@apple.com">
+<link rel="help" href="https://drafts.csswg.org/css-box-4/#margin-trim-block">
+<meta name="assert" content="item with border should prevent trimming within itself">
+<style>
+container {
+    display: block;
+    margin-trim: block;
+    border: 1px solid black;
+    margin-block-start: 10px;
+}
+item {
+    display: block;
+    inline-size: 50px;
+    block-size: 50px;
+    background-color: green;
+}
+.collapsed {
+    margin-block: 50px;
+    block-size: 0px
+}
+.with-border-and-margin {
+    border: 1px solid black;
+    margin-block-start: 30px;
+}
+</style>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="/resources/check-layout-th.js"></script>
+</head>
+<body onload="checkLayout('item')">
+    <container>
+        <item data-expected-margin-top="0" class="collapsed">
+            <item data-expected-margin-top="0" class="collapsed"></item>
+        </item>
+        <item class="with-border-and-margin" data-expected-margin-top="0">
+            <item class="with-border-and-margin" data-expected-margin-top="30"></item>
+        </item>
+    </container>
+</body>
+</html>
diff --git a/...s/css/css-box/margin-trim/computed-margin-values/block-container-block-start-expected.txt b/...s/css/css-box/margin-trim/computed-margin-values/block-container-block-start-expected.txt
@@ -0,0 +1,3 @@
+
+PASS item 1
+
diff --git a/...im/computed-margin-values/block-container-block-start-self-collapsing-nested-expected.txt b/...im/computed-margin-values/block-container-block-start-self-collapsing-nested-expected.txt
@@ -0,0 +1,9 @@
+
+PASS container, item 1
+PASS container, item 2
+PASS container, item 3
+PASS container, item 4
+PASS container, item 5
+PASS container, item 6
+PASS container, item 7
+
diff --git a/...argin-trim/computed-margin-values/block-container-block-start-self-collapsing-nested.html b/...argin-trim/computed-margin-values/block-container-block-start-self-collapsing-nested.html
@@ -0,0 +1,41 @@
+<!DOCTYPE html>
+<html>
+<head>
+<link rel="author" title="Sammy Gill" href="mailto:sammy.gill@apple.com">
+<link rel="help" href="https://drafts.csswg.org/css-box-4/#margin-trim-block">
+<meta name="assert" content="self-collapsing items at block start should have margins trimmed along with first non self-collapsing child block-start margins">
+<style>
+container {
+    display: block;
+    margin-trim: block;
+    border: 1px solid black;
+    margin-block-start: 10px;
+}
+item {
+    display: block;
+    inline-size: 50px;
+    background-color: green;
+}
+.collapsed {
+    margin-block-start: 50px;
+    block-size: 0px
+}
+</style>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="/resources/check-layout-th.js"></script>
+</head>
+<body onload=" checkLayout('container, item');">
+    <container data-expected-margin-top="10">
+        <item data-expected-margin-top="0" class="collapsed">
+            <item data-expected-margin-top="0" class="collapsed"></item>
+        </item>
+        <item data-expected-margin-top="0" style="margin-block: 40px">
+            <item data-expected-margin-top="0" data-expected-margin-bottom="0" class="collapsed"></item>
+            <item data-expected-margin-top="0" style="margin-top: 30px;">
+                <item data-expected-margin-top="0" style="margin-block-start: 100px; height: 50px;"></item>
+            </item>
+        </item>
+    </container>
+</body>
+</html>
diff --git a/...orm-tests/css/css-box/margin-trim/computed-margin-values/block-container-block-start.html b/...orm-tests/css/css-box/margin-trim/computed-margin-values/block-container-block-start.html
@@ -0,0 +1,34 @@
+<!DOCTYPE html>
+<html>
+<head>
+<link rel="author" title="Sammy Gill" href="mailto:sammy.gill@apple.com">
+<link rel="help" href="https://drafts.csswg.org/css-box-4/#margin-trim-block">
+<meta name="assert" content="block-start margin of item should be trimmed">
+<style>
+container {
+    display: block;
+    margin-trim: block;
+    border: 1px solid black;
+    margin-block-start: 10px;
+}
+item {
+    display: block;
+    inline-size: 50px;
+    block-size: 50px;
+    background-color: green;
+}
+.collapsed {
+    margin-block: 50px;
+    block-size: 0px
+}
+</style>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="/resources/check-layout-th.js"></script>
+</head>
+<body onload="checkLayout('item')">
+    <container>
+        <item style="margin-top: 30px;" data-expected-margin-top="0"></item>
+    </container>
+</body>
+</html>
diff --git a/Source/WebCore/css/ComputedStyleExtractor.cpp b/Source/WebCore/css/ComputedStyleExtractor.cpp
@@ -2304,7 +2304,7 @@ static bool rendererCanHaveTrimmedMargin(const RenderBox& renderer, MarginTrimTy
     // of an ancestor containing block with the property, so we will just return true and let
     // the rest of the logic in RenderBox::hasTrimmedMargin to determine if the rare data bit
     // were set at some point during layout
-    if (containingBlock->isBlockContainer() && containingBlock->isHorizontalWritingMode() && renderer.isBlockLevelBox() && marginTrimType == MarginTrimType::BlockEnd)
+    if (containingBlock->isBlockContainer() && containingBlock->isHorizontalWritingMode() && renderer.isBlockLevelBox())
         return true;
     return false;
 }

diff --git a/Source/WebCore/rendering/RenderBlockFlow.cpp b/Source/WebCore/rendering/RenderBlockFlow.cpp
@@ -790,14 +790,30 @@ void RenderBlockFlow::layoutBlockChildren(bool relayoutChildren, LayoutUnit& max
     LayoutUnit afterEdge = borderAndPaddingAfter() + scrollbarLogicalHeight();
 
     setLogicalHeight(beforeEdge);
-
+    auto* layoutState = view().frameView().layoutContext().layoutState(); 
     // Lay out our hypothetical grid line as though it occurs at the top of the block.
-    if (view().frameView().layoutContext().layoutState()->lineGrid() == this)
+    if (layoutState->lineGrid() == this)
         layoutLineGridBox();
 
     // The margin struct caches all our current margin collapsing state.
     MarginInfo marginInfo(*this, beforeEdge, afterEdge);
 
+    auto hasMarginTrimState = false;
+    auto updateMarginTrimStateIfNeeded = [&] {
+        auto containingBlockTrimmingState = layoutState->blockStartTrimming();
+        if (style().marginTrim().contains(MarginTrimType::BlockStart))
+            layoutState->pushBlockStartTrimming(true);
+        else if (!marginInfo.canCollapseMarginBeforeWithChildren() && containingBlockTrimmingState)
+            layoutState->pushBlockStartTrimming(false);
+        else if (marginInfo.canCollapseMarginBeforeWithChildren() && containingBlockTrimmingState)
+            layoutState->pushBlockStartTrimming(containingBlockTrimmingState.value());
+        else
+            return;
+        hasMarginTrimState = true;
+    };
+
+    updateMarginTrimStateIfNeeded();
+
     // Fieldsets need to find their legend and position it inside the border of the object.
     // The legend then gets skipped during normal layout. The same is true for ruby text.
     // It doesn't get included in the normal layout process but is instead skipped.
@@ -855,6 +871,8 @@ void RenderBlockFlow::layoutBlockChildren(bool relayoutChildren, LayoutUnit& max
     // Now do the handling of the bottom of the block, adding in our bottom border/padding and
     // determining the correct collapsed bottom margin information.
     handleAfterSideOfBlock(beforeEdge, afterEdge, marginInfo);
+    if (hasMarginTrimState)
+        layoutState->popBlockStartTrimming();
 }
 
 
@@ -1056,9 +1074,14 @@ void RenderBlockFlow::layoutBlockChild(RenderBox& child, MarginInfo& marginInfo,
 
     // We are no longer at the top of the block if we encounter a non-empty child.  
     // This has to be done after checking for clear, so that margins can be reset if a clear occurred.
-    if (marginInfo.atBeforeSideOfBlock() && !child.isSelfCollapsingBlock())
+    if (marginInfo.atBeforeSideOfBlock() && !child.isSelfCollapsingBlock()) {
         marginInfo.setAtBeforeSideOfBlock(false);
 
+        if (auto* layoutState = frame().view()->layoutContext().layoutState(); layoutState && layoutState->blockStartTrimming()) {
+            layoutState->popBlockStartTrimming();
+            layoutState->pushBlockStartTrimming(false);
+        }
+    }
     // Now place the child in the correct left position
     determineLogicalLeftPositionForChild(child, ApplyLayoutDelta);
 
@@ -1326,18 +1349,20 @@ LayoutUnit RenderBlockFlow::collapseMarginsWithChildInfo(RenderBox* child, Rende
         auto childBlockFlow = dynamicDowncast<RenderBlockFlow>(child);
         if (childBlockFlow)
             childBlockFlow->setMaxMarginBeforeValues(0_lu, 0_lu);
-        child->setMarginBefore(0_lu, &style());
+        setTrimmedMarginForChild(*child, MarginTrimType::BlockStart);
 
         // The margin after for a self collapsing child should also be trimmed so it does not 
         // influence the margins of the first non collapsing child
         if (childIsSelfCollapsing) {
             if (childBlockFlow)
                 childBlockFlow->setMaxMarginAfterValues(0_lu, 0_lu);
-            child->setMarginAfter(0_lu, &style());
+            setTrimmedMarginForChild(*child, MarginTrimType::BlockEnd);
         }
     };
-    if (marginInfo.atBeforeSideOfBlock() && style().marginTrim().contains(MarginTrimType::BlockStart))
+    if (frame().view()->layoutContext().layoutState()->blockStartTrimming().value_or(false)) {
+        ASSERT(marginInfo.atBeforeSideOfBlock());
         trimChildBlockMargins();
+    }
 
     // Get the four margin values for the child and cache them.
     MarginValues childMargins = child ? marginValuesForChild(*child) : MarginValues(0, 0, 0, 0);

diff --git a/Source/WebCore/rendering/RenderBox.cpp b/Source/WebCore/rendering/RenderBox.cpp
@@ -1404,13 +1404,6 @@ bool RenderBox::hasTrimmedMargin(std::optional<MarginTrimType> marginTrimType) c
 {
     if (!isInFlow())
         return false;
-#if ASSERT_ENABLED
-    // We should assert here if this function is called with a layout system and
-    // MarginTrimType combination that is not supported yet (i.e. the layout system
-    // does not set the margin trim rare data bit for that margin)
-    if (!isFlexItem() && !isGridItem() && isBlockLevelBox())
-        ASSERT(!marginTrimType || marginTrimType.value() == MarginTrimType::BlockEnd);
-#endif
     if (!hasRareData())
         return false;
     return marginTrimType ? (rareData().trimmedMargins() & static_cast<unsigned>(*marginTrimType)) : rareData().trimmedMargins();

diff --git a/Source/WebCore/rendering/RenderLayoutState.cpp b/Source/WebCore/rendering/RenderLayoutState.cpp
@@ -42,6 +42,7 @@ RenderLayoutState::RenderLayoutState(RenderElement& renderer, IsPaginated isPagi
 #if ASSERT_ENABLED
     , m_layoutDeltaXSaturated(false)
     , m_layoutDeltaYSaturated(false)
+    , m_blockStartTrimming(Vector<bool>(0))
     , m_renderer(&renderer)
 #endif
 {
@@ -70,6 +71,7 @@ RenderLayoutState::RenderLayoutState(const LocalFrameViewLayoutContext::LayoutSt
     , m_layoutDeltaXSaturated(false)
     , m_layoutDeltaYSaturated(false)
 #endif
+    , m_blockStartTrimming(Vector<bool>(0))
     , m_lineClamp(lineClamp)
     , m_leadingTrim(leadingTrim)
 #if ASSERT_ENABLED

diff --git a/Source/WebCore/rendering/RenderLayoutState.h b/Source/WebCore/rendering/RenderLayoutState.h
@@ -60,6 +60,7 @@ class RenderLayoutState {
         , m_layoutDeltaXSaturated(false)
         , m_layoutDeltaYSaturated(false)
 #endif
+        , m_blockStartTrimming(Vector<bool>(0))
     {
     }
     RenderLayoutState(const LocalFrameViewLayoutContext::LayoutStateStack&, RenderBox&, const LayoutSize& offset, LayoutUnit pageHeight, bool pageHeightChanged, std::optional<LineClamp>, std::optional<LeadingTrim>);
@@ -111,6 +112,13 @@ class RenderLayoutState {
     void addLeadingTrimEnd(const RenderBlockFlow& targetInlineFormattingContext);
     void resetLeadingTrim() { m_leadingTrim = { }; }
 
+    void pushBlockStartTrimming(bool blockStartTrimming) { m_blockStartTrimming.append(blockStartTrimming); }
+    std::optional<bool> blockStartTrimming() const { return m_blockStartTrimming.isEmpty() ? std::nullopt : std::optional(m_blockStartTrimming.last()); }
+    void popBlockStartTrimming() 
+    {
+        m_blockStartTrimming.removeLast(); 
+    }
+
 private:
     void computeOffsets(const RenderLayoutState& ancestor, RenderBox&, LayoutSize offset);
     void computeClipRect(const RenderLayoutState& ancestor, RenderBox&);
@@ -129,6 +137,8 @@ class RenderLayoutState {
     bool m_layoutDeltaXSaturated : 1;
     bool m_layoutDeltaYSaturated : 1;
 #endif
+    Vector<bool> m_blockStartTrimming;
+
     // The current line grid that we're snapping to and the offset of the start of the grid.
     WeakPtr<RenderBlockFlow> m_lineGrid;