Bug 197430 - [css-text] break-all shouldn't allow breaking opportunities before a slash character
Summary: [css-text] break-all shouldn't allow breaking opportunities before a slash ch...
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Text (show other bugs)
Version: Safari Technology Preview
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-04-30 13:05 PDT by Javier Fernandez
Modified: 2019-05-08 13:31 PDT (History)
2 users (show)

See Also:


Attachments
Test case to reproduce the issue (158 bytes, text/html)
2019-04-30 13:05 PDT, Javier Fernandez
no flags Details
Test case to reproduce the issue (159 bytes, text/html)
2019-04-30 13:08 PDT, Javier Fernandez
no flags Details
Actual result (1.05 KB, image/png)
2019-04-30 13:11 PDT, Javier Fernandez
no flags Details
Expected result (1.06 KB, image/png)
2019-04-30 13:12 PDT, Javier Fernandez
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Javier Fernandez 2019-04-30 13:05:50 PDT
Created attachment 368592 [details]
Test case to reproduce the issue

What steps will reproduce the problem?
(1) Load the attached test case

What is the expected result?

The line is broken before the last letter before the slash "X|X/X" (see expected.png)

What happens instead?

The line is broken before the slash character "XX|/X" (see actual.png)
Comment 1 Javier Fernandez 2019-04-30 13:08:48 PDT
Created attachment 368595 [details]
Test case to reproduce the issue
Comment 2 Javier Fernandez 2019-04-30 13:11:56 PDT
Created attachment 368597 [details]
Actual result
Comment 3 Javier Fernandez 2019-04-30 13:12:33 PDT
Created attachment 368598 [details]
Expected result
Comment 4 Javier Fernandez 2019-04-30 13:15:20 PDT
According to the unicode rules UAX#14 breaking opportunities around slash characters are only possible after, not before, the character. 

https://www.unicode.org/reports/tr14/#SY

"The SY line breaking property is intended to provide a break opportunity after, except in front of digits, so as to not break “1/2” or “06/07/99”."
Comment 5 Myles C. Maxfield 2019-05-06 17:55:07 PDT
Looks like our behavior matches Chrome's but not Firefox's.
Comment 6 Myles C. Maxfield 2019-05-06 17:57:03 PDT
(In reply to Myles C. Maxfield from comment #5)
> Looks like our behavior matches Chrome's but not Firefox's.

Whoops, I was mixed up. Our behavior matches Firefox's but not Chrome's.
Comment 7 Myles C. Maxfield 2019-05-06 17:59:23 PDT
There's a big comment about this in the table in BreakLines.cpp that refers to https://bugs.webkit.org/show_bug.cgi?id=37698
Comment 8 Javier Fernandez 2019-05-07 03:32:34 PDT
(In reply to Myles C. Maxfield from comment #7)
> There's a big comment about this in the table in BreakLines.cpp that refers
> to https://bugs.webkit.org/show_bug.cgi?id=37698

I still think Chrome's behavior is correct and this bug report is valid, but   admit I've got doubts, so I'd appreciate any feedback you can provide so that I can understand the issue better.

I could try to file a bug in Firefox, if there isn't any already, so that we can gather feedback. It'd be good to improve interoperability in these kind of cases.
Comment 9 Myles C. Maxfield 2019-05-08 13:31:26 PDT
Looks like Chrome has the same table that we do, except for a difference in one line:

https://trac.webkit.org/browser/webkit/trunk/Source/WebCore/rendering/BreakLines.cpp#L53

https://github.com/chromium/chromium/blob/master/third_party/blink/renderer/platform/text/text_break_iterator.cc#L124

The line that's different is:

{ B(1, 1, 1, 1, 1, 1, 1, 1), B(1, 1, 1, 0, 1, 0, 1, 0), 0, B(0, 1, 1, 1, 1, 1, 1, 1), F, F, F, B(1, 1, 1, 1, 1, 1, 1, 1), F, F, F, B(1, 1, 1, 1, 1, 1, 1, 1) }, // - Note: breaking before '0'-'9' is handled hard-coded in shouldBreakAfter().

vs

{ B(0, 1, 1, 0, 1, 1, 1, 1), B(0, 1, 1, 0, 1, 0, 0, 0), 0, B(0, 0, 0, 1, 1, 1, 0, 1), F, F, F, B(1, 1, 1, 1, 0, 1, 1, 1), F, F, F, B(1, 1, 1, 1, 0, 1, 1, 1) }, // - Note: breaking before '0'-'9' is handled hard-coded in shouldBreakAfter().

It looks like the Chrome line was modified in:

https://codereview.chromium.org/2490153002
https://codereview.chromium.org/1370203005

and the WebKit line was modified in:

https://bugs.webkit.org/show_bug.cgi?id=185899