Closed Bug 1709491 Opened 6 months ago Closed 5 months ago

Subgrid item's size can be different depend on its container's `overflow` property value

Categories

(Core :: Layout: Grid, defect)

defect

Tracking

()

RESOLVED FIXED
90 Branch
Tracking Status
firefox90 --- fixed

People

(Reporter: TYLin, Assigned: TYLin)

References

(Depends on 1 open bug)

Details

Attachments

(5 files)

While working on bug 1527539, I'm playing with subgrid with overflow property, but I found that the subgrid item's size can be different depend on its container's overflow property value.

Please see the attached testcase. I expect the auto sized <div class="subitem"></div> covering the entire area of the subgrid, and the three cases should have the same rendering.

Also, the second case (overflow:auto) doesn't stable. On my linux machine, the subgrid container doesn't have any scrollbars, but after opening devtools, the scrollbars show. On my Mac, the behavior is just the opposite.

Mats or Daniel, does the above expectation sound correct to you?

Flags: needinfo?(mats)
Flags: needinfo?(dholbert)

Yeah, something seems wrong here. We're probably not properly accounting for the borderpadding when computing the size of the grid area that's available to the innermost item (the child of the subgrid).

Note that in this testcase's examples, each inner item's size is determined by its default align-self:stretch; justify-self:stretch behavior, which make it grow to fill its grid area, since it has width and height of auto. If you set it to align-self:start or end, then it will instead size to its content (which is empty).

So, the subitem here is just showing how large we're thinking the grid area is.

(In reply to Ting-Yu Lin [:TYLin] (UTC-7) from comment #0)

Also, the second case (overflow:auto) doesn't stable. On my linux machine, the subgrid container doesn't have any scrollbars, but after opening devtools, the scrollbars show. On my Mac, the behavior is just the opposite.

If I resize my window horizontally, the scrollbars alternately disappear & show up again with each "tick". I think opening/closing devtools is just another way of triggering this same behavior (forcing an incremental reflow).

Flags: needinfo?(dholbert)

Here's another testcase with 4 grid items in each subgrid, snapped to each corner with align/justify-self.

In the bottom testcase, the end-edge items are positioned out of view for some reason and get clipped (which may indicate that we're making the grid area too large).

Thank you Daniel! I think I know what went wrong.

Assignee: nobody → aethanyc
Status: NEW → ASSIGNED
Flags: needinfo?(mats)

The helper is going to be used in a later part.

I don't add the physical version ComputedPhysicalBorder() deliberately
because I don't want to promote the usage of physical coordinate.

SizeComputationInput's logical margin and border&padding getters accept
arbitrary writing mode, so we can operate on cbWM directly without
using physical coordinates.

This patch is a preparation of the next part, and it shouldn't change
the behavior.

Depends on D114544

When aGridFrame.mFrame is a scroll frame, the inner scrolled frame
subgridFrame only has padding, but no margin nor border. We should add
margin and border on the outer scroll frame to mMarginBorderPadding so
that the grid area of the subgrid is computed correctly.

Depends on D114545

Pushed by aethanyc@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/4c5d0ee7fbe4
Part 1 - Add SizeComputationInput::ComputedLogicalBorder(), and adapt callers. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/d90d530ebb5d
Part 2 - Simplify Subgrid::mMarginBorderPadding computation. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/02aeefba2bce
Part 3 - Fix Subgrid::mMarginBorderPadding when the subgrid is a scroll frame. r=dholbert
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/28969 for changes under testing/web-platform/tests
Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → 90 Branch
Upstream PR merged by moz-wptsync-bot
Blocks: 1711803
You need to log in before you can comment on or make changes to this bug.