1657345 - Using display:grid inside div with column-count does not render properly until Inspector is opened

Status: RESOLVED FIXED

(Core :: Layout: Columns, defect)

Description

[craigbrown24]

Attachment: firefoxbug.html

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_6) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/13.1.2 Safari/605.1.15

Steps to reproduce:

1. Create a two-column grid (using display: grid) inside of a two-column layout (using column-count: 2).
2. Add content to the grid such that the left column (of the outer two-column layout) will have one more row than the right column.

-OR-
Instead of 1 and 2 use the attached example.

3. Load the page. (If it matters, I did this with the browser at the same width as my screen, where my resolution is 1152x720. I'm using MacOS 10.15.6).

Actual results:

The page renders with the content of the right column positioned below the left column (although still on the right). A LOT of blank space is rendered below this section. Any content below it is pushed way down the bottom of a very long page.

Expected results:

The content of the right column should be directly to the right of the left column. The blank space should not be there. (It renders as expected on Chrome if that helps).

Comment 1

[craigbrown24]

I should have mentioned - the layout automatically fixes itself as soon as I try to look at the elements in the Inspector tool. It also fixes itself if I resize the window. Refreshing the page without the inspector tool open will then reproduce the bug.

Comment 2

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

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Comment 3

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

This bug happens after bug 1647332.

Comment 4

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

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

Comment 5

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

This seems like an existing bug in grid container because I see the following log in my local debug build.

nsBlockReflowContext: GridContainer(div)(1)@7f6e94ec5580 metrics=29400,1073736123!

Bug 1647332 can change the way we approach the optimal column balancing block-size. After the bug, the testcase triggers the code path that the column set changed the last column's block-size to unconstrained [1] when reflowing its children one last time (due to its previous balancing iteration is infeasible). Grid container can set its block-size to a very large size when its available block-size is unconstrained.

Before 1647332 landed, the bug was covered because we happen to have an incremental reflow that reflows the entire page again. (I didn't debug to find what triggers it).

[1] https://searchfox.org/mozilla-central/rev/ab81b8552f4aa9696a2524f97fdfeb59d4dc31c1/layout/generic/nsColumnSetFrame.cpp#1189-1191

Comment 6

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

Attachment: Bug 1657345 - Make grid container aware that its available block-size can be unconstrained in paginated context.

nsColumnSetFrame can reflow its last column in an unconstrained
available block-size, so we need to reflow a grid container's children
in fragmentation setup if it has a prev-in-flow.

Comment 7

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

Attachment: Bug 1657345 - Don't force unbounded last column block-size if we've discovered a feasible balancing block-size.

When we reflow the children one last time (due to previous iteration was
infeasble), we don't need to reflow the last column in unconstrained
available block-size if we have discovered a feasible block-size. The
children must fit into the feasible block-size.

This is an optional patch, but I feel it makes sense to do so.

Depends on D86672

Comment 8

[Mats Palmgren (inactive)]

Attachment: Something like this... (in addition to your change)

Comment 9

[Julien Cristau [:jcristau]]

What's the status here TYLin?

Comment 10

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

My patch is waiting for the review.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f32d958a56b6
Make grid container aware that its available block-size can be unconstrained in paginated context. r=mats

Comment 12

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

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

Comment 13

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/f32d958a56b6

Comment 14

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

Upstream PR merged by moz-wptsync-bot