[LFC][IFC] tabCharacter and newlineCharacter are also word separators when not preserved

[zalan]

I can't find it here though https://drafts.csswg.org/css-text-3/#propdef-word-spacing

[zalan]

Created attachment 415998 [details]
Patch

[Antti Koivisto]

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?

[zalan]

Apparently I confused it with the case when the tab character is resolved to a single space character (non-preserve case).

[zalan]

let's repurpose this bug

[zalan]

Created attachment 416055 [details]
Patch

[Antti Koivisto]

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?

[zalan]

Created attachment 416101 [details]
Patch

[EWS]

Committed r270747: <https://trac.webkit.org/changeset/270747>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 416101 [details].

[Radar WebKit Bug Importer]

<rdar://problem/72265348>

[Darin Adler]

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?

[zalan]

(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.
...