[css-grid] Transfer size for grid item with an aspect-ratio and stretch alignment against the definite row

• Component: CSS
• Status: RESOLVED FIXED

This is similar to bug 231802. Instead of having a percentage of the definite row, the grid item stretches in the block-axis.

Affected test -

imported/w3c/web-platform-tests/css/css-grid/grid-items/aspect-ratio-004.html

Created attachment 443923 [details]
Patch

Created attachment 443938 [details]
Patch

[Javier Fernandez]

Comment on attachment 443938 [details]
Patch

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

> Source/WebCore/rendering/RenderGrid.cpp:965
> +        if (autoGridItem->hasStretchedLogicalHeight())

I think a comment here, explaining why we need to apply the stretch alignment, and how it affects the aspect-ratio, would be a good idea.

> Source/WebCore/rendering/RenderReplaced.cpp:805
> +    return (hasRelativeLogicalHeight() || (isGridItem() && hasStretchedLogicalHeight())) && style().logicalWidth().isAuto();

Do we need to check for isGridItem() ? wouldn't this be relevant for flexbox (the other layout model applying stretch alignment) as well ?

Created attachment 444370 [details]
Patch

(In reply to Javier Fernandez from comment #3)
> Comment on attachment 443938 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=443938&action=review
> 
> > Source/WebCore/rendering/RenderGrid.cpp:965
> > +        if (autoGridItem->hasStretchedLogicalHeight())
> 
> I think a comment here, explaining why we need to apply the stretch
> alignment, and how it affects the aspect-ratio, would be a good idea.
> 

Comments added.

> > Source/WebCore/rendering/RenderReplaced.cpp:805
> > +    return (hasRelativeLogicalHeight() || (isGridItem() && hasStretchedLogicalHeight())) && style().logicalWidth().isAuto();
> 
> Do we need to check for isGridItem() ? wouldn't this be relevant for flexbox
> (the other layout model applying stretch alignment) as well ?

Yes, I think we do need to check for isGridItem(). I have seen a couple of table related tests failures without this check. Tried running WPT flexbox tests with the check (e.g. add (isGridItem() || isFlexItem()) but couldn't see it makes any difference on the test result for flexibox tests.

[Javier Fernandez]

Comment on attachment 444370 [details]
Patch

r=me

[EWS]

Committed r285987 (244384@main): <https://commits.webkit.org/244384@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 444370 [details].

[Radar WebKit Bug Importer]

<rdar://problem/85540225>