Closed Bug 1549728 Opened 4 months ago Closed 4 months ago

word-break: break-all does not work when applied to a fragment within a word

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla68
Tracking Status
firefox68 --- fixed

People

(Reporter: jfkthame, Assigned: jfkthame)

References

Details

Attachments

(1 file)

Follow-up to bug 1507744: the patch there fixes word-break:break-all when applied to an inline element (e.g. span) where there are word boundaries (whitespace) surrounding, but it still doesn't work correctly when the element boundaries fall within a larger word:

data:text/html,<div style="font-family:monospace;width:5ch">abcd<span style="word-break:break-all">xxxx</span>efgh

renders all on one line, although the fragment with word-break:break-all should have provided a usable break position.

(See also currently-failing WPT testcases at css/css-text/word-break/word-break-break-all-inline-* being added in bug 1507744.)

Digging in to this a bit more after the initial patch in bug 1507744, I think there's a better solution. We don't actually care about whether the text run construction spans the inline boundary; what's important is the word-break setting passed to the nsLineBreaker, and the extent of the text that it treats as a "word" for the purpose of marking potential break positions. Any given "word" (generally, space-delimited run of characters) will be handled using the mWordBreak setting of the line-breaker.

The line-breaker accumulates text independently of where text-run boundaries occur, so that even if, say, a single letter in the middle of a word is <b>bolded</b>, the word will be handled as a single unit for line-break computation. This is important in order for things like hyphenation to work properly.

So what we need to do here is to check for when the word-break value changes, and at that point, flush any "current word" the line-breaker is accumulating, independently of whether there's a text-run break. Then we can reset line-breaker's mWordBreak for the next fragment.

By not interrupting the text run when word-break changes, we'll avoid disrupting any shaping such as ligatures or kerning that might apply across the inline boundary.

(This is still not strictly perfect, as it means word-break changes within a word will interfere with hyphenation of that word. In practice, though, that's unlikely to be an issue: word-break is primarily used to control breaking in CJK text, where hyphenation isn't used anyway. And its other value word-break:break-all, which is sometimes used with Latin and similar scripts, makes hyphenation superfluous.)

This allows us to revert part of the change from bug 1507744: we don't actually need to break text runs
across an inline boundary when word-break changes, so BuildTextRunsScanner::ContinueTextRunAcrossFrames
can ignore this property. Flushing the "current word" being accumulated by the line-breaker is independent
of whether the text run continues across the inline boundary.

The patch above fixes a couple of the failing tests added in bug 1507744, where a span with word-break:break-all begins without a preceding space.

There are still two remaining test failures, but those depend on behavior that is unclear to me in the spec; I've opened https://github.com/w3c/csswg-drafts/issues/3897 to seek clarification. The tests as currently written assume Safari's behavior is correct, but I actually think the behavior we get with the patch here is preferable and should be adopted; if the CSS WG can be persuaded of that, the tests can be updated accordingly and will pass in Firefox with this patch.

Attachment #9063327 - Attachment description: Bug 1549728 - Flush line-breaker rather than interrupting text runs whenever the word-break property changes. r=emilio → Bug 1549728 - Flush line-breaker whenever the word-break property changes. r=emilio
Pushed by jkew@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e663519fd15d
Flush line-breaker whenever the word-break property changes. r=emilio
Status: NEW → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
Assignee: nobody → jfkthame
You need to log in before you can comment on or make changes to this bug.