1902156 - Anonymous flex item gets left behind with just whitespace inside of it, when linewrapped text gets removed from flex container

Status: VERIFIED FIXED

(Core :: Layout: Flexbox, defect)

Description

[Daniel Holbert [:dholbert]]

Spinning this off from bug 1901233, for the unusual situation that we ran into there.

Adding and then removing a text node in a flex container will cause us to add and then remove an anonymous flex item wrapper-box -- that's what's supposed to happen, at least. BUT: if the text is linewrapped, we fail to remove the anonymous flex item, for some reason.

The straggling anon box can cause trouble because (a) it violates invariants (which caused a crash in bug 1901233) and (b) it influences how packing space is distributed, e.g. if the flex container uses justify-content:space-around or gap to distribute packing space around flex items.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Seems this could be a regression from bug 1850834? I'm afk for the day, but if you can check and ni? me if so, I can look.

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: testcase 1

Comment 3

[Daniel Holbert [:dholbert]]

It is indeed! Thanks.

Testing Nightlies from the day before vs. the day after that bug's patch merged to central:

• Nightly 2024-06-03 gives "good" results on the attached testcase (lime square)
• Nightly 2024-06-05 gives "bad" results on the attached testcase (mostly-red square)

Comment 4

[Daniel Holbert [:dholbert]]

Also, warning, the attached testcase might cause a crash in devtools code, in nightlies from around 2024-06-05 to 2024-06-11 or so, if you try to inspect the <div id="flex"> node with the "Layout" devtools pane open. (That crash is bug 1901233, which we've made into a more graceful silent failure via the patch that landed over there.)

Comment 5

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

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

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1902156 - Account for continuations to remove anonymous wrappers. r=dholbert,TYLin

Since RemoveFrame also removes them and would leave the wrapping box
empty.

Comment 7

[Daniel Holbert [:dholbert]]

Attachment: testcase 2 (with whitespace that gets left behind)

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0018551e3c86
Account for continuations to remove anonymous wrappers. 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/46746 for changes under testing/web-platform/tests

Comment 10

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/0018551e3c86

Comment 11

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

Upstream PR merged by moz-wptsync-bot

Comment 12

[Daniel Holbert [:dholbert]]

ni as reminder to request uplift in a few days

Comment 13

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9407181 [details]
Bug 1902156 - Account for continuations to remove anonymous wrappers. r=dholbert,TYLin

Beta/Release Uplift Approval Request

• User impact if declined: Incorrect layout in some cases
• 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: comment 0
• List of other uplifts needed: none
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Trivialish fix.
• String changes made/needed: none
• Is Android affected?: Yes

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9407181 [details]
Bug 1902156 - Account for continuations to remove anonymous wrappers. r=dholbert,TYLin

Approved for 128.0b4.

Comment 15

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/bc185cd0cb3b

Comment 16

[Peter Magyari (Desktop QA)]

Verified fixed using Nightly 129.0a1 (20240616215431) and Beta 128.0b4 (20240615190628) on Windows 10, MacOS 14 and Ubuntu 24.04.