Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[getComputedStyle][margin] isLayoutDependent should handle flow relat…
…ive margin values

https://bugs.webkit.org/show_bug.cgi?id=255890

Reviewed by Antti Koivisto.

Shorthand values should simply resolve for individual css property values.

* LayoutTests/imported/w3c/web-platform-tests/css/css-logical/parsing/margin-block-inline-computed-expected.txt:
* LayoutTests/platform/mac-wk1/TestExpectations:
* LayoutTests/platform/mac/TestExpectations:
* Source/WebCore/css/ComputedStyleExtractor.cpp:
(WebCore::formattingContextRootStyle):
(WebCore::physicalToFlowRelativeDirection):
(WebCore::flowRelativeToPhysicalDirection):
(WebCore::toMarginPropertyID):
(WebCore::isLayoutDependent):

Canonical link: https://commits.webkit.org/263372@main
  • Loading branch information
alanbaradlay committed Apr 25, 2023
1 parent 33f8953 commit 0f08796
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 16 deletions.
@@ -1,12 +1,12 @@

PASS Property margin-block-start value '10px'
FAIL Property margin-block-end value '10%' assert_equals: expected "20px" but got "0px"
PASS Property margin-block-end value '10%'
PASS Property margin-inline-start value '30px'
PASS Property margin-inline-end value '1em'
FAIL Property margin-block-start value 'calc(10% + 40px)' assert_equals: expected "60px" but got "0px"
PASS Property margin-block-start value 'calc(10% + 40px)'
PASS Property margin-block-end value 'calc(10px + 0.5em)'
PASS Property margin-inline-start value 'calc(10px + 0.5em)'
FAIL Property margin-inline-end value 'calc(10% + 40px)' assert_equals: expected "60px" but got "40px"
PASS Property margin-inline-end value 'calc(10% + 40px)'
PASS Property margin-block value '10px'
PASS Property margin-block value '10px 20px'
PASS Property margin-inline value '30px'
Expand Down
2 changes: 0 additions & 2 deletions LayoutTests/platform/mac-wk1/TestExpectations
Expand Up @@ -1996,8 +1996,6 @@ webkit.org/b/215033 imported/w3c/web-platform-tests/websockets/cookies/third-par

webkit.org/b/215296 media/modern-media-controls/placard/placard-ltr.html [ Pass Timeout ]

webkit.org/b/215299 imported/w3c/web-platform-tests/css/css-logical/parsing/margin-block-inline-computed.html [ Pass Failure ]

webkit.org/b/215336 [ Release ] svg/text/hidpi-text-selection-rect-position.html [ Pass Failure ]

webkit.org/b/215392 webaudio/oscillator-sawtooth.html [ Pass Failure ]
Expand Down
1 change: 0 additions & 1 deletion LayoutTests/platform/mac/TestExpectations
Expand Up @@ -1726,7 +1726,6 @@ imported/w3c/web-platform-tests/css/css-sizing/percentage-height-in-flexbox.html
[ Monterey+ ] imported/w3c/web-platform-tests/dom/events/Event-dispatch-redispatch.html [ Pass Failure ]

# rdar://66841102 ([AS Layout Tests] REGRESSION (r264522): [ MacOS wk1 ] 2 css-logical/parsing tests are a flaky failure)
imported/w3c/web-platform-tests/css/css-logical/parsing/margin-block-inline-computed.html [ Pass Failure ]
imported/w3c/web-platform-tests/css/css-logical/parsing/padding-block-inline-computed.html [ Pass Failure ]

