1966450 - Reddit is broken after enabling layout.css.grid-multi-pass-track-sizing.enabled

Status: RESOLVED FIXED

(Core :: Layout: Grid, defect)

Description

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: Reddit (broken after pref on).png

Reddit front page is broken when layout.css.grid-multi-pass-track-sizing.enabled is enabled. We should fix it before enabling the pref.

Comment 1

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: Reddit (good).png

Comment 2

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Reddit layout is regressed by bug 1944423.

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1944423

Comment 4

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

We haven't enabled layout.css.grid-multi-pass-track-sizing.enabled yet, so set status-firefox140 to disabled.

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1944423

Comment 6

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: Testcase #1: Reduced from Reddit

I have a reduced test case from Reddit that shows the issue. It uses a nested layout: grid, then flex, then another grid.

To reproduce the problem:

1. Set the preference layout.css.grid.multi.pass.track.sizing.enabled to true in about:config.
2. Load the test case.

Observed behavior:
The grid item inside the inner grid container starts with a width of zero, which causes the text to collapse into a single column. After opening DevTools by pressing F12, the width of the grid item updates to 500px.

Additional observation:
If the preference layout.flexbox.item.final.reflow.optimization.enabled is set to false, which forces the flex container to always perform a final reflow for its flex items, the inner grid item has a width of 500px immediately after loading the test case.

Comment 7

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: Bug 1966450 - Skip measuring reflow for grid items when grid container is computing its intrinsic inline contribution.

In Bug 1944423, we introduced logic to resolve row sizes when computing a grid
container's intrinsic inline-size. This can cause grid items to be reflowed to
measure their block-sizes as their content contributions. Since the grid
container is still computing its intrinsic inline-size at this point, the reflow
occurs under provisional inline constraints. As a result, the grid items' sizes
are temporary and must be reflowed again later under the container's final
inline-size.

In grid-container-as-flex-item-001.html, when the outer grid container
prepares a ReflowInput for the final reflow of its grid item (the flex
container), it calls GetMinISize() [1], which recursively calls into the inner
grid container's GetMinISize(), triggering the row size resolution and reflows
the inner grid items as described above.

However, if the inner grid container has already been reflowed during a previous
content measurement by the flex container, its block-size might be cached. In
this case, the flex container may skip reflowing the inner grid container,
leading to incorrect layout results as demonstrated in
grid-container-as-flex-item-001.html.

This patch fixes the issue by marking the grid container (which is a flex item)
as having dirty children when it reflows grid items during intrinsic size
computation. This informs its parent (the flex container) that the block-size
cache is invalid, and a new measuring reflow or final reflow is needed [2][3].

[1] GetMinSize() is called in nsIFrame::ComputeSize():
https://searchfox.org/mozilla-central/rev/fdb34ddfe30bd54aba991feb72b1476c77938e46/layout/generic/nsIFrame.cpp#6771-6773
[2] The flex item's cache is rejected if it has dirty children, triggering a
measuring reflow:
https://searchfox.org/mozilla-central/rev/fdb34ddfe30bd54aba991feb72b1476c77938e46/layout/generic/nsFlexContainerFrame.cpp#2054
[3] If the flex item didn't receive a measuring reflow, it will need a final
reflow:
https://searchfox.org/mozilla-central/rev/fdb34ddfe30bd54aba991feb72b1476c77938e46/layout/generic/nsFlexContainerFrame.cpp#2676-2687

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/b0174fe002b7
https://hg.mozilla.org/integration/autoland/rev/96dfec394a18
Skip measuring reflow for grid items when grid container is computing its intrinsic inline contribution. r=dholbert

Comment 9

[agoloman]

https://hg.mozilla.org/mozilla-central/rev/96dfec394a18

Comment 10

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/53473 for changes under testing/web-platform/tests

Comment 11

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot