Closed Bug 1827582 Opened 2 years ago Closed 1 years ago

Tab becomes unresponsive on page load

Categories

(Core :: Layout: Flexbox, defect)

defect

Tracking

()

VERIFIED FIXED
114 Branch
Tracking Status
firefox-esr102 --- unaffected
firefox112 --- unaffected
firefox113 --- verified
firefox114 --- verified

People

(Reporter: cg+zbmvyynohtmvyyn, Assigned: TYLin)

References

(Regression)

Details

(Keywords: regression)

Attachments

(2 files)

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

Keywords: regression
Regressed by: 1743890

: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.

Flags: needinfo?(aethanyc)

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

Status: UNCONFIRMED → NEW
Ever confirmed: true

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

Severity: -- → S2

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

Assignee: nobody → aethanyc
Status: NEW → ASSIGNED
Flags: needinfo?(aethanyc)
Attached file 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.

Regressions: 1828977
Pushed by tlin@mozilla.com: 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
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/39612 for changes under testing/web-platform/tests
Status: ASSIGNED → RESOLVED
Closed: 1 years ago
Resolution: --- → FIXED
Target Milestone: --- → 114 Branch
Upstream PR merged by moz-wptsync-bot

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
Attachment #9328651 - Flags: approval-mozilla-beta?
Flags: qe-verify+
QA Whiteboard: [qa-triaged]

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 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.

Attachment #9328651 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

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

Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: