Bug 252245 – [css-contain-intrinsic-size] auto-011.html is failing

• Component: CSS
• Status: RESOLVED FIXED

[cathiechen]

Per[1], contain:inline-size applies contain:size to the inline direction, so contain-intrinsic-size:auto should save last remembered size in the inline direction.

[1] https://drafts.csswg.org/css-contain-3/#containment-inline-size

[cathiechen]

Created attachment 464983 [details]
WIP-patch

[Radar WebKit Bug Importer]

<rdar://problem/105728798>

[Oriol Brufau]

Comment on attachment 464983 [details]
WIP-patch

View in context: https://bugs.webkit.org/attachment.cgi?id=464983&action=review

> Source/WebCore/rendering/GridTrackSizingAlgorithm.cpp:798
> +    if (m_direction == ForColumns && (m_strategy->isComputingSizeContainment() || m_strategy->isComputingInlineSizeContainment())) {

May be good to add GridTrackSizingAlgorithmStrategy::isComputingSizeOrInlineSizeContainment()

> Source/WebCore/rendering/RenderBox.cpp:5604
> +    ASSERT(shouldApplySizeContainment() || (isHorizontalWritingMode() && shouldApplyInlineSizeContainment()));

Nit: shouldApplySizeContainment() and shouldApplyInlineSizeContainment() have some common logic that might run twice, so this seems better:

isHorizontalWritingMode() ? shouldApplySizeOrInlineSizeContainment() : shouldApplySizeContainment()

> Source/WebCore/rendering/RenderBox.cpp:5620
> +    ASSERT(shouldApplySizeContainment() || (!isHorizontalWritingMode() && shouldApplyInlineSizeContainment()));

Nit: isHorizontalWritingMode() ? shouldApplySizeContainment() : shouldApplySizeOrInlineSizeContainment()

> Source/WebCore/rendering/RenderGrid.cpp:678
> +    if (direction == ForColumns && shouldApplyInlineSizeContainment())
> +        return true;
> +
> +    return shouldApplySizeContainment();

Nit: return direction == ForColumns ? shouldApplySizeOrInlineSizeContainment() : shouldApplySizeContainment();

[cathiechen]

Comment on attachment 464983 [details]
WIP-patch

View in context: https://bugs.webkit.org/attachment.cgi?id=464983&action=review

Thanks for the review, Oriol!

>> Source/WebCore/rendering/GridTrackSizingAlgorithm.cpp:798
>> +    if (m_direction == ForColumns && (m_strategy->isComputingSizeContainment() || m_strategy->isComputingInlineSizeContainment())) {
> 
> May be good to add GridTrackSizingAlgorithmStrategy::isComputingSizeOrInlineSizeContainment()

Done!

>> Source/WebCore/rendering/RenderBox.cpp:5604
>> +    ASSERT(shouldApplySizeContainment() || (isHorizontalWritingMode() && shouldApplyInlineSizeContainment()));
> 
> Nit: shouldApplySizeContainment() and shouldApplyInlineSizeContainment() have some common logic that might run twice, so this seems better:
> 
> isHorizontalWritingMode() ? shouldApplySizeOrInlineSizeContainment() : shouldApplySizeContainment()

Done

>> Source/WebCore/rendering/RenderBox.cpp:5620
>> +    ASSERT(shouldApplySizeContainment() || (!isHorizontalWritingMode() && shouldApplyInlineSizeContainment()));
> 
> Nit: isHorizontalWritingMode() ? shouldApplySizeContainment() : shouldApplySizeOrInlineSizeContainment()

Done

>> Source/WebCore/rendering/RenderGrid.cpp:678
>> +    return shouldApplySizeContainment();
> 
> Nit: return direction == ForColumns ? shouldApplySizeOrInlineSizeContainment() : shouldApplySizeContainment();

Done:)

[cathiechen]

Created attachment 465135 [details]
WIP-patch

[Oriol Brufau]

Comment on attachment 465135 [details]
WIP-patch

View in context: https://bugs.webkit.org/attachment.cgi?id=465135&action=review

> Source/WebCore/rendering/GridTrackSizingAlgorithm.h:290
> +    bool isComputingSizeOrInlineSizeContainment() { return isComputingSizeContainment() || isComputingInlineSizeContainment(); }

Nit: probably not that important in practice, but making it virtual and then in IndefiniteSizeStrategy

bool isComputingSizeOrInlineSizeContainment() const override { return renderGrid()->shouldApplySizeOrInlineSizeContainment(); }

would ensure a single shouldApplySizeOrStyleContainment().

[cathiechen]

Comment on attachment 465135 [details]
WIP-patch

View in context: https://bugs.webkit.org/attachment.cgi?id=465135&action=review

>> Source/WebCore/rendering/GridTrackSizingAlgorithm.h:290
>> +    bool isComputingSizeOrInlineSizeContainment() { return isComputingSizeContainment() || isComputingInlineSizeContainment(); }
> 
> Nit: probably not that important in practice, but making it virtual and then in IndefiniteSizeStrategy
> 
> bool isComputingSizeOrInlineSizeContainment() const override { return renderGrid()->shouldApplySizeOrInlineSizeContainment(); }
> 
> would ensure a single shouldApplySizeOrStyleContainment().

Done, thanks:)

[cathiechen]

Pull request: https://github.com/WebKit/WebKit/pull/10797

[EWS]

Committed 261003@main (9b61d115bb39): <https://commits.webkit.org/261003@main>

Reviewed commits have been landed. Closing PR #10797 and removing active labels.