Closed Bug 1657345 Opened 1 year ago Closed 1 year ago

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

Categories

(Core :: Layout: Columns, defect)

80 Branch
defect

Tracking

()

RESOLVED FIXED
82 Branch
Tracking Status
firefox-esr68 --- unaffected
firefox-esr78 --- unaffected
firefox79 --- wontfix
firefox80 --- wontfix
firefox81 --- wontfix
firefox82 --- fixed

People

(Reporter: craigbrown24, Assigned: TYLin)

References

(Regression)

Details

(Keywords: regression)

Attachments

(3 files, 1 obsolete file)

Attached file 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.

  1. 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).

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.

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

Component: Untriaged → Inspector
Product: Firefox → DevTools
Component: Inspector → Layout: Columns
Product: DevTools → Core

This bug happens after bug 1647332.

Severity: -- → S3
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(aethanyc)
Regressed by: 1647332

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

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.

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

Assignee: nobody → aethanyc
Status: NEW → ASSIGNED
Flags: needinfo?(aethanyc)
Attachment #9171437 - Attachment is patch: true
Attachment #9171437 - Attachment mime type: application/octet-stream → text/plain
Attachment #9171437 - Attachment description: Something like this... → Something like this... (in addition to your change)
Attachment #9169355 - Attachment is obsolete: true
Attachment #9169354 - Attachment description: Bug 1657345 - Find grid's fragmentainer if the grid container has a prev-in-flow. → Bug 1657345 - Make grid container aware that its available block-size can be unconstrained in paginated context.

What's the status here TYLin?

Flags: needinfo?(aethanyc)

My patch is waiting for the review.

Flags: needinfo?(aethanyc) → needinfo?(mats)
Pushed by aethanyc@gmail.com:
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
Flags: needinfo?(mats)
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/25500 for changes under testing/web-platform/tests
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 82 Branch
Upstream PR merged by moz-wptsync-bot
You need to log in before you can comment on or make changes to this bug.