1870973 - New update (121.0) sometimes renders ideographic space (U+3000) oddly

Status: VERIFIED FIXED

(Core :: Layout: Text and Fonts, defect)

Description

[lutallium]

Attachment: ideographic_space.html

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:121.0) Gecko/20100101 Firefox/121.0

Steps to reproduce:

I was visiting the website Tenhou to play Mahjong as usual and I noticed some texts are rendered incorrectly. They're relevant to the special character ideographic space (U+3000), which is no longer handled correctly since the new update 121.0. Sometimes, if a string begins or ends with U+3000, it will get wrapped unexpectedly, which never happened before the update, and never happens in other browsers. Also in the DevTool inspector it behaves in this way.

The bug never occured before the update, as I tested on the old version 120.0.1. Therefore I assume the bug must be relevant to the newest update 121.0.

TO REPRODUCE: Open the attached minimal example ideographic_space.html with Fire Fox, which includes a button with text "　test xxx yyy zzz". The character at the beginning is an ideographic space (U+3000).

Actual results:

The button is rendered as

[　test xxx yyy
zzz ]

• with a strange line break.

https://ibb.co/1JS28Kj

Expected results:

The button is expected to be rendered as

[　test xxx yyy zzz]

• without any line break.

Comment 1

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

The Bugbug bot thinks this bug should belong to the 'Toolkit::Application Update' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[Ryan VanderMeulen [:RyanVM][PTO June 24-28]]

Hello reporter, if possible, can you please try to narrow down a regression range with the mozregression tool? It won't have any impact on your regular Firefox profile. Thanks!
https://mozilla.github.io/mozregression/

Comment 3

[Ryan VanderMeulen [:RyanVM][PTO June 24-28]]

Nevermind, the attached testcase reproduces for me too.
INFO: Last good revision: 0d91ef074019779822e1fa8c7887ba528cafe6af
INFO: First bad revision: 3ff9d913d60ad145b0ba96f2bb2a686902421f99
INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=0d91ef074019779822e1fa8c7887ba528cafe6af&tochange=3ff9d913d60ad145b0ba96f2bb2a686902421f99

Setting this as a regression from bug 1862249, but it looks like it might have been an intentional behavior change?

Comment 4

[Jonathan Kew [:jfkthame]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #3)

Nevermind, the attached testcase reproduces for me too.
INFO: Last good revision: 0d91ef074019779822e1fa8c7887ba528cafe6af
INFO: First bad revision: 3ff9d913d60ad145b0ba96f2bb2a686902421f99
INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=0d91ef074019779822e1fa8c7887ba528cafe6af&tochange=3ff9d913d60ad145b0ba96f2bb2a686902421f99

Setting this as a regression from bug 1862249, but it looks like it might have been an intentional behavior change?

No, it's true that bug 1862249 was related to ideographic space, but I don't think this was an expected result. I'll try to look into it, though if we don't find a quick fix we might want to consider backing out that patch.

Leaving ni? flag until I have a chance to look more closely...

Comment 5

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1870973 - Back out changeset 3ff9d913d60a (bug 1862249) due to line-wrapping regressions. r=#layout

This reverts the change to make ideographic space trimmable, because of unintended impact on line
breaking and measurement in other cases. We'll need a more carefully targeted fix for the original
issue in bug 1862249, as well as better test coverage when landing a new patch there.

(Given that we've had multiple regression reports about this, I think we should uplift the backout
to 122. We could even consider it for a 121.x dot-release if one happens, as it's just reverting
to longstanding behavior here.)

Comment 6

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

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

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/877aa1219c43
Back out changeset 3ff9d913d60a (bug 1862249) due to line-wrapping regressions. r=layout-reviewers,emilio

Comment 8

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/877aa1219c43

Comment 9

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1870973 - Back out changeset 3ff9d913d60a (bug 1862249) due to line-wrapping regressions.

This reverts the change to make ideographic space trimmable, because of unintended impact on line
breaking and measurement in other cases. We'll need a more carefully targeted fix for the original
issue in bug 1862249, as well as better test coverage when landing a new patch there.

(Given that we've had multiple regression reports about this, I think we should uplift the backout
to 122. We could even consider it for a 121.x dot-release if one happens, as it's just reverting
to longstanding behavior here.)

Original Revision: https://phabricator.services.mozilla.com/D197150

Comment 10

[Phabricator Automation]

Uplift Approval Request

• Explanation of risk level: clean backout of the regressing patch
• Needs manual QE test: no
• Fix verified in Nightly: no
• User impact if declined: unexpected line-wrap in some CJK contexts
• Is Android affected?: yes
• String changes made/needed: none
• Steps to reproduce for manual QE testing: n/a
• Code covered by automated testing: yes
• Risk associated with taking this patch: minimal

Comment 11

[Pulsebot]

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

Comment 12

[Ryan VanderMeulen [:RyanVM][PTO June 24-28]]

Hi Jonathan, please nominate this for Release approval as well for next week's planned dot release.

Comment 13

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1870973 - Back out changeset 3ff9d913d60a (bug 1862249) due to line-wrapping regressions.

This reverts the change to make ideographic space trimmable, because of unintended impact on line
breaking and measurement in other cases. We'll need a more carefully targeted fix for the original
issue in bug 1862249, as well as better test coverage when landing a new patch there.

(Given that we've had multiple regression reports about this, I think we should uplift the backout
to 122. We could even consider it for a 121.x dot-release if one happens, as it's just reverting
to longstanding behavior here.)

Original Revision: https://phabricator.services.mozilla.com/D197150

Comment 14

[Phabricator Automation]

Uplift Approval Request

• Is Android affected?: yes
• Needs manual QE test: no
• String changes made/needed: none
• User impact if declined: unexpected line-wrap in some CJK contexts
• Steps to reproduce for manual QE testing: n/a
• Code covered by automated testing: yes
• Fix verified in Nightly: no
• Risk associated with taking this patch: minimal
• Explanation of risk level: clean backout of the regressing patch

Comment 15

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-release/rev/634993c56c3f

Comment 17

[Ryan VanderMeulen [:RyanVM][PTO June 24-28]]

Added to the 121.0.1 relnotes:

Fixed unexpected line wrapping in some CJK contexts caused by changes in ideographic space handling

Comment 18

[Alexandru Trif, Desktop QA [:atrif]]

Reproduced the issue with Firefox 121.0 by using the attached test case from comment 0 on Windows 10x64. A line break is displayed on the button from the test case.
The issue is verified fixed with Firefox 123.0a1 (2024-01-08), 122.0b7 and Firefox 121.0.1 on Windows 10x64, macOS 12 and Ubuntu 22. No line break is displayed on the button from the test case.