Trimmed inline-end margins for flex items in horizontal writing-mode …

…should be reflected in computed style. https://bugs.webkit.org/show_bug.cgi?id=253715 rdar://106559532 Reviewed by Alan Baradlay. When trimming the inline-end margins in RenderFlexibleBox::trimMainAxisMarginEnd and RenderFlexibleBox::trimCrossAxisMarginEnd, instead of trimming the margins there directly, we can replace the trimming with calls to RenderBlock::setTrimmedMarginForChild with MarginTrimType::InlineEnd as an argument. This will both trim the margins by calling setMarginEndForChild and also set the appropriate rare data bit to indicate that the inline-end margin for this box has been trimmed. In horizonal writing modes this bit that is set should refer to margin-right. This can be used after layout has completed to check if a box has a trimmed margin. When ComputedStyleExtractor tries to obtain the "margin-right," value, it will need to check if this margin has been trimmed. This is done by calling RenderBox::hasTrimmedMargin(PhysicalDirection::Right), which will transform this physical direction into a flow relative direction. In horizontal writing mode with LTR direction, the "right," physical direction should correspond to the "inline-end," FlowRelativeDirection. hasTrimmedMargin should then take this new flow relative direction and check to see if the box has this margin marked as trimmed. If this returns true, then this means that the box must have had its margin-right trimmed during layout and ComputedStyleExtractor should consule the box's m_marinBox for the trimmed value. * LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/computed-margin-values/flexbox-column-inline-end-expected.txt: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/computed-margin-values/flexbox-column-inline-end.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/computed-margin-values/flexbox-column-multi-line-inline-end-expected.txt: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/computed-margin-values/flexbox-column-multi-line-inline-end.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/computed-margin-values/flexbox-row-inline-end-expected.txt: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/computed-margin-values/flexbox-row-inline-end.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/computed-margin-values/flexbox-row-multi-line-inline-end-expected.txt: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/computed-margin-values/flexbox-row-multi-line-inline-end.html: Added. * Source/WebCore/css/ComputedStyleExtractor.cpp: (WebCore::rendererContainingBlockHasMarginTrim): (WebCore::isLayoutDependent): (WebCore::ComputedStyleExtractor::valueForPropertyInStyle): * Source/WebCore/rendering/RenderBlock.cpp: (WebCore::RenderBlock::setTrimmedMarginForChild): * Source/WebCore/rendering/RenderBox.cpp: (WebCore::RenderBox::physicalToFlowRelativeDirectionMapping const): (WebCore::RenderBox::hasTrimmedMargin const): * Source/WebCore/rendering/RenderFlexibleBox.cpp: (WebCore::RenderFlexibleBox::trimMainAxisMarginEnd): (WebCore::RenderFlexibleBox::trimCrossAxisMarginEnd): Canonical link: https://commits.webkit.org/262663@main
WebKit · Apr 6, 2023 · 1764795 · 1764795
1 parent 8d8fa5f
commit 1764795
Show file tree

Hide file tree

Showing 12 changed files with 217 additions and 12 deletions.
diff --git a/...sts/css/css-box/margin-trim/computed-margin-values/flexbox-column-inline-end-expected.txt b/...sts/css/css-box/margin-trim/computed-margin-values/flexbox-column-inline-end-expected.txt
@@ -0,0 +1,5 @@
+
+PASS flexbox > item 1
+PASS flexbox > item 2
+PASS flexbox > item 3
+
diff --git a/...tform-tests/css/css-box/margin-trim/computed-margin-values/flexbox-column-inline-end.html b/...tform-tests/css/css-box/margin-trim/computed-margin-values/flexbox-column-inline-end.html
@@ -0,0 +1,43 @@
+<!DOCTYPE html>
+<html>
+<head>
+<link rel="author" href="mailto:sammy.gill@apple.com">
+<link rel="help" href="https://drafts.csswg.org/css-box-4/#margin-trim-flex">
+<meta name="assert" content="trimmed inline-end margins should be reflected in computed style">
+<style>
+flexbox {
+    display: flex;
+    flex-direction: column;
+    width: min-content;
+    margin-trim: inline-end;
+}
+item {
+    display: block;
+    background-color: green;
+    width: 50px;
+    height: 50px;
+}
+item:nth-child(1) {
+    margin-inline-end: 10px;
+}
+item:nth-child(2) {
+    margin-inline-end: -10px;
+}
+item:nth-child(3) {
+    margin-inline-end: 30%;
+}
+</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('flexbox > item')">
+<div id="target">
+<flexbox>
+    <item data-expected-margin-right="0" data-offset-x="8"></item>
+    <item data-expected-margin-right="0" data-offset-x="8"></item>
+    <item data-expected-margin-right="0" data-offset-x="8"></item>
+</flexbox>
+</div>
+</body>
+</html>
diff --git a/...-box/margin-trim/computed-margin-values/flexbox-column-multi-line-inline-end-expected.txt b/...-box/margin-trim/computed-margin-values/flexbox-column-multi-line-inline-end-expected.txt
@@ -0,0 +1,6 @@
+
+PASS flexbox > item 1
+PASS flexbox > item 2
+PASS flexbox > item 3
+PASS flexbox > item 4
+
diff --git a/.../css/css-box/margin-trim/computed-margin-values/flexbox-column-multi-line-inline-end.html b/.../css/css-box/margin-trim/computed-margin-values/flexbox-column-multi-line-inline-end.html
@@ -0,0 +1,38 @@
+<!DOCTYPE html>
+<html>
+<head>
+<link rel="author" href="mailto:sammy.gill@apple.com">
+<link rel="help" href="https://drafts.csswg.org/css-box-4/#margin-trim-flex">
+<meta name="assert" content="trimmed inline-end margins should be reflected in computed style">
+<style>
+flexbox {
+    display: flex;
+    flex-direction: column;
+    flex-wrap: wrap;
+    width: min-content;
+    height: 100px;
+    margin-trim: inline-end;
+}
+item {
+    display: block;
+    background-color: green;
+    width: 50px;
+    height: 50px;
+    margin-inline-end: 10px;
+}
+</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('flexbox > item')">
+<div id="target">
+<flexbox>
+    <item data-offset-x="8" data-expected-margin-right="10"></item>
+    <item data-offset-x="8" data-expected-margin-right="10"></item>
+    <item data-offset-x="68" data-expected-margin-right="0"></item>
+    <item data-offset-x="68" data-expected-margin-right="0"></item>
+</flexbox>
+</div>
+</body>
+</html>
diff --git a/...-tests/css/css-box/margin-trim/computed-margin-values/flexbox-row-inline-end-expected.txt b/...-tests/css/css-box/margin-trim/computed-margin-values/flexbox-row-inline-end-expected.txt
@@ -0,0 +1,5 @@
+
+PASS flexbox > item 1
+PASS flexbox > item 2
+PASS flexbox > item 3
+
diff --git a/...platform-tests/css/css-box/margin-trim/computed-margin-values/flexbox-row-inline-end.html b/...platform-tests/css/css-box/margin-trim/computed-margin-values/flexbox-row-inline-end.html
@@ -0,0 +1,33 @@
+<!DOCTYPE html>
+<html>
+<head>
+<link rel="author" href="mailto:sammy.gill@apple.com">
+<link rel="help" href="https://drafts.csswg.org/css-box-4/#margin-trim-flex">
+<meta name="assert" content="trimmed inline-end margins should be reflected in computed style">
+<style>
+flexbox {
+    display: flex;
+    margin-trim: inline-end;
+}
+item {
+    display: block;
+    background-color: green;
+    width: 50px;
+    height: 50px;
+    margin-inline-end: 20px;
+}
+</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('flexbox > item')">
+<div id="target">
+<flexbox>
+    <item data-expected-margin-right="20" data-offset-x="8"></item>
+    <item data-expected-margin-right="20" data-offset-x="78"></item>
+    <item data-expected-margin-right="0" data-offset-x="148"></item>
+</flexbox>
+</div>
+</body>
+</html>
diff --git a/...css-box/margin-trim/computed-margin-values/flexbox-row-multi-line-inline-end-expected.txt b/...css-box/margin-trim/computed-margin-values/flexbox-row-multi-line-inline-end-expected.txt
@@ -0,0 +1,6 @@
+
+PASS flexbox > item 1
+PASS flexbox > item 2
+PASS flexbox > item 3
+PASS flexbox > item 4
+
diff --git a/...sts/css/css-box/margin-trim/computed-margin-values/flexbox-row-multi-line-inline-end.html b/...sts/css/css-box/margin-trim/computed-margin-values/flexbox-row-multi-line-inline-end.html
@@ -0,0 +1,47 @@
+<!DOCTYPE html>
+<html>
+<head>
+<link rel="author" href="mailto:sammy.gill@apple.com">
+<link rel="help" href="https://drafts.csswg.org/css-box-4/#margin-trim-flex">
+<meta name="assert" content="trimmed inline-end margins should be reflected in computed style">
+<style>
+flexbox {
+    display: flex;
+    width: 110px;
+    flex-wrap: wrap;
+    margin-trim: inline-end;
+}
+item {
+    display: block;
+    background-color: green;
+    width: 50px;
+    height: 50px;
+}
+item:nth-child(1) {
+    margin-inline-end: 10px;
+}
+item:nth-child(2) {
+    margin-inline-end: -10px;
+}
+item:nth-child(3) {
+    margin-inline-end: 10px;
+}
+item:nth-child(4) {
+    margin-inline-end: 50%;
+}
+</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('flexbox > item')">
+<div id="target">
+<flexbox>
+    <item data-expected-margin-right="10" data-offset-x="8"></item>
+    <item data-expected-margin-right="0" data-offset-x="68"></item>
+    <item data-expected-margin-right="10" data-offset-x="8"></item>
+    <item data-expected-margin-right="0" data-offset-x="68"></item>
+</flexbox>
+</div>
+</body>
+</html>
diff --git a/Source/WebCore/css/ComputedStyleExtractor.cpp b/Source/WebCore/css/ComputedStyleExtractor.cpp
@@ -2305,8 +2305,8 @@ static bool rendererContainingBlockHasMarginTrim(const RenderBox& renderer, std:
     }
 
     if (containingBlock->isFlexibleBox()) {
-        ASSERT(!marginTrimType || *marginTrimType == MarginTrimType::BlockStart);
-        return marginTrimType ? containingBlock->style().marginTrim().contains(*marginTrimType) : !containingBlock->style().marginTrim().isEmpty();
+        ASSERT(!marginTrimType || marginTrimType.value() == MarginTrimType::BlockStart || marginTrimType.value() == MarginTrimType::InlineEnd);
+        return marginTrimType ? containingBlock->style().marginTrim().contains(marginTrimType.value()) : !containingBlock->style().marginTrim().containsAny({ MarginTrimType::BlockStart, MarginTrimType::InlineEnd });
     }
     ASSERT_NOT_REACHED();
     return false;
@@ -2347,7 +2347,7 @@ static bool isLayoutDependent(CSSPropertyID propertyID, const RenderStyle* style
     case CSSPropertyMarginTop:
         return paddingOrMarginIsRendererDependent<&RenderStyle::marginTop>(style, renderer) || (isFlexItem(renderer) && rendererContainingBlockHasMarginTrim(downcast<RenderBox>(*renderer), MarginTrimType::BlockStart));
     case CSSPropertyMarginRight:
-        return paddingOrMarginIsRendererDependent<&RenderStyle::marginRight>(style, renderer);
+        return paddingOrMarginIsRendererDependent<&RenderStyle::marginRight>(style, renderer) || (isFlexItem(renderer) && rendererContainingBlockHasMarginTrim(downcast<RenderBox>(*renderer), MarginTrimType::InlineEnd));
     case CSSPropertyMarginBottom:
         return paddingOrMarginIsRendererDependent<&RenderStyle::marginBottom>(style, renderer);
     case CSSPropertyMarginLeft:
@@ -3284,6 +3284,10 @@ RefPtr<CSSValue> ComputedStyleExtractor::valueForPropertyInStyle(const RenderSty
         return zoomAdjustedPaddingOrMarginPixelValue<&RenderStyle::marginTop, &RenderBoxModelObject::marginTop>(style, renderer);
     }
     case CSSPropertyMarginRight: {
+        if (auto* box = dynamicDowncast<RenderBox>(renderer); box && box->isFlexItem() 
+            && rendererContainingBlockHasMarginTrim(*box, MarginTrimType::InlineEnd)
+            && box->hasTrimmedMargin(PhysicalDirection::Right))
+            return zoomAdjustedPixelValue(box->marginRight(), style);
         Length marginRight = style.marginRight();
         if (marginRight.isFixed() || !is<RenderBox>(renderer))
             return zoomAdjustedPixelValueForLength(marginRight, style);

diff --git a/Source/WebCore/rendering/RenderBlock.cpp b/Source/WebCore/rendering/RenderBlock.cpp
@@ -3040,11 +3040,14 @@ bool RenderBlock::updateFragmentRangeForBoxChild(const RenderBox& box) const
 void RenderBlock::setTrimmedMarginForChild(RenderBox &child, MarginTrimType marginTrimType)
 {
     switch (marginTrimType) {
-    case MarginTrimType::BlockStart: {
+    case MarginTrimType::BlockStart:
         setMarginBeforeForChild(child, 0_lu);
         child.markMarginAsTrimmed(MarginTrimType::BlockStart);
         break;
-    }
+    case MarginTrimType::InlineEnd:
+        setMarginEndForChild(child, 0_lu);
+        child.markMarginAsTrimmed(MarginTrimType::InlineEnd);
+        break;
     default:
         ASSERT_NOT_IMPLEMENTED_YET();
     }

diff --git a/Source/WebCore/rendering/RenderBox.cpp b/Source/WebCore/rendering/RenderBox.cpp
@@ -1391,13 +1391,28 @@ void RenderBox::clearOverridingLogicalWidthLength()
 
 FlowRelativeDirection RenderBox::physicalToFlowRelativeDirectionMapping(PhysicalDirection direction) const
 {
-    auto isHorizontalWritingMode = style().isHorizontalWritingMode();
-    auto isLeftToRightDirection = style().isLeftToRightDirection();
+    auto determineFormattingContextRootStyle = [&]() -> const RenderStyle& {
+        if (isFlexItem())
+            return parent()->style();
+        ASSERT_NOT_IMPLEMENTED_YET();
+        return style();
+    };
+    auto& formattingContextRootStyle = determineFormattingContextRootStyle();
+    auto isHorizontalWritingMode = formattingContextRootStyle.isHorizontalWritingMode();
+    auto isLeftToRightDirection = formattingContextRootStyle.isLeftToRightDirection();
+    // vertical-rl and horizontal-bt writing modes
+    auto isFlippedBlocksWritingMode = formattingContextRootStyle.isFlippedBlocksWritingMode();
 
-    if (isHorizontalWritingMode == containingBlock()->style().isHorizontalWritingMode()) {
+    if (isHorizontalWritingMode == formattingContextRootStyle.isHorizontalWritingMode()) {
         switch (direction) {
         case PhysicalDirection::Top:
-            return isHorizontalWritingMode ? FlowRelativeDirection::BlockStart : (isLeftToRightDirection ? FlowRelativeDirection::InlineStart : FlowRelativeDirection::InlineEnd);
+            if (isHorizontalWritingMode)
+                return FlowRelativeDirection::BlockStart;
+            return isLeftToRightDirection ? FlowRelativeDirection::InlineStart : FlowRelativeDirection::InlineEnd;
+        case PhysicalDirection::Right:
+            if (isHorizontalWritingMode)
+                return isLeftToRightDirection ? FlowRelativeDirection::InlineEnd : FlowRelativeDirection::InlineStart;
+            return isFlippedBlocksWritingMode ? FlowRelativeDirection::BlockStart : FlowRelativeDirection::BlockEnd;
         default:
             ASSERT_NOT_IMPLEMENTED_YET();
         } 
@@ -1465,7 +1480,7 @@ bool RenderBox::hasTrimmedMargin(PhysicalDirection physicalDirection) const
 #endif
     ASSERT(!needsLayout());
 
-    if (physicalDirection == PhysicalDirection::Top)
+    if (physicalDirection == PhysicalDirection::Top || physicalDirection == PhysicalDirection::Right)
         return hasTrimmedMargin(flowRelativeDirectionToMarginTrimType(physicalToFlowRelativeDirectionMapping(physicalDirection)));
     ASSERT_NOT_IMPLEMENTED_YET();
     return false;

diff --git a/Source/WebCore/rendering/RenderFlexibleBox.cpp b/Source/WebCore/rendering/RenderFlexibleBox.cpp
@@ -984,7 +984,7 @@ void RenderFlexibleBox::trimMainAxisMarginEnd(const FlexItem& flexItem)
     auto horizontalFlow = isHorizontalFlow();
     flexItem.mainAxisMargin -= horizontalFlow ? flexItem.box.marginEnd(&style()) : flexItem.box.marginAfter(&style());
     if (horizontalFlow)
-        flexItem.box.setMarginEnd(0_lu, &style());
+        setTrimmedMarginForChild(flexItem.box, MarginTrimType::InlineEnd);
     else
         flexItem.box.setMarginAfter(0_lu, &style());
     m_marginTrimItems.m_itemsAtFlexLineEnd.add(&flexItem.box);
@@ -1004,7 +1004,7 @@ void RenderFlexibleBox::trimCrossAxisMarginEnd(const FlexItem& flexItem)
     if (isHorizontalFlow())
         flexItem.box.setMarginAfter(0_lu, &style());
     else
-        flexItem.box.setMarginEnd(0_lu, &style());
+        setTrimmedMarginForChild(flexItem.box, MarginTrimType::InlineEnd);
     m_marginTrimItems.m_itemsOnLastFlexLine.add(&flexItem.box);
 }