1725973 - Percentage flex-basis under-invalidation

Status: VERIFIED FIXED

(Core :: Layout: Flexbox, defect)

Description

[Ian Kilpatrick [:iank]]

Attachment: c.html

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/92.0.4515.131 Safari/537.36

Steps to reproduce:

See attached file. Currently there is a %-height-descendant under-invalidation issue.

If a flex-item in a column flex-box has a %-flex-basis, e.g. "flex: 1 0 100%;" it currently doesn't have a 2nd layout pass if the definiteness changes.

This has a different rendering if there is a sibling which is a %-height-descentant.
(In the example I added "max-height: 1000%;" to a sibling to trigger the 2nd pass, and show the difference).

Actual results:

The two examples don't render the same.

Expected results:

The two examples should render the same.

Comment 1

[Ian Kilpatrick [:iank]]

Just also realized that if you do a dynamic change to remove the "max-height: 1000%" it'll render differently, while the DOM trees are the same.

Ian

Comment 2

[Daniel Holbert [:dholbert]]

Looks like this is a regression. In old builds, the left part of the testcase is tall like the right part of the testcase (and like in Chrome).

Regression range:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=fb2b08c6bc8c320127586fcedb2626e496bf3dcf&tochange=ee89deeb4fb6d12456c551c5ab4aaae151ceedae

--> Probably regression from bug 1492538.

Comment 3

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

Attachment: Bug 1725973 Part 1 - Add a FLEX_LOG in MoveFlexItemToFinalPosition(). r?dholbert

Make it easier to spot a flex item is being moved to the final position
rather than going through the final reflow.

Comment 4

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

Attachment: Bug 1725973 Part 2 - Mark flex items with percentage flex-basis in column flex container as depending on CB's block-size. r?dholbert

If so, dependsOnCBBSize will be set to true, and later in
ReflowInput::InitResizeFlags() we will add
NS_FRAME_CONTAINS_RELATIVE_BSIZE to the appropriate ancestor.

Sorted the #include statements in ReflowInput.cpp because I added
nsFlexContainerFrame.h.

Depends on D123702

Comment 5

[Daniel Holbert [:dholbert]]

Thanks for fixing this, TYLin!

Side note (not of particular importance, just an observation): the WPT test here (https://wpt.fyi/results/css/css-flexbox/flex-basis-011.html ) isn't currently included in the compat2021 set:
https://github.com/Ecosystem-Infra/wpt-results-analysis/blob/6d50d72f7c977b7750b53c844a6390db81f0c717/compat-2021/css-flexbox-tests.txt#L453
That's presumably just because this test was only added to WPT ~recently, in May[1], vs. the compat2021 "css-flexbox-tests.txt" list was most recently updated before that, in February.

[1] via https://github.com/web-platform-tests/wpt/commit/9bc1ce9ce46196179a01d238a3df9226bbc2e6be

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3bbec943b9a4
Part 1 - Add a FLEX_LOG in MoveFlexItemToFinalPosition(). r=dholbert
https://hg.mozilla.org/integration/autoland/rev/4fcd1b02c5bd
Part 2 - Mark flex items with percentage flex-basis in column flex container as depending on CB's block-size. r=dholbert

Comment 7

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/3bbec943b9a4
https://hg.mozilla.org/mozilla-central/rev/4fcd1b02c5bd

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Is this something we should consider taking on ESR91?

Comment 9

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

Thanks for the ping! This bug is an edge case and I'm not aware of any webcompat report, so it should be fine to let it ride the train.

Comment 10

[Ciprian Georgiu, Desktop QA]

I've reproduced this issue on an affected Nightly build (2021-08-16), using the test case from comment 0.

The issue is verified as fixed RC 93.0 (20210927210923) across platforms: Win 11 x64, macOS 11, Ubuntu 18.x64.