Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[margin-trim] Trimmed inline-start margins for grid items in horizont… 
…al writing-mode should be reflected in computed style.

https://bugs.webkit.org/show_bug.cgi?id=253718
rdar://106559597

Reviewed by Alan Baradlay.

When a grid has inline-start specified for margin trim, it will trim the
inline-start margins of any items that are placed in the first column
of the grid. These trimmed margins should be reflected when querying the
computed style for the item. In order for this to happen,
ComputedStyleExtractor must be able to identify when the inline-start/left
margins of a grid item are trimmed. We can accomplish this by setting
the rare data bits for the renderer as this margin is trimmed during
layout. Then, ComputedStyleExtractor will be able to check to see if
this bit is set.

The inline-start margins for grid items are trimmed inside of
RenderBox::computeOrTrimInlineMargin as the renderer is going through
layout. This function is called as the renderer is trying to determine
its inline margin values as part of RenderBox::computeLogicalWidthInFragment.
The renderer of the grid item will consult with the grid via
shouldTrimChildMargin to determine if it should return a trimmed margin
value. At this time the renderer can also call into markMarginAsTrimmed
to set the rare data bit for the margin.

Once layout is finished, ComputedStyleExtractor can use
RenderBox::hasTrimmedMargin to check if the "left," margin is trimmed.
This function will return true only if the rare data bit is set for that
specified margin.

* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/computed-margin-values/grid-inline-start-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/computed-margin-values/grid-inline-start-item-negative-span-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/computed-margin-values/grid-inline-start-item-negative-span.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/computed-margin-values/grid-inline-start.html: Added.
* Source/WebCore/css/ComputedStyleExtractor.cpp:
(WebCore::rendererCanHaveTrimmedMargin):
(WebCore::isLayoutDependent):
(WebCore::ComputedStyleExtractor::valueForPropertyInStyle):
* Source/WebCore/rendering/RenderBox.cpp:
(WebCore::RenderBox::physicalToFlowRelativeDirectionMapping const):
(WebCore::RenderBox::hasTrimmedMargin const):
(WebCore::RenderBox::computeOrTrimInlineMargin const):

Canonical link: https://commits.webkit.org/263006@main
  • Loading branch information
@sgill26
sgill26 authored and Sammy Gill committed Apr 16, 2023
1 parent 0e0b997 commit 53332e5
Show file tree
Hide file tree
Showing 6 changed files with 134 additions and 7 deletions.
7 changes: 7 additions & 0 deletions ...tform-tests/css/css-box/margin-trim/computed-margin-values/grid-inline-start-expected.txt
View file
@@ -0,0 +1,7 @@

PASS grid > item 1
PASS grid > item 2
PASS grid > item 3
PASS grid > item 4
PASS grid > item 5

4 changes: 4 additions & 0 deletions ...-box/margin-trim/computed-margin-values/grid-inline-start-item-negative-span-expected.txt
View file
@@ -0,0 +1,4 @@

PASS grid > item 1
PASS grid > item 2

42 changes: 42 additions & 0 deletions .../css/css-box/margin-trim/computed-margin-values/grid-inline-start-item-negative-span.html
View file
@@ -0,0 +1,42 @@
<!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 inline-start margins in grid should be reflected in computed style">
</head>
<style>
grid {
display: grid;
width: min-content;
outline: 1px solid black;
grid-template-columns: repeat(2, auto);
margin-trim: inline-start;
}
item {
display: block;
width: 50px;
height: 50px;
margin-inline-start: 10px;
background-color: green;
}
.negative-line-number {
width: 50px;
grid-row: 2;
grid-column: -3;
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 data-expected-margin-left="0" class="negative-line-number"></item>
<item data-expected-margin-left="0"></item>
</grid>
</div>
</body>
</html>
63 changes: 63 additions & 0 deletions .../web-platform-tests/css/css-box/margin-trim/computed-margin-values/grid-inline-start.html
View file
@@ -0,0 +1,63 @@
<!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 inline-start margins in grid should be reflected in computed style">
</head>
<style>
grid {
display: grid;
width: min-content;
outline: 1px solid black;
grid-template-columns: repeat(2, auto);
margin-trim: inline-start;
}
item {
display: block;
width: 50px;
height: 50px;
}
.locked-position {
grid-row: 3;
grid-column: 1;
margin-inline-start: -30px;
}
item:nth-child(1) {
background-color: green;
margin-inline-start: 30px;
}
item:nth-child(2) {
background-color: blue;
margin-inline-start: 10px;
}
item:nth-child(3) {
background-color: orchid;
margin-inline-start: 10%;
}
item:nth-child(4) {
background-color: maroon;
}
item:nth-child(5) {
background-color: salmon;
width: auto;
grid-column: span 2;
margin-inline-start: 10px;
}
</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-left="0"></item>
<item data-expected-margin-left="10"></item>
<item data-expected-margin-left="0"></item>
<item class="locked-position" data-expected-margin-left="0"></item>
<item data-expected-margin-left="0"></item>
</grid>
</div>
</body>
</html>
13 changes: 8 additions & 5 deletions Source/WebCore/css/ComputedStyleExtractor.cpp
View file
Expand Up @@ -2312,9 +2312,9 @@ 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 || marginTrimType.value() == MarginTrimType::BlockEnd)) {
if (containingBlock->isRenderGrid() && (!marginTrimType || marginTrimType.value() == MarginTrimType::BlockStart || marginTrimType.value() == MarginTrimType::BlockEnd || marginTrimType.value() == MarginTrimType::InlineStart)) {
if (!marginTrimType)
return containingBlock->style().marginTrim().containsAny({ MarginTrimType::BlockStart, MarginTrimType::BlockEnd });
return containingBlock->style().marginTrim().containsAny({ MarginTrimType::BlockStart, MarginTrimType::BlockEnd, MarginTrimType::InlineStart });
return containingBlock->style().marginTrim().contains(marginTrimType.value());
}
return false;
Expand Down Expand Up @@ -2359,7 +2359,7 @@ static bool isLayoutDependent(CSSPropertyID propertyID, const RenderStyle* style
case CSSPropertyMarginBottom:
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));
return paddingOrMarginIsRendererDependent<&RenderStyle::marginLeft>(style, renderer) || (is<RenderBox>(renderer) && rendererCanHaveTrimmedMargin(downcast<RenderBox>(*renderer), MarginTrimType::InlineStart));
case CSSPropertyPadding: {
if (!renderer || !renderer->isBox())
return false;
Expand Down Expand Up @@ -3317,10 +3317,13 @@ RefPtr<CSSValue> ComputedStyleExtractor::valueForPropertyInStyle(const RenderSty
&& box->hasTrimmedMargin(PhysicalDirection::Bottom))
return zoomAdjustedPixelValue(box->marginBottom(), style);
return zoomAdjustedPaddingOrMarginPixelValue<&RenderStyle::marginBottom, &RenderBoxModelObject::marginBottom>(style, renderer);
case CSSPropertyMarginLeft:
if (auto* box = dynamicDowncast<RenderBox>(renderer); box && box->isFlexItem() && box->hasTrimmedMargin(PhysicalDirection::Left))
case CSSPropertyMarginLeft: {
if (auto* box = dynamicDowncast<RenderBox>(renderer); box
&& rendererCanHaveTrimmedMargin(*box, MarginTrimType::InlineStart)
&& box->hasTrimmedMargin(PhysicalDirection::Left))
return zoomAdjustedPixelValue(box->marginLeft(), style);
return zoomAdjustedPaddingOrMarginPixelValue<&RenderStyle::marginLeft, &RenderBoxModelObject::marginLeft>(style, renderer);
}
case CSSPropertyMarginTrim: {
auto marginTrim = style.marginTrim();
if (marginTrim.isEmpty())
Expand Down
12 changes: 10 additions & 2 deletions Source/WebCore/rendering/RenderBox.cpp
View file
Expand Up @@ -1474,7 +1474,7 @@ bool RenderBox::hasTrimmedMargin(std::optional<MarginTrimType> marginTrimType) c
return false;
}
if (containingBlock && containingBlock->isRenderGrid())
ASSERT(!marginTrimType || marginTrimType.value() == MarginTrimType::BlockStart || marginTrimType.value() == MarginTrimType::BlockEnd);
ASSERT(!marginTrimType || marginTrimType.value() == MarginTrimType::BlockStart || marginTrimType.value() == MarginTrimType::BlockEnd || marginTrimType.value() == MarginTrimType::InlineStart);
#endif
if (!hasRareData())
return false;
Expand Down Expand Up @@ -3033,8 +3033,16 @@ bool RenderBox::sizesLogicalWidthToFitContent(SizeType widthType) const
template<typename Function>
LayoutUnit RenderBox::computeOrTrimInlineMargin(const RenderBlock& containingBlock, MarginTrimType marginSide, const Function& computeInlineMargin) const
{
if (containingBlock.shouldTrimChildMargin(marginSide, *this))
if (containingBlock.shouldTrimChildMargin(marginSide, *this)) {
// FIXME(255434): This should be set when the margin is being trimmed
// within the context of its layout system (block, flex, grid) and should not
// 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::InlineStart)
const_cast<RenderBox&>(*this).markMarginAsTrimmed(MarginTrimType::InlineStart);
return 0_lu;
}
return computeInlineMargin();
}

Expand Down

0 comments on commit 53332e5

Please sign in to comment.