# <rdar://problem/62848940> [ Mac Debug ] imported/w3c/web-platform-tests/html/semantics/embedded-content/media-elements/media_fragment_seek.html is flaky failing.
Expand Down
90 changes: 80 additions & 10 deletions Source/WebCore/css/ComputedStyleExtractor.cpp
Expand Up @@ -2311,20 +2311,22 @@ static bool rendererCanHaveTrimmedMargin(const RenderBox& renderer, std::optiona
using PhysicalDirection = BoxSide;
using FlowRelativeDirection = LogicalBoxSide;

static const RenderStyle& formattingContextRootStyle(const RenderBox& renderer)
{
if (auto* ancestorToUse = (renderer.isFlexItem() || renderer.isGridItem()) ? renderer.parent() : renderer.containingBlock())
return ancestorToUse->style();
ASSERT_NOT_REACHED();
return renderer.style();
};

// Mapping is done according to the table in section 6.4 (Abstract-to-Physical Mappings)
static FlowRelativeDirection physicalToFlowRelativeDirection(const RenderBox& renderer, PhysicalDirection direction)
{
auto determineFormattingContextRootStyle = [&]() -> const RenderStyle& {
if (auto* ancestorToUse = (renderer.isFlexItem() || renderer.isGridItem()) ? renderer.parent() : renderer.containingBlock())
return ancestorToUse->style();
ASSERT_NOT_REACHED();
return renderer.style();
};
auto& formattingContextRootStyle = determineFormattingContextRootStyle();
auto isHorizontalWritingMode = formattingContextRootStyle.isHorizontalWritingMode();
auto isLeftToRightDirection = formattingContextRootStyle.isLeftToRightDirection();
auto& styleToUse = formattingContextRootStyle(renderer);
auto isHorizontalWritingMode = styleToUse.isHorizontalWritingMode();
auto isLeftToRightDirection = styleToUse.isLeftToRightDirection();
// vertical-rl and horizontal-bt writing modes
auto isFlippedBlocksWritingMode = formattingContextRootStyle.isFlippedBlocksWritingMode();
auto isFlippedBlocksWritingMode = styleToUse.isFlippedBlocksWritingMode();

switch (direction) {
case PhysicalDirection::Top:
Expand All @@ -2349,6 +2351,37 @@ static FlowRelativeDirection physicalToFlowRelativeDirection(const RenderBox& re
}
}

static PhysicalDirection flowRelativeToPhysicalDirection(const RenderBox& renderer, FlowRelativeDirection direction)
{
auto& styleToUse = formattingContextRootStyle(renderer);
auto isHorizontalWritingMode = styleToUse.isHorizontalWritingMode();
auto isLeftToRightDirection = styleToUse.isLeftToRightDirection();
// vertical-rl and horizontal-bt writing modes
auto isFlippedBlocksWritingMode = styleToUse.isFlippedBlocksWritingMode();

switch (direction) {
case FlowRelativeDirection::BlockStart:
if (isHorizontalWritingMode)
return !isFlippedBlocksWritingMode ? PhysicalDirection::Top : PhysicalDirection::Bottom;
return !isFlippedBlocksWritingMode ? PhysicalDirection::Left : PhysicalDirection::Right;
case FlowRelativeDirection::BlockEnd:
if (isHorizontalWritingMode)
return !isFlippedBlocksWritingMode ? PhysicalDirection::Bottom : PhysicalDirection::Top;
return !isFlippedBlocksWritingMode ? PhysicalDirection::Right : PhysicalDirection::Left;
case FlowRelativeDirection::InlineStart:
if (isHorizontalWritingMode)
return isLeftToRightDirection ? PhysicalDirection::Left : PhysicalDirection::Right;
return isLeftToRightDirection ? PhysicalDirection::Top : PhysicalDirection::Bottom;
case FlowRelativeDirection::InlineEnd:
if (isHorizontalWritingMode)
return isLeftToRightDirection ? PhysicalDirection::Right : PhysicalDirection::Left;
return isLeftToRightDirection ? PhysicalDirection::Bottom : PhysicalDirection::Top;
default:
ASSERT_NOT_REACHED();
return { };
}
}

static MarginTrimType toMarginTrimType(const RenderBox& renderer, CSSPropertyID propertyID)
{
auto flowRelativeDirectionToMarginTrimType = [](auto direction) {
Expand Down Expand Up @@ -2382,6 +2415,23 @@ static MarginTrimType toMarginTrimType(const RenderBox& renderer, CSSPropertyID
}
}

static CSSPropertyID toMarginPropertyID(FlowRelativeDirection direction, const RenderBox& renderer)
{
switch (flowRelativeToPhysicalDirection(renderer, direction)) {
case PhysicalDirection::Top:
return CSSPropertyMarginTop;
case PhysicalDirection::Right:
return CSSPropertyMarginRight;
case PhysicalDirection::Bottom:
return CSSPropertyMarginBottom;
case PhysicalDirection::Left:
return CSSPropertyMarginLeft;
default:
ASSERT_NOT_REACHED();
return { };
}
}

static bool isLayoutDependent(CSSPropertyID propertyID, const RenderStyle* style, RenderObject* renderer)
{
switch (propertyID) {
Expand Down Expand Up @@ -2414,6 +2464,26 @@ static bool isLayoutDependent(CSSPropertyID propertyID, const RenderStyle* style
&& style->marginBottom().isFixed() && style->marginLeft().isFixed())
|| (rendererCanHaveTrimmedMargin(downcast<RenderBox>(*renderer), { }));
}
case CSSPropertyMarginBlock:
return isLayoutDependent(CSSPropertyMarginBlockStart, style, renderer) || isLayoutDependent(CSSPropertyMarginBlockEnd, style, renderer);
case CSSPropertyMarginInline:
return isLayoutDependent(CSSPropertyMarginInlineStart, style, renderer) || isLayoutDependent(CSSPropertyMarginInlineEnd, style, renderer);
case CSSPropertyMarginBlockStart:
if (auto* renderBox = dynamicDowncast<RenderBox>(renderer))
return isLayoutDependent(toMarginPropertyID(FlowRelativeDirection::BlockStart, *renderBox), style, renderBox);
return false;
case CSSPropertyMarginBlockEnd:
if (auto* renderBox = dynamicDowncast<RenderBox>(renderer))
return isLayoutDependent(toMarginPropertyID(FlowRelativeDirection::BlockEnd, *renderBox), style, renderBox);
return false;
case CSSPropertyMarginInlineStart:
if (auto* renderBox = dynamicDowncast<RenderBox>(renderer))
return isLayoutDependent(toMarginPropertyID(FlowRelativeDirection::InlineStart, *renderBox), style, renderBox);
return false;
case CSSPropertyMarginInlineEnd:
if (auto* renderBox = dynamicDowncast<RenderBox>(renderer))
return isLayoutDependent(toMarginPropertyID(FlowRelativeDirection::InlineEnd, *renderBox), style, renderBox);
return false;
case CSSPropertyMarginTop:
return paddingOrMarginIsRendererDependent<&RenderStyle::marginTop>(style, renderer) || (is<RenderBox>(renderer) && (rendererCanHaveTrimmedMargin(downcast<RenderBox>(*renderer), MarginTrimType::BlockStart)));
case CSSPropertyMarginRight:
Expand Down

0 comments on commit 0f08796

Please sign in to comment.