1827582 - Tab becomes unresponsive on page load

Status: VERIFIED FIXED

(Core :: Layout: Flexbox, defect)

Description

[Henri]

After the fix for bug 1743890 was committed, trying to load cdkeys.com causes the tab to become unresponsive. Switching to another tab and back may cause a spinner on gray backround to appear. Page never displays.

Regression found by bisecting with mozregression:
5:38.82 INFO: Last good revision: a5e0bf1c32f05bf8ca038ae759569742d565a20d
5:38.82 INFO: First bad revision: 6064a73fc99f012a7400393bdc2ade89fbdf30f8
5:38.82 INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=a5e0bf1c32f05bf8ca038ae759569742d565a20d&tochange=6064a73fc99f012a7400393bdc2ade89fbdf30f8

Running on Arch Linux 64bit, LxQt desktop on X11

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

:TYLin, since you are the author of the regressor, bug 1743890, could you take a look? Also, could you set the severity field?

For more information, please visit auto_nag documentation.

Comment 2

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

I can reproduce this issue. Trying to find a reduced testcase.

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1743890

Comment 4

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

Attachment: Bug 1827582 - Fix flex container's reflow status when unbreakable flex items consume all content block-size in current fragment. r?dholbert

The bug occurs in PopulateReflowOutput() when we change the reflow status of
an auto-height flex container to "complete" because unbreakable tall flex items
have consumed all the theoretical/unfragmented content block-size. Later in
PopulateReflowOutput(), we'll change the reflow status [1] to "overflow
incomplete," which cause the next-in-flow to become an overflow container.

It is possible that the container's block-size will grow due to pushed items.
However, nsSplittableFrame::CalcAndCacheConsumedBSize() [2] doesn't consider
the overflow container's block-size, so we never correctly consume the extra
block-size growth in later fragments due to pushed items. We end up creating
infinite columns.

[1] https://searchfox.org/mozilla-central/rev/54c533e94ae786056a43231f230c7d9b0773cb80/layout/generic/nsFlexContainerFrame.cpp#5623-5626
[2] https://searchfox.org/mozilla-central/rev/54c533e94ae786056a43231f230c7d9b0773cb80/layout/generic/nsSplittableFrame.cpp#201-204

Comment 5

[Daniel Holbert [:dholbert]]

Attachment: breaktest 1

Here's a "breaktest" inspired by my thoughts in https://phabricator.services.mozilla.com/D175543#inline-972121

For the "img and div" subtests here, we fail the !aAnyChildIncomplete check (i.e. we do have some child which is incomplete), and so that makes us decline to declare our flex container complete -- so we end up drawing our end border in the second column, in a build with this bug's patch applied.

In a build without the patch (e.g. current Firefox), we draw the border in the first column, which makes sense to me and is consistent with the "Just an img" subtests here.

This unexpected behavior-change, and the asymmetry (between tall-unfragmentable-thing-that-eats-all-our-remaining-height vs. that same thing plus a sibling we can fragment) suggests to me that we don't have the logic quite right here, and !aAnyChildIncomplete isn't quite the right check.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c6c27f64a655
Fix flex container's reflow status when unbreakable flex items consume all content block-size in current fragment. r=dholbert

Comment 7

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

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

Comment 8

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/c6c27f64a655

Comment 9

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

Upstream PR merged by moz-wptsync-bot

Comment 10

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

Comment on attachment 9328651 [details]
Bug 1827582 - Fix flex container's reflow status when unbreakable flex items consume all content block-size in current fragment. r?dholbert

Beta/Release Uplift Approval Request

• User impact if declined: Opening https://cdkeys.com hangs the browser, and the tab becomes unresponsive.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: Open https://cdkeys.com
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The change is small, and only applies to flexbox in multicol layout with a very small height.
• String changes made/needed: None
• Is Android affected?: Yes

Comment 11

[Giorgia Nichita, Release Desktop QA]

Reproduced this issue on an affected Nightly build from 2023-04-12, on Windows 10.
Verified as fixed on Firefox 114.0a1 (20230420212414) on Win 10 x65, macOS 10.14 and Ubuntu 20.04.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9328651 [details]
Bug 1827582 - Fix flex container's reflow status when unbreakable flex items consume all content block-size in current fragment. r?dholbert

Approved for 113.0b7.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/78aa5cc8856b

Comment 14

[Giorgia Nichita, Release Desktop QA]

Verified as fixed on Firefox 113.0b7 (20230423180101) on Win 10 x65, macOS 10.15 and Ubuntu 20.04.