RESOLVED FIXED Bug 181169
Implement "line-break: anywhere"
https://bugs.webkit.org/show_bug.cgi?id=181169
Summary Implement "line-break: anywhere"
c_erok
Reported 2017-12-26 19:39:52 PST
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.
Attachments
Sample of the line break. (68.77 KB, image/png)
2017-12-26 19:39 PST, c_erok
no flags
Patch (73.61 KB, patch)
2019-04-30 14:07 PDT, Javier Fernandez
no flags
Patch (72.62 KB, patch)
2019-04-30 15:34 PDT, Javier Fernandez
no flags
Patch (72.87 KB, patch)
2019-05-14 03:21 PDT, Javier Fernandez
no flags
Myles C. Maxfield
Comment 1 2017-12-26 21:42:54 PST
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
Comment 2 2019-03-01 04:15:04 PST
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
Comment 3 2019-03-01 04:17:31 PST
Myles C. Maxfield
Comment 4 2019-03-01 13:34:00 PST
(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
Comment 5 2019-04-30 14:07:17 PDT
Javier Fernandez
Comment 6 2019-04-30 15:34:07 PDT
Created attachment 368620 [details] Patch Patch rebased
Myles C. Maxfield
Comment 7 2019-05-06 17:49:16 PDT
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 8 2019-05-07 03:22:32 PDT
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
Comment 9 2019-05-08 13:06:32 PDT
> 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
Comment 10 2019-05-14 02:38:07 PDT
(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
Comment 11 2019-05-14 03:21:30 PDT
Created attachment 369832 [details] Patch Patch rebased
WebKit Commit Bot
Comment 12 2019-05-14 06:21:00 PDT
Comment on attachment 369832 [details] Patch Clearing flags on attachment: 369832 Committed r245275: <https://trac.webkit.org/changeset/245275>
WebKit Commit Bot
Comment 13 2019-05-14 06:21:01 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.