Bug 231802 – [css-grid] update the content-sized grid width before laying out a grid item with block constraints and aspect-ratio

• Component: CSS
• Status: RESOLVED FIXED

We need to relayout grid items when the grid width changes due to the change on item size.

Created attachment 441359 [details]
Patch

[Sergio Villar Senin]

Comment on attachment 441359 [details]
Patch

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

> Source/WebCore/rendering/RenderGrid.cpp:974
> +            child->layoutIfNeeded();

We shouldn't layout the item twice. If you need to recompute the preferred width then just do it before the child->layoutIfNeeded() call above and do the layout just once.

Recompute the preferred with before laying out a grid item with block constraints

Created attachment 441373 [details]
Patch

[Sergio Villar Senin]

Comment on attachment 441373 [details]
Patch

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

> Source/WebCore/rendering/RenderGrid.cpp:972
> +            updateLogicalWidth();

The part I don't understand is that you mention that you need to recompute the preferred width of the item but you call updateLogicalWidth() in the grid. This means that we're recomputing the width of the grid in the middle of the the layout of the items. That's problematic, note that some items might be laid out with the old grid width while some others will use the updated one.

[Radar WebKit Bug Importer]

<rdar://problem/84542687>

[Frédéric Wang (:fredw)]

Comment on attachment 441373 [details]
Patch

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

> Source/WebCore/ChangeLog:14
> +        IOS platform. However, we noticed that they also fail for GTK at

iOS*

> Source/WebCore/rendering/RenderGrid.cpp:970
> +        // We need to recompute the preferred width for gird items with block constraints.

grid*

Created attachment 443394 [details]
Patch

Created attachment 443397 [details]
Patch

[Javier Fernandez]

Comment on attachment 443397 [details]
Patch

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

> Source/WebCore/rendering/RenderGrid.cpp:282
> +            updateLogicalWidth();

I don't understand why this solves the issue. I thought we needed to repeat the track sizing algorithm; don't we ?

Additionally, wouldn't we need to call setPreferredLogicalWidthsDirty for the grid container so that we run the track sizing algorithm again to adjust its intrinsic size ?

Created attachment 443532 [details]
Patch

Created attachment 443546 [details]
Patch

[Javier Fernandez]

Comment on attachment 443546 [details]
Patch

The change looks good, but I think we should add the new test that we discussed today; the one to verify that we update the tracks after applying the aspect ratio.

Created attachment 443579 [details]
Patch

[EWS Watchlist]

This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess

[Javier Fernandez]

Comment on attachment 443579 [details]
Patch

r=me

Please, ensure the new test is exported to the WPT repository.

[EWS]

Committed r285497 (244021@main): <https://commits.webkit.org/244021@main>

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