Closed Bug 1308502 Opened 3 years ago Closed 3 years ago

lines do not wrap at space when it is followed by zwj or combining mark

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox52 --- fixed

People

(Reporter: eusgf4u4pw, Assigned: jfkthame)

Details

Attachments

(6 files)

Attached file WrapTest.htm
User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/53.0.2785.143 Safari/537.36

Steps to reproduce:

1) Open WrapTest.htm (attached)
2) Narrow the window until lines in table begin to wrap.
3) Keep making the window narrower until all 4 lines should wrap

Tested on Windows; Same results in release 49.0.1 and nightly 52.0a1 (2016-10-06) (64-bit)


Actual results:

Second two lines wrap; First two lines never wrap even though there are multiple U+0020 SPACE chars in them.

Same results show in text that is not in a table, as demonstrated in the 2nd half of the file.


Expected results:

All four lines in the table should wrap when the table is narrow enough. Similarly, all four lines below the table should wrap when the window is narrow enough.
Attached image Expected Result
Attached image Actual Result
The lines that do not wrap contain spaces, but every space is followed by (in the first line) a zero width joiner or (in the second line) a combining mark.
While such long sequences are unlikely in the wild, the fact that space+zwj is not seen as an opportunity for line breaking would cause Firefox to choose an earlier break opportunity, making for an overly-short line in that paragraph.
Yes, I think this is incorrect behavior. According to my reading of http://unicode.org/reports/tr14/, there should be a break opportunity following the <space> in a sequence <space, zwj> or <space, combining-mark>.

(Note that there is a legacy behavior, described in http://unicode.org/reports/tr14/#LegacySpace, where there would NOT be a break between the space and mark here; but even then, our behavior would not be correct, because in that tailoring the combination <space, mark> is supposed to be treated as ID (like an ideographic character), which would allow a break on either side. But we fail to break at all.)

I think the immediate cause of the problem is that we form "clusters" in the textrun, where trailing combining marks and join controls become part of the same cluster as their preceding base -- which is important for things like letter-spacing and justification -- but then we don't allow any line-breaks within a cluster, which in this case should be permitted.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Component: Untriaged → Layout: Text
Product: Firefox → Core
Version: 49 Branch → unspecified
Here are a couple of testcases (currently failing) for this issue in reftest form.
Attachment #8799133 - Flags: review?(m_kato)
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
Patch that fixes the problem. If/when we eventually replace our line-break code with an ICU-based implementation, we may no longer need this, but for now I think it's the simplest/safest fix.
Attachment #8799135 - Flags: review?(m_kato)
Attachment #8799135 - Flags: review?(m_kato) → review+
Attachment #8799133 - Flags: review?(m_kato) → review+
Attachment #8799136 - Flags: review?(m_kato) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/c99f2fb3dc50e2aae4710aa93674e170ff2a6be7
Bug 1308502 - Add reftests for line-breaking at clusters having <space> as their base. r=m_kato

https://hg.mozilla.org/integration/mozilla-inbound/rev/f1b5a733bc5467719caf44bdc18a9318b25c80b3
Bug 1308502 - Allow line-break even within a cluster if the preceding character (i.e. base of cluster) is a space. r=m_kato

https://hg.mozilla.org/integration/mozilla-inbound/rev/ffcf738fc20023b939afec0ced3be7b209573155
Bug 1308502 followup, add missing const-ness to gfxTextRun::SetPotentialLineBreaks param, tidy up a bit. r=m_kato
https://hg.mozilla.org/mozilla-central/rev/c99f2fb3dc50
https://hg.mozilla.org/mozilla-central/rev/f1b5a733bc54
https://hg.mozilla.org/mozilla-central/rev/ffcf738fc200
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in before you can comment on or make changes to this bug.