628825 - Line breaking should not occur before punctuation, even when preceded by space

Status: RESOLVED FIXED

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

Description

[Vincent Lefevre]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X 10.4; en-GB; rv:1.9.2.13) Gecko/20101203 Firefox/3.6.13
Build Identifier: 

Firefox currently allows line breaking before a punctuation mark when it is preceded by a breakable space, such as a thin space U+2009 (as used in French). This is incorrect. The line breaking algorithm should be improved, for instance as described by:

  http://www.unicode.org/reports/tr14/

(Unicode Standard Annex #14 - Unicode Line Breaking Algorithm). In particular, see LB13: "Do not break before ‘]’ or ‘!’ or ‘;’ or ‘/’, even after spaces." Note that the list of cited characters is not exhaustive.

Reproducible: Always

Steps to Reproduce:
1. Open a page with a word followed by a thin space then by a punctuation mark such as "!" or "?" (see testcase).
2. Adjust the window width to see the various line breaking positions.
Actual Results:  
For some width, Firefox breaks the line before the punctuation mark.

Expected Results:  
Firefox should break the line before the word preceding it.

WebKit-based browsers have no such problem (tested only with a thin space).

The normal space should be handled as well, because some authors use it instead of a thin space (even with such a slightly incorrect typography, authors often don't use the typical no-break space, probably because a normal space is easier to type on a keyboard).

Comment 1

[Vincent Lefevre]

Attachment: testcase (HTML document with thin space before punctuation)

Comment 2

[(mostly gone) XtC4UaLL [:xtc4uall]]

See also Bug 56652 and Bug 206152 with its Dependencies.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Last I checked we purposefully allowed breaking before '/' because it's common for sites to depend on that.

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

How about the importance of this bug for Western language users? Is that a major format?

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Hmm, looks like the spaces are marked as breakable in nsLineBreaker::AppendText(). However, we need to access to nsILineBreaker's implementation when we check whether the next character is open punctuation or not...

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

s/open punctuation/close punctuation

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

http://mxr.mozilla.org/mozilla-central/source/intl/lwbrk/src/nsJISx4501LineBreaker.cpp#867

Now, GetJISx4051Breaks() always returns FALSE for first character. It might be usable. If the first character is not breakable even with a breakable character, we can set TRUE. Then, nsLineBreaker can know the first character's class of "word" and it can overwrite the previous space's state. But this is just my guess.

Comment 8

[Makoto Kato [:m_kato]]

As long as I test, this is already fixed by bug 1719535.

Comment 9

[Vincent Lefevre]

Attachment: screenshot

(In reply to Makoto Kato [:m_kato] from comment #8)

As long as I test, this is already fixed by bug 1719535.

In which FF version? According to bug 1719535, this seems to be fixed in 118, but with Firefox 118.0 under Linux, the bug still occurs. See attached screenshot for the testcase of this bug.

Comment 10

[Vincent Lefevre]

And I can still reproduce the bug with Nightly 120.0a1 (2023-10-01).

Comment 11

[Makoto Kato [:m_kato]]

(In reply to Vincent Lefevre from comment #10)

And I can still reproduce the bug with Nightly 120.0a1 (2023-10-01).

Thank you for testing. OK, I will reopen this

Comment 12

[Makoto Kato [:m_kato]]

data:text/html,<div style="width: 1px;">test&thinsp;!</div> seems to be simple test case. ICU4X can handle correctly, so this issue is layout side.

Comment 13

[Makoto Kato [:m_kato]]

Maybe ... https://searchfox.org/mozilla-central/rev/40d51bef58b8e901d6ab4b60dd280f372a0e417d/intl/lwbrk/LineBreaker.h#55-59

Comment 14

[Makoto Kato [:m_kato]]

Attachment: Bug 628825 - Don't break line by Break After. r=TYLin!,jfkthame!

Legacy segmenter breaks line by some Break After properties. But UAX#14 rule
doesn't break Break After (BA) at force.

So I would like to remove some legacy rules that isn't ASCII white space from
nsLineBreaker.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e8dbf59dd03b
Don't break line by Break After. r=TYLin,jfkthame

Comment 16

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

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

Comment 17

[Noemi Erli[:noemi_erli]]

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

Comment 18

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

Upstream PR merged by moz-wptsync-bot