Bug 227896 - PUA characters have the wrong advance in the fast text codepath
Summary: PUA characters have the wrong advance in the fast text codepath
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Myles C. Maxfield
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-07-12 23:03 PDT by Myles C. Maxfield
Modified: 2021-07-23 14:52 PDT (History)
7 users (show)

See Also:

Attachments
Patch (14.24 KB, patch)
2021-07-12 23:06 PDT, Myles C. Maxfield 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Myles C. Maxfield 2021-07-12 23:03:34 PDT 
PUA characters have the wrong advance in the fast text codepath
Comment 1 Myles C. Maxfield 2021-07-12 23:06:56 PDT 
Created attachment 433389 [details]
Patch
Comment 2 Myles C. Maxfield 2021-07-12 23:07:27 PDT 
<rdar://problem/80037103>
Comment 3 Tim Horton 2021-07-12 23:24:33 PDT 
Comment on attachment 433389 [details]
Patch

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

> Source/WebCore/platform/graphics/FontCascade.h:254
> +    static bool treatAsSpace(UChar32 c) { return c == ' ' || c == '\t' || c == '\n' || c == noBreakSpace; }

I'm confused why the silent truncation wasn't a compiler error??

> LayoutTests/fast/text/pua-charactersTreatedAsSpace-expected.html:16
> +This test passes if you see a particular pattern of black boxes below.

particular but indescribable?
Comment 4 EWS 2021-07-13 00:52:45 PDT 
Committed r279868 (239621@main): <https://commits.webkit.org/239621@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 433389 [details].
Comment 5 Darin Adler 2021-07-23 14:52:15 PDT 
Comment on attachment 433389 [details]
Patch

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

>> Source/WebCore/platform/graphics/FontCascade.h:254
>> +    static bool treatAsSpace(UChar32 c) { return c == ' ' || c == '\t' || c == '\n' || c == noBreakSpace; }
> 
> I'm confused why the silent truncation wasn't a compiler error??

Integer truncation has always been silent, with few exceptions. It would be great to find a way to tighten that up.

One trick that can be used in particular cases where we want to be careful to be clever is to overload, making the integer type ambiguous if it’s not a perfect match.