[margin-trim] Trimmed block-end margins for grid items in horizontal …

…writing-mode should be refected in computed style. https://bugs.webkit.org/show_bug.cgi?id=253717 rdar://106559562 Reviewed by Alan Baradlay. When a grid has block-end margin-trim specified, then it should trim the block-end margins of any of its items on the last row. These margins that end up getting trimmed should be reflected in the computed style of that margin as having being trimmed (value of 0). This can be done by setting the rare data margin-trim bit for the "bottm," margin of the renderer during layout. Since the trimming for the block-end margins of grid item occurs in RenderBox::constrainBlockMarginInAvailableSpaceOrTrim, then that is the appropriate place to set the rare data bit for the trimmed margin. The renderer can set this bit by calling markMarginAsTrimmed and passing in the margin that has been trimmed. ComputedStyleExtractor should then be able to use RenderBox::hasTrimmedMargin, which returns true if the rare data bit is set for the passed in margin, to determine if the "bottom," margin has been trimmed during layout. * LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/computed-margin-values/grid-block-end-column-auto-flow-expected.txt: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/computed-margin-values/grid-block-end-column-auto-flow.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/computed-margin-values/grid-block-end-expected.txt: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/computed-margin-values/grid-block-end-item-spans-multiple-rows-expected.txt: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/computed-margin-values/grid-block-end-item-spans-multiple-rows.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/computed-margin-values/grid-block-end.html: Added. * Source/WebCore/css/ComputedStyleExtractor.cpp: (WebCore::rendererCanHaveTrimmedMargin): (WebCore::isLayoutDependent): (WebCore::ComputedStyleExtractor::valueForPropertyInStyle): * Source/WebCore/rendering/RenderBox.cpp: (WebCore::RenderBox::hasTrimmedMargin const): (WebCore::RenderBox::constrainBlockMarginInAvailableSpaceOrTrim const): Canonical link: https://commits.webkit.org/263002@main
WebKit · Apr 15, 2023 · 7e82880 · 7e82880
1 parent 13134c6
commit 7e82880
Show file tree

Hide file tree

Showing 8 changed files with 186 additions and 6 deletions.
diff --git a/...s/css-box/margin-trim/computed-margin-values/grid-block-end-column-auto-flow-expected.txt b/...s/css-box/margin-trim/computed-margin-values/grid-block-end-column-auto-flow-expected.txt
@@ -0,0 +1,7 @@
+
+PASS grid > item 1
+PASS grid > item 2
+PASS grid > item 3
+PASS grid > item 4
+PASS grid > item 5
+
diff --git a/...tests/css/css-box/margin-trim/computed-margin-values/grid-block-end-column-auto-flow.html b/...tests/css/css-box/margin-trim/computed-margin-values/grid-block-end-column-auto-flow.html
@@ -0,0 +1,60 @@
+<!DOCTYPE html>
+<html>
+<head>
+<link rel="author" href="mailto:sammy.gill@apple.com">
+<link rel="help" href="https://w3c.github.io/csswg-drafts/css-box-4/#margin-trim-grid">
+<meta name="assert" content="Trimmed block-end margins for grid items should be reflected in computed style">
+</head>
+<style>
+grid {
+    display: grid;
+    width: min-content;
+    grid-auto-flow: column;
+    border: 1px solid black;
+    grid-template-rows: auto auto auto;
+    margin-trim: block-end;
+}
+item {
+    display: block;
+    width: 50px;
+    height: 50px;
+    margin-block-end: 10px;
+}
+.span-four {
+    grid-row: span 4;
+}
+item:nth-child(1) {
+    grid-row: 1;
+    background-color: green;
+}
+item:nth-child(2) {
+    grid-row: 2;
+    background-color: blue;
+}
+item:nth-child(3) {
+    grid-row: 3;
+    background-color: purple;
+}
+item:nth-child(4) {
+    background-color:burlywood;
+}
+item:nth-child(5) {
+    background-color: grey;
+    grid-row: 4;
+}
+</style>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="/resources/check-layout-th.js"></script>
+<body onload="checkLayout('grid > item')">
+    <div id="target">
+    <grid>
+        <item data-expected-margin-bottom="10"></item>
+        <item data-expected-margin-bottom="10"></item>
+        <item data-expected-margin-bottom="10"></item>
+        <item data-expected-margin-bottom="0" class="span-four"></item>
+        <item data-expected-margin-bottom="0"></item>
+    </grid>
+    </div>
+</body>
+</html>
diff --git a/...platform-tests/css/css-box/margin-trim/computed-margin-values/grid-block-end-expected.txt b/...platform-tests/css/css-box/margin-trim/computed-margin-values/grid-block-end-expected.txt
@@ -0,0 +1,6 @@
+
+PASS grid > item 1
+PASS grid > item 2
+PASS grid > item 3
+PASS grid > item 4
+
diff --git a/...x/margin-trim/computed-margin-values/grid-block-end-item-spans-multiple-rows-expected.txt b/...x/margin-trim/computed-margin-values/grid-block-end-item-spans-multiple-rows-expected.txt
@@ -0,0 +1,4 @@
+
+PASS grid > item 1
+PASS grid > item 2
+
diff --git a/...s/css-box/margin-trim/computed-margin-values/grid-block-end-item-spans-multiple-rows.html b/...s/css-box/margin-trim/computed-margin-values/grid-block-end-item-spans-multiple-rows.html
@@ -0,0 +1,43 @@
+<!DOCTYPE html>
+<html>
+<head>
+<title></title>
+<link rel="author" href="mailto:sammy.gill@apple.com">
+<link rel="help" href="https://w3c.github.io/csswg-drafts/css-box-4/#margin-trim-grid">
+<meta name="assert" content="item that spans into last row should have block-end margin trimmed">
+</head>
+<style>
+grid {
+    display: grid;
+    border: 1px solid black;
+    grid-template-columns: auto auto;
+    margin-trim: block-end;
+}
+item {
+    display: block;
+    width: 50px;
+    height: 50px;
+    margin-bottom: 10px;
+}
+.row-two {
+    grid-row: 2;
+    background-color: green;
+}
+.span-two-rows {
+    grid-row: span 2;
+    background-color: blue;
+    height: 90px;
+}
+</style>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="/resources/check-layout-th.js"></script>
+<body onload="checkLayout('grid > item')">
+    <div id="target">
+    <grid>
+        <item data-expected-margin-bottom="0" class="span-two-rows"></item>
+        <item data-expected-margin-bottom="0" class="row-two"></item>
+    </grid>
+    </div>
+</body>
+</html>
diff --git a/...w3c/web-platform-tests/css/css-box/margin-trim/computed-margin-values/grid-block-end.html b/...w3c/web-platform-tests/css/css-box/margin-trim/computed-margin-values/grid-block-end.html
@@ -0,0 +1,56 @@
+<!DOCTYPE html>
+<html>
+<head>
+<title></title>
+<link rel="author" href="mailto:sammy.gill@apple.com">
+<link rel="help" href="https://w3c.github.io/csswg-drafts/css-box-4/#margin-trim-grid">
+<meta name="assert" content="Trimmed block-end margins for grid items should be reflected in computed style">
+</head>
+<style>
+grid {
+    display: grid;
+    width: min-content;
+    border: 1px solid black;
+    grid-template-columns: repeat(4, auto);
+    margin-trim: block-end;
+}
+item {
+    display: block;
+    width: 50px;
+    height: 50px;
+    margin-bottom: 10px;
+}
+.locked-position {
+    grid-row: 2;
+    grid-column: 2;
+    margin-block-end: 50px;
+    background-color: magenta;
+}
+.span-three-columns {
+    grid-column: span 3;
+    background-color: purple;
+}
+.span-five-columns {
+    grid-column: span 5;
+}
+item:nth-child(1) {
+    background-color: green;
+}
+item:nth-child(4) {
+    background-color: blue;
+}
+</style>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="/resources/check-layout-th.js"></script>
+<body onload="checkLayout('grid > item')">
+    <div id="target">
+    <grid>
+        <item class="span-five-columns" data-expected-margin-bottom="10"></item>
+        <item class="locked-position" data-expected-margin-bottom="50"></item>
+        <item class="span-three-columns" data-expected-margin-bottom="10"></item>
+        <item data-expected-margin-bottom="0"></item>
+    </grid>
+    </div>
+</body>
+</html>
diff --git a/Source/WebCore/css/ComputedStyleExtractor.cpp b/Source/WebCore/css/ComputedStyleExtractor.cpp
@@ -2312,8 +2312,11 @@ static bool rendererCanHaveTrimmedMargin(const RenderBox& renderer, std::optiona
             return !containingBlock->style().marginTrim().isEmpty();
         return containingBlock->style().marginTrim().contains(marginTrimType.value());
     }
-    if (containingBlock->isRenderGrid() && (!marginTrimType || marginTrimType.value() == MarginTrimType::BlockStart))
+    if (containingBlock->isRenderGrid() && (!marginTrimType || marginTrimType.value() == MarginTrimType::BlockStart || marginTrimType.value() == MarginTrimType::BlockEnd)) {
+        if (!marginTrimType)
+            return containingBlock->style().marginTrim().containsAny({ MarginTrimType::BlockStart, MarginTrimType::BlockEnd });
         return containingBlock->style().marginTrim().contains(marginTrimType.value());
+    }
     return false;
 }
 
@@ -2354,7 +2357,7 @@ static bool isLayoutDependent(CSSPropertyID propertyID, const RenderStyle* style
     case CSSPropertyMarginRight:
         return paddingOrMarginIsRendererDependent<&RenderStyle::marginRight>(style, renderer) || (isFlexItem(renderer) && rendererCanHaveTrimmedMargin(downcast<RenderBox>(*renderer), MarginTrimType::InlineEnd));
     case CSSPropertyMarginBottom:
-        return paddingOrMarginIsRendererDependent<&RenderStyle::marginBottom>(style, renderer) || (isFlexItem(renderer) && rendererCanHaveTrimmedMargin(downcast<RenderBox>(*renderer), MarginTrimType::BlockEnd));
+        return paddingOrMarginIsRendererDependent<&RenderStyle::marginBottom>(style, renderer) ||  (is<RenderBox>(renderer) && rendererCanHaveTrimmedMargin(downcast<RenderBox>(*renderer), MarginTrimType::BlockEnd));
     case CSSPropertyMarginLeft:
         return paddingOrMarginIsRendererDependent<&RenderStyle::marginLeft>(style, renderer) || (isFlexItem(renderer) && rendererCanHaveTrimmedMargin(downcast<RenderBox>(*renderer), MarginTrimType::InlineStart));
     case CSSPropertyPadding: {
@@ -3309,7 +3312,8 @@ RefPtr<CSSValue> ComputedStyleExtractor::valueForPropertyInStyle(const RenderSty
         return zoomAdjustedPixelValue(value, style);
     }
     case CSSPropertyMarginBottom:
-        if (auto* box = dynamicDowncast<RenderBox>(renderer); box && rendererCanHaveTrimmedMargin(*box, MarginTrimType::BlockEnd) 
+        if (auto* box = dynamicDowncast<RenderBox>(renderer); box 
+            && rendererCanHaveTrimmedMargin(*box, MarginTrimType::BlockEnd) 
             && box->hasTrimmedMargin(PhysicalDirection::Bottom))
             return zoomAdjustedPixelValue(box->marginBottom(), style);
         return zoomAdjustedPaddingOrMarginPixelValue<&RenderStyle::marginBottom, &RenderBoxModelObject::marginBottom>(style, renderer);

diff --git a/Source/WebCore/rendering/RenderBox.cpp b/Source/WebCore/rendering/RenderBox.cpp
@@ -1474,7 +1474,7 @@ bool RenderBox::hasTrimmedMargin(std::optional<MarginTrimType> marginTrimType) c
         return false;
     }
     if (containingBlock && containingBlock->isRenderGrid())
-        ASSERT(!marginTrimType || marginTrimType.value() == MarginTrimType::BlockStart);
+        ASSERT(!marginTrimType || marginTrimType.value() == MarginTrimType::BlockStart || marginTrimType.value() == MarginTrimType::BlockEnd);
 #endif
     if (!hasRareData())
         return false;
@@ -3824,8 +3824,8 @@ LayoutUnit RenderBox::constrainBlockMarginInAvailableSpaceOrTrim(const RenderBox
         // be done at this level within RenderBox. We should be able to leave the 
         // trimming responsibility to each of those contexts and not need to
         // do any of it here (trimming the margin and setting the rare data bit)
-        if (isGridItem() && marginSide == MarginTrimType::BlockStart)
-            const_cast<RenderBox&>(*this).markMarginAsTrimmed(MarginTrimType::BlockStart);
+        if (isGridItem())
+            const_cast<RenderBox&>(*this).markMarginAsTrimmed(marginSide);
         return 0_lu;
     }