1690423 - Incorrect calculation of block content contribution because we skip aspect-ratio when measuring reflow in Grid layout

Status: RESOLVED FIXED

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

Description

[Boris Chiou [:boris]]

When using aspect-ratio on grid items, we should use the current intrinsic inline size and the aspect-ratio property to calculate the block size of the grid tracks.

For example:
https://codepen.io/jensimmons/pen/JjRZVwg?editors=1100

The content contribution of each grid item doesn't take the aspect-ratio into account, so the actual block size of these grid tracks is incorrect. It seems we shouldn't skip aspect-ratio when measuring reflow.

Comment 1

[Boris Chiou [:boris]]

Attachment: Bug 1690423 - Part 2: Unified auto block-size behavior with UseAutoBSize flag in nsIFrame::ComputeSize().

For grid layout, we set UseAutoBSize flag during measuring reflow to
resolve the grid track sizing. We still have to respect aspect ratio
for the calculation of the content contribution.

Comment 2

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: Unified auto block-size behavior with UseAutoBSize flag in nsIFrame::ComputeSize().

Comment 3

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: Unified auto block-size behavior with UseAutoBSize flag in nsIFrame::ComputeSize().

Comment 4

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

I was trying to unified the nsLayoutUtils::IsAutoBSize usage with the flag ComputeSizeFlag::UseAutoBSize with the patch in comment 2, but I got one subgrid test failure testing/web-platform/tests/css/css-grid/subgrid/orthogonal-writing-mode-003.html. https://treeherder.mozilla.org/jobs?repo=try&revision=cec564abaa37b3f39c7145883cd7fe8a17a2373d

I believe the patch should be a reasonable approach going forward because it give us a foundation to replace the flag with StyleSizeOverrides. I'm replacing UseAutoISize flag in https://phabricator.services.mozilla.com/D103438.

I'm not familiar with subgrid, and probably don't have time to investigate further. If anyone have cycles to investigate, please feel free.

Comment 5

[Boris Chiou [:boris]]

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

I was trying to unified the nsLayoutUtils::IsAutoBSize usage with the flag ComputeSizeFlag::UseAutoBSize with the patch in comment 2, but I got one subgrid test failure testing/web-platform/tests/css/css-grid/subgrid/orthogonal-writing-mode-003.html. https://treeherder.mozilla.org/jobs?repo=try&revision=cec564abaa37b3f39c7145883cd7fe8a17a2373d

I believe the patch should be a reasonable approach going forward because it give us a foundation to replace the flag with StyleSizeOverrides. I'm replacing UseAutoISize flag in https://phabricator.services.mozilla.com/D103438.

I'm not familiar with subgrid, and probably don't have time to investigate further. If anyone have cycles to investigate, please feel free.

Thanks, TYLin. It makes more sense to use the way you provide.

Comment 6

[Boris Chiou [:boris]]

Attachment: Bug 1690423 - Part 1: Fix stretched grid items with aspect-ratio.

Based on https://drafts.csswg.org/css-grid/#grid-item-sizing, the grid
items should igonre aspect-ratio if it's stretched in that axis.

1. If the alignment value is stretch: don't apply aspect ratio.
2. If the alignment value is normal: apply aspect ratio if we have one.
3. Other values: apply aspect ratio if we have one.

Comment 7

[Boris Chiou [:boris]]

Attachment: Bug 1690423 - Part 3: No need to handle transferred size suggestion for the minimum contribution on non-replaced elements.

The spec says: the content-based minimum size for a grid item in a given
dimension is its specified size suggestion if it exists, otherwise its
transferred size suggestion if that exists and the element is replaced, else
its content size suggestion. So for non-replaced elements, we don't have
to take transferred size suggestion into account.

Add some tests when using the preferred size with percentage value and
min-size:auto in grid items, and a test for overflow:auto.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/49ed09f9753a
Part 1: Fix stretched grid items with aspect-ratio. r=mats
https://hg.mozilla.org/integration/autoland/rev/8c8b2e32217e
Part 2: Unified auto block-size behavior with UseAutoBSize flag in nsIFrame::ComputeSize(). r=mats
https://hg.mozilla.org/integration/autoland/rev/1a498df37bd5
Part 3: No need to handle transferred size suggestion for the minimum contribution on non-replaced elements. r=mats

Comment 9

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

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

Comment 10

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

Upstream PR merged by moz-wptsync-bot

Comment 11

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/49ed09f9753a
https://hg.mozilla.org/mozilla-central/rev/8c8b2e32217e
https://hg.mozilla.org/mozilla-central/rev/1a498df37bd5