1654044 - Firefox incorrectly treats flex items' heights as definite if they have `overflow: {scroll, hidden,auto}`, in vertical indefinite-height flex container

Status: RESOLVED FIXED

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

Description

[Daniel Holbert [:dholbert]]

Attachment: testcase 1

STR:

1. Load attached testcase.

EXPECTED RESULTS:
No orange in any of the subtests (aside from the scrollbar tracks in the middle two subtests, which might show orange)

ACTUAL RESULTS:
The first three subtests are orange on their bottom half.

This indicates that we're treating the flex item's resolved main-size (height) as "definite" in these overflow:{hidden,auto,scroll} cases (but not in the fourth case, where the flex item has overflow:visible).

It seems we're misapplying "point 2" from https://drafts.csswg.org/css-flexbox-1/#definite-sizes here (inadvertently); the flex container has an indefinite height, so we shouldn't be promoting the flex items' main size to be definite.

Chrome gives EXPECTED RESULTS.

Comment 1

[Daniel Holbert [:dholbert]]

I suspect the issue here is that we're failing to propagate mTreatBSizeAsIndefinite through to the scrolled frame, or something like that.

Comment 2

[Daniel Holbert [:dholbert]]

(CC'ing TYLin in case he's interested in taking a look.)

Comment 3

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

Grid container does seem to have this bug by changing display: inline-flex to display: inline-grid in .container, and grid doesn't use mTreatBSizeAsIndefinite. So we might want to find a solution that can fix both.

Comment 4

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

Attachment: Bug 1654044 - Carry mTreatBSizeAsIndefinite flag from scroll frame to scrolled inner frame (-moz-scrolled-content). r?dholbert

Comment 5

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

This bug affects YouTube reported in bug 1723448.

Comment 7

[Daniel Holbert [:dholbert]]

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

This bug affects YouTube reported in bug 1723448.

Given this^, it might be worth nominating this for uplift to beta during the "early beta" period, after it's landed and baked on Nightly for a few days.

(In the pretty-unlikely scenario that it causes some unforeseen layout breakage, and we have to rethink this somehow, then of course it'd be extremely easy & safe to back out from beta at that point, since it's just a single line of code. So: relatively high reward & low risk.)

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/341b95ccba1f
Carry mTreatBSizeAsIndefinite flag from scroll frame to scrolled inner frame (-moz-scrolled-content). r=dholbert

Comment 9

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

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

Comment 10

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/341b95ccba1f

Comment 11

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

Upstream PR merged by moz-wptsync-bot

Comment 12

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

(In reply to Daniel Holbert [:dholbert] from comment #7)

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

This bug affects YouTube reported in bug 1723448.

Given this^, it might be worth nominating this for uplift to beta during the "early beta" period, after it's landed and baked on Nightly for a few days.

Per bug 1723448 comment 7, Youtube seems changed to workaround this bug, so we don't have the pressure to uplift my patch unless we get another webcompat report.