Closed Bug 1805597 Opened 2 years ago Closed 2 years ago

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

Categories

(Core :: Layout: Floats, defect)

Firefox 104
defect

Tracking

()

VERIFIED FIXED
112 Branch
Webcompat Priority ?
Tracking Status
firefox-esr102 --- unaffected
firefox108 --- wontfix
firefox109 --- wontfix
firefox110 --- wontfix
firefox111 + verified
firefox112 --- verified

People

(Reporter: cpeterson, Assigned: TYLin)

References

(Regression)

Details

(Keywords: regression)

Attachments

(6 files)

Attached file test.html (deleted) —

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

Flags: needinfo?(aethanyc)
Summary: Text is right justified and overlaps text in Firefox, but left justified in Chrome → Text is right justified and overlaps text in Firefox, but left justified in Chrome and Safari
Attached image Chrome_screenshot.png (deleted) —

Screenshot of expected behavior in Chrome

Attached image Firefox_screenshot.png (deleted) —

Screenshot of new buggy behavior in Firefox 104+

Version: unspecified → Firefox 104
Severity: -- → S3

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

I can reproduce on macOS 13.0 and 13.1.

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

Webcompat Priority: --- → ?

@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?

Performance Impact: --- → ?

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

Tracking for our 110 release given the webcompat impact.

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

Flags: needinfo?(cpeterson)
Attached file testcase 2 (deleted) —

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.

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

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

Performance Impact: ? → ---

(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

Flags: needinfo?(cpeterson)

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.

Flags: needinfo?(fgriffith)

(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?

Flags: needinfo?(fgriffith) → needinfo?(pascalc)

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.

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

Assignee: nobody → aethanyc
Attached file testcase 3 (reduced from testcase 2) (deleted) —

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

Flags: needinfo?(aethanyc)

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

Attachment #9317270 - Attachment description: Bug 1805597 - Place floats with insufficient available inline-size below current line regarless of its clearance value. → Bug 1805597 - Place a float with insufficient available inline-size below current line regarless of its clearance value.
Pushed by aethanyc@gmail.com: 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
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/38474 for changes under testing/web-platform/tests
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 112 Branch
Upstream PR merged by moz-wptsync-bot

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

Flags: needinfo?(aethanyc)

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
Flags: needinfo?(aethanyc)
Attachment #9317270 - Flags: approval-mozilla-beta?

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

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

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

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

Attachment

General

Created:
Updated:
Size: