2014220 - flex item ends up the wrong size, in situation with several nested flex containers (related to layout.flexbox.item-final-reflow-optimization.enabled)

Status: RESOLVED FIXED

(Core :: Layout: Flexbox, defect)

Description

[Daniel Holbert [:dholbert]]

Attachment: testcase 1

STR:

1. Load attached testcase.

EXPECTED RESULTS:
No red (lime element should vertically grow to cover the red).

ACTUAL RESULTS:
Red is visible. (Lime element is short and doesn't cover the red.)

This is the root cause of bug 2011640.

This seems to be a bug with the optimization that's controlled by layout.flexbox.item-final-reflow-optimization.enabled -- if I turn off that pref, then we get expected results.

Also, if I make various minor changes in the testcase (e.g. removing one of the two col wrappers) then we get expected results.

Comment 1

[Daniel Holbert [:dholbert]]

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

This is the root cause of bug 2011640.

That site-report bug is not longer reproducible, as noted over in that bug just now - likely due to a site change.

Here's our reduced testcase from that bug (from which I reduced the testcase that I attached here in comment 0):
https://bugzilla.mozilla.org/attachment.cgi?id=9539227

When we fix this bug, we should check that that^ reduced testcase is fixed too.

Comment 2

[David Shin[:dshin]]

So, one of the checks for bailing out of final reflow is FrameHasRelativeBSizeDependency, which checks for the flex container and its direct children having NS_FRAME_CONTAINS_RELATIVE_BSIZE.

So the general flow is

1. Do the measuring reflow
2. For each line:
3. For each item:
4. Check if the item frame has NS_FRAME_CONTAINS_RELATIVE_BSIZE
5. Check if any of the item frame's children has NS_FRAME_CONTAINS_RELATIVE_BSIZE
6. If 4. or 5. is true, the item requires the final reflow.

For this particular case, for .row, the item frame it looks at is the outer .col. It and its sole child (The inner .col) doesn't have NS_FRAME_CONTAINS_RELATIVE_BSIZE, so it doesn't do the final reflow.

If one of the .col containers are erased, .row looks at .col, which then looks at .elem-with-min-height, which has NS_FRAME_CONTAINS_RELATIVE_BSIZE set, so everything works as intended.

Comment 3

[David Shin[:dshin]]

:TYLin, do you know if there's a historical reason behind flexboxes not inheriting NS_FRAME_CONTAINS_RELATIVE_BSIZE from its children? Doc for the flag itself makes me think it should...

Comment 4

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

This sounds like NS_FRAME_CONTAINS_RELATIVE_BSIZE is not propagated properly. I'm wonder if we should generalize NS_FRAME_CONTAINS_RELATIVE_BSIZE bit, and always add it to the flexible flex item in a column-oriented flex container, because the item's block-size is somewhat depending on its parent flex container.

Comment 5

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

:TYLin, do you know if there's a historical reason behind flexboxes not inheriting NS_FRAME_CONTAINS_RELATIVE_BSIZE from its children? Doc for the flag itself makes me think it should...

I don't think there is any good reason for that. NS_FRAME_CONTAINS_RELATIVE_BSIZE for the percentage height was invented before flexbox ;)

Comment 6

[David Shin[:dshin]]

Hrm, digging further, there's probably more to this. The flag only propagates up to the containing block, which is deliberately kept as short as possible AFAICT.

Comment 7

[David Shin[:dshin]]

Attachment: Bug 2014220: Propagate up `NS_FRAME_CONTAINS_RELATIVE_BSIZE` from items in nested flex containers. r=#layout

... Since the final reflow of the ancestor flex container may make the block size definite.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/5180854d6536
https://hg.mozilla.org/integration/autoland/rev/91d59dd14d3c
Propagate up `NS_FRAME_CONTAINS_RELATIVE_BSIZE` from items in nested flex containers. r=layout-reviewers,TYLin

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/61055 for changes under testing/web-platform/tests

Comment 10

[amarc]

https://hg.mozilla.org/mozilla-central/rev/91d59dd14d3c

Comment 11

[BMO Github Automation]

Authored by https://github.com/dshin-moz
https://github.com/mozilla/enterprise-firefox/commit/5180854d6536e98dbc21901e35a3931961c978b8
[enterprise-main] Bug 2014220: Propagate up NS_FRAME_CONTAINS_RELATIVE_BSIZE from items in nested flex containers. r=layout-reviewers,TYLin

Comment 12

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

Upstream PR merged by moz-wptsync-bot