1800563 - [css-grid] WPT test subgrid/independent-formatting-context.html fails in Firefox

Status: RESOLVED FIXED

(Core :: Layout: Grid, defect, P3)

Description

[Daniel Holbert [:dholbert]]

This WPT test fails just in Firefox:
https://wpt.fyi/results/css/css-grid/subgrid/independent-formatting-context.html

Direct link to test:
https://wpt.live/css/css-grid/subgrid/independent-formatting-context.html

It happens to be included in the interop2022 test suite, in the subgrid category. Hence: filing under that metabug. (Here's the link for all such tests that we fail, FWIW:
https://wpt.fyi/results/css/css-grid/subgrid?label=master&label=experimental&aligned&view=subtest&q=label%3Ainterop-2022-subgrid%20firefox%3A%21pass )

Comment 1

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

The severity field is not set for this bug.
:dholbert, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 2

[Daniel Holbert [:dholbert]]

There are a bunch of rects here, each testing a different thing that produces an independent formatting context.

We only fail (render red) on the first one, where the .subgrid element has position:absolute. I confirmed that if I set grid-template-rows: initial on that element (make the subgrid styling be ignored/overridden), then we render green there (i.e. we will do the right thing if we start ignoring the subgrid styling on that element).

That element's child is the thing that's green, and we seem to be resolving that green child to have 0 height for some reason.

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: Bug 1800563: Inhibit out-of-flow grid containers from being subgrids. r?TYLin

Comment 4

[Daniel Holbert [:dholbert]]

Attachment: Bug 1800563 part 1: Don't inadvertently clobber masonry bits just because a nested grid can't be a subgrid. r?TYLin

This ComputeSelfSubgridMasonryBits() function has some logic to determine
whether a grid is using subgrid layout (i.e. participating in a parent grid)
and also whether it's using masonry layout at all.

There's one early-return for a case where we can't be a subgrid, which
inadvertently also disregards the masonry state that we've set. It looks like
this was an oversight, since in fact we can still use masonry regardless of
whether we're allowed to be a subgrid.

Comment 5

[Phabricator Automation]

Comment on attachment 9369497 [details]
Bug 1800563 part 1: Don't inadvertently clobber masonry bits just because a nested grid can't be a subgrid. r?TYLin

Revision D196891 was moved to bug 1870911. Setting attachment 9369497 [details] to obsolete.

Comment 6

[Daniel Holbert [:dholbert]]

Attachment: Bug 1800563 part 1: Simplify ComputeSelfSubgridMasonryBits() to return early once we know we can't be a subgrid. r?TYLin

This patch doesn't change behavior.

The code that follows the new early-return is all in the service of making
changes to the return value bits; and all of the modifications are guarded
directly or indirectly by checks for !inhibitSubgrid. So we might as well
just morph this inhibitSubgrid variable into an early-return and skip all of
that code entirely when it evaluates to true.

Depends on D196891

Comment 7

[Phabricator Automation]

Comment on attachment 9369511 [details]
Bug 1800563 part 1: Simplify ComputeSelfSubgridMasonryBits() to return early once we know we can't be a subgrid. r?TYLin

Revision D196900 was moved to bug 1870906. Setting attachment 9369511 [details] to obsolete.

Comment 8

[Daniel Holbert [:dholbert]]

sorry for bugspam; I decided to split parts of this off as patches in helper-bugs, but I left this bug's bug number in the commit messages of some of those helper-bugs. :) Cleaned up now, I think.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9ac6d4619164
Inhibit out-of-flow grid containers from being subgrids. r=TYLin

Comment 10

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/9ac6d4619164