Bug 181169 – Implement "line-break: anywhere"

• Component: Text
• Status: RESOLVED FIXED

Created attachment 330206 [details]
Sample of the line break.

In the situation of a left quotation appearing in the last of a line, the left quotation will wrap to a new line with one character before it. But in Chinese text, a left quotation appearing in the front of a line is allowed. Same as above, a right quotation should be able to appear in the last of a line.

[Myles C. Maxfield]

This behavior is compliant with the spec and interoperable (to some degree) with all the major browsers on the Mac (I haven't tested Edge to know if it behaves the same way, but I would guess that it would). In addition, this is consistent with the system editing behavior for native apps (like TextEdit).

You probably want to style your content with "line-break: anywhere" which isn't implemented in WebKit yet. As soon as we implement that (or if it gets implemented), adding that style to your content will cause it to have the behavior you desire.

Until then, you can work around this bug by adding "word-break: break-all" which currently, erroneously, has the behavior you desire. However, we want to fix "line-break: break-all" to have your undesirable behavior (so that it is spec-compliant).

Because "line-break: anywhere" is at risk[1] in the CSS Text spec, it is possible that the spec may change in the future, so it probably wouldn't be a good idea to blindly add it to your content right now.

[1] https://drafts.csswg.org/css-text-3/

[Javier Fernandez]

I started to work on this feature for Chrome; if nobody does it before, I may eventually implement it for WebKit, so I'd appreciate feedback on the interest of webkit on the feature.

[Radar WebKit Bug Importer]

<rdar://problem/48507088>

[Myles C. Maxfield]

(In reply to Javier Fernandez from comment #2)
> I started to work on this feature for Chrome; if nobody does it before, I
> may eventually implement it for WebKit, so I'd appreciate feedback on the
> interest of webkit on the feature.

We are definitely interested! :)

[Javier Fernandez]

Created attachment 368610 [details]
Patch

[Javier Fernandez]

Created attachment 368620 [details]
Patch

Patch rebased

[Myles C. Maxfield]

Comment on attachment 368620 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=368620&action=review

> Source/WebCore/rendering/BreakLines.h:194
> +    NonSharedCharacterBreakIterator iterator(stringView);

Is this really the correct type of iterator here?

There's likely a significant performance opportunity here, we should probably file a new bug about using a shared iterator and add a FIXME here.

> Source/WebCore/rendering/BreakLines.h:206
> +    if (breakAnywhere) {
> +        nextBreakable = nextBreakablePositionBreakCharacter(lazyBreakIterator, startPosition);
> +    } else if (keepAllWords) {

Didn't the style checker catch the unnecessary { } ?

Do we need to have two variants for the NBSP behavior?

[Javier Fernandez]

Comment on attachment 368620 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=368620&action=review

>> Source/WebCore/rendering/BreakLines.h:194
>> +    NonSharedCharacterBreakIterator iterator(stringView);
> 
> Is this really the correct type of iterator here?
> 
> There's likely a significant performance opportunity here, we should probably file a new bug about using a shared iterator and add a FIXME here.

This is the approach Chrome is following now, although it implemented some optimizations to avoid iterating through the whole text line's length every time the function is called. 

I could definitively evaluating the Shared iterator approach, but I think I'd need some guidance.

>> Source/WebCore/rendering/BreakLines.h:206
>> +    } else if (keepAllWords) {
> 
> Didn't the style checker catch the unnecessary { } ?
> 
> Do we need to have two variants for the NBSP behavior?

Ups, I'm not sure, but I'll fix it before landing.

breakAnywhere should be include a wider range of breaking opportunities than just NBSP; do you mean that I should refactor the breakNBSP case and this one ?

[Myles C. Maxfield]

> breakAnywhere should be include a wider range of breaking opportunities than
> just NBSP; do you mean that I should refactor the breakNBSP case and this
> one ?

I just mean the code directly below has a bunch of "if (breakNBSP)" blocks. Do we need the same thing here?

[Javier Fernandez]

(In reply to Myles C. Maxfield from comment #9)
> > breakAnywhere should be include a wider range of breaking opportunities than
> > just NBSP; do you mean that I should refactor the breakNBSP case and this
> > one ?
> 
> I just mean the code directly below has a bunch of "if (breakNBSP)" blocks.
> Do we need the same thing here?

My understanding of current code and what the breakNBSP condition would imply is that breakAnywhere assume always that condition as true. 

The line-break:anywhere feature indeed allows to break "non-breakable spaces"; but as I commented before, it allows breaking on other usually unbreakable characters as well. Hence, I decided to implement this logic in a new function, which doesen't depend on this breakNBSP condition.

[Javier Fernandez]

Created attachment 369832 [details]
Patch

Patch rebased

[WebKit Commit Bot]

Comment on attachment 369832 [details]
Patch

Clearing flags on attachment: 369832

Committed r245275: <https://trac.webkit.org/changeset/245275>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.