Bug 219784

Summary: [LFC][IFC] tabCharacter and newlineCharacter are also word separators when not preserved
Product: WebKit Reporter: zalan <zalan>
Component: Layout and RenderingAssignee: zalan <zalan>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, darin, koivisto, simon.fraser, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 219706    
Attachments:
Description Flags
Patch
koivisto: review+
Patch
none
Patch none

Description zalan 2020-12-11 08:10:15 PST
I can't find it here though https://drafts.csswg.org/css-text-3/#propdef-word-spacing
Comment 1 zalan 2020-12-11 08:15:17 PST
Created attachment 415998 [details]
Patch
Comment 2 Antti Koivisto 2020-12-11 08:18:20 PST
Comment on attachment 415998 [details]
Patch

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

> Source/WebCore/layout/inlineformatting/InlineTextItem.cpp:109
>                      isWordSeparator = text[startPosition] == space
> +                        || text[startPosition] == tabCharacter
>                          || text[startPosition] == noBreakSpace
>                          || text[startPosition] == ethiopicWordspace
>                          || text[startPosition] == aegeanWordSeparatorLine

lambda?
Comment 3 zalan 2020-12-11 09:15:48 PST
Apparently I confused it with the case when the tab character is resolved to a single space character (non-preserve case).
Comment 4 zalan 2020-12-11 15:22:56 PST
let's repurpose this bug
Comment 5 zalan 2020-12-11 15:34:20 PST
Created attachment 416055 [details]
Patch
Comment 6 Antti Koivisto 2020-12-12 02:13:57 PST
Comment on attachment 416055 [details]
Patch

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

> Source/WebCore/layout/inlineformatting/InlineTextItem.cpp:120
> +                    return text[startPosition] == space
>                          || text[startPosition] == noBreakSpace
>                          || text[startPosition] == ethiopicWordspace
>                          || text[startPosition] == aegeanWordSeparatorLine
>                          || text[startPosition] == aegeanWordSeparatorDot
>                          || text[startPosition] == ugariticWordDivider;

maybe just put the text[startPosition] into a local?
Comment 7 zalan 2020-12-12 11:31:18 PST
Created attachment 416101 [details]
Patch
Comment 8 EWS 2020-12-12 23:03:31 PST
Committed r270747: <https://trac.webkit.org/changeset/270747>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 416101 [details].
Comment 9 Radar WebKit Bug Importer 2020-12-12 23:04:16 PST
<rdar://problem/72265348>
Comment 10 Darin Adler 2020-12-17 17:55:55 PST
Comment on attachment 416055 [details]
Patch

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

>> Source/WebCore/layout/inlineformatting/InlineTextItem.cpp:120
>>                          || text[startPosition] == ugariticWordDivider;
> 
> maybe just put the text[startPosition] into a local?

Or make another isWordSeparator function/overload that just takes a character?
Comment 11 zalan 2021-02-23 19:24:22 PST
(In reply to Darin Adler from comment #10)
> Comment on attachment 416055 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=416055&action=review
> 
> >> Source/WebCore/layout/inlineformatting/InlineTextItem.cpp:120
> >>                          || text[startPosition] == ugariticWordDivider;
> > 
> > maybe just put the text[startPosition] into a local?
> 
> Or make another isWordSeparator function/overload that just takes a
> character?
I was just about to make this change but this code got moved over to "moveToNextNonWhitespacePosition" where I actually introduced a function for this

...
    auto isWhitespaceCharacter = [&](auto character) {
        // white space processing in CSS affects only the document white space characters: spaces (U+0020), tabs (U+0009), and segment breaks.
...