1805597 - Text is right justified and overlaps text in Firefox, but left justified in Chrome and Safari

Status: VERIFIED FIXED

(Core :: Layout: Floats, defect)

Description

[Chris Peterson [:cpeterson]]

Attachment: test.html

In the test case, the account name “123 MAIN ST” is right justified and overlaps the account balance “$123.45”, but the account name is left justified in Chrome and Safari (macOS and iOS).

Unfortunately, the test case is on my bank website, but I whittled down the page's HTML to create a smaller test case that is ugly but still reproduces the bug.

Ting-Yu, I bisected this regression to a pushlog that points to your fix for writing-mode bug 1323517 in Firefox 104:

https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=205ee354f1982f2a4badc380bdeda0d0dbb09f41&tochange=d08bab0259f7e15564f2473d545c8fdb7511a025

Comment 1

[Chris Peterson [:cpeterson]]

Attachment: Chrome_screenshot.png

Screenshot of expected behavior in Chrome

Comment 2

[Chris Peterson [:cpeterson]]

Attachment: Firefox_screenshot.png

Screenshot of new buggy behavior in Firefox 104+

Comment 3

[David Shin[:dshin]]

(Small drive-by observation: The page looks ok on Ubuntu 20.04, but not on Windows for me. Both 2022-12-15 builds)

Comment 4

[Chris Peterson [:cpeterson]]

I can reproduce on macOS 13.0 and 13.1.

Comment 5

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

Chris, thanks for filing this bug and David for the test. I can reproduce on macOS and Windows, but not on Linux. (Keeping the NI as a reminder.)

Comment 6

[Jason Prickett]

@TYLin Given that this seems be on a financial website with a potentially wide impact, are there plans for this to be uplifted into 110 beta?

Comment 7

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

I haven't got a patch for this bug, so no worry about the uplift.

Comment 8

[Pascal Chevrel:pascalc]

Tracking for our 110 release given the webcompat impact.

Comment 9

[James Graham [:jgraham]]

:cpeterson can you file the site breakage on webcompat.com, and link the report here via See Also, so that we can track the site breakage there?

Comment 10

[Daniel Holbert [:dholbert]]

Attachment: testcase 2

Here's a reduced testcase (which could probably be reduced a bit further).

In Firefox versions after the regression range (I tested 2022-07-08 and current Nightly), the teal & orange boxes are right-aligned and intersect with the blue box.

In Chrome and in Firefox versions before the regression range (I tested 2022-07-06), the teal & orange boxes are left-aligned (the grey box is shifted lower to create space for them), and they don't intersect with the blue box.

Comment 11

[Daniel Holbert [:dholbert]]

(In reply to Ting-Yu Lin [:TYLin] (UTC-8) from comment #5)

I can reproduce on macOS and Windows, but not on Linux.

This was probably due to font-specific sizing differences, and the resulting impact on the layout.

Testcase 2 just uses boxes with non-font-specific sizes, and it reproduces the issue for me on macOS and Linux at least. (I didn't test Windows but I assume it repro's there too.)

Comment 12

[Daniel Holbert [:dholbert]]

(There's also no performance impact here, i.e. no CPU spikes or hangs or battery-usage issues. --> Clearing that flag.)

Comment 13

[Chris Peterson [:cpeterson]]

(In reply to James Graham [:jgraham] from comment #9)

:cpeterson can you file the site breakage on webcompat.com, and link the report here via See Also, so that we can track the site breakage there?

ok. I filed https://github.com/webcompat/web-bugs/issues/117602

Comment 14

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

The bug is marked as tracked for firefox110 (beta). We have limited time to fix this, the soft freeze is in 9 days. However, the bug still isn't assigned and has low severity.

:fgriffith, could you please find an assignee and increase the severity for this tracked bug? Given that it is a regression and we know the cause, we could also simply backout the regressor. If you disagree with the tracking decision, please talk with the release managers.

For more information, please visit auto_nag documentation.

Comment 15

[Daniel Holbert [:dholbert]]

(In reply to Release mgmt bot [:suhaib / :marco/ :calixte] from comment #14)

:fgriffith, could you please find an assignee and increase the severity for this tracked bug? Given that it is a regression and we know the cause, we could also simply backout the regressor. If you disagree with the tracking decision, please talk with the release managers.

[stealing Frank's nagbot-needinfo here]

Given that the regressor landed 7 months ago (and fixed rendering bugs / interop issues, per the various .ini test-failure-annotations removed in its final patch), it's unlikely that we'd back it out at this point (and it might not be possible to back it out cleanly.

This bug is also not really a showstopper for 110, since we've been shipping the current behavior since v104. And when we do have a fix, it feels like it'd be risky to uplift to beta at this point in the release cycle; floats are fiddly and can have all sorts of unforeseen edge cases. So (when we have a fix) I'd prefer to let this ride the trains, or uplift early in the beta cycle to be sure it's properly baked.

Would be great to fix soon, of course; and this is already on TYLin's needinfo queue; hopefully he can take a look soon, or hand it off if he doesn't have cycles to look.

So: I suggest we remove the tracking flag for 110 and assume this'll be wontfix for 110, and plan on tracking this for 111 instead. Pascal, does that sound OK to you?

Comment 16

[Pascal Chevrel:pascalc]

OK for me.

Comment 17

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

This is a reminder regarding comment #14!

The bug is marked as tracked for firefox111 (nightly). We have limited time to fix this, the soft freeze is in 3 days. However, the bug still isn't assigned.

Comment 18

[Daniel Holbert [:dholbert]]

(The bot got a little confused; tracking status has changed since comment 14. However: I think TYLin is planning to take a look at this, so I'll assign it to him to appease the bot & help prevent this from getting missed. If this misses the soft freeze, it'd likely be a good candidate for uplift during the early-beta period, too.)

Comment 19

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

Attachment: testcase 3 (reduced from testcase 2)

Re comment 10:

Here's a reduced testcase (which could probably be reduced a bit further).

A reduced testcase like testcase 2 helps a lot when identifying the root cause. Thank you Daniel! I've found a simpler testcase (as attached).

Comment 20

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

Attachment: Bug 1805597 - Place a float with insufficient available inline-size below current line regarless of its clearance value.

This bug is a regression of Bug 1323517. This patch fixes the bug by restoring
the behavior to what it was prior to Bug 1323517 Part 7 [1], i.e. a float
element with insufficient available inline-size will be placed below the current
line, regardless of its clearance value. The old logic is here [2] .

floats-placement-008.html already passes with current Nightly, but it triggers
this assertion [3] while exploring a solution for this bug. I feel it is worth
to include it as a test.

[1] https://hg.mozilla.org/mozilla-central/rev/d08bab0259f7
[2] https://searchfox.org/mozilla-central/rev/b9cb8817f510057021874627487c6c14f69287c3/layout/generic/BlockReflowState.cpp#588-589
[3] https://searchfox.org/mozilla-central/rev/b25ff1fab82c2d3a91531ad3735e50422407b163/layout/generic/BlockReflowState.cpp#1026-1027

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/eaea6f52a564
Place a float with insufficient available inline-size below current line regarless of its clearance value. r=jfkthame,layout-reviewers

Comment 22

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

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

Comment 23

[Serban Stanca [:SerbanS]]

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

Comment 24

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

Upstream PR merged by moz-wptsync-bot

Comment 25

[Donal Meehan [:dmeehan]]

:TYLin could you add a beta uplift request on this when ready?

Comment 26

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

Comment on attachment 9317270 [details]
Bug 1805597 - Place a float with insufficient available inline-size below current line regarless of its clearance value.

Beta/Release Uplift Approval Request

• User impact if declined: Float elements with clearance value might overlap inline elements, e.g. the screenshot in comment 2.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The patch is a one line change to the condition to place the float elements when there are not enough space.
• String changes made/needed: none
• Is Android affected?: Yes

Comment 27

[Donal Meehan [:dmeehan]]

Comment on attachment 9317270 [details]
Bug 1805597 - Place a float with insufficient available inline-size below current line regarless of its clearance value.

Approved for 111.0b2

Comment 28

[Donal Meehan [:dmeehan]]

https://hg.mozilla.org/releases/mozilla-beta/rev/3b41b984530a

Comment 29

[Chris Peterson [:cpeterson]]

Verified fixed in Nightly 112 and Beta 111.0b2 using the original STR on my bank's website.