Bug 235559

Summary: REGRESSION(r281419): iCloud.com Notes web app fonts render incorrectly
Product: WebKit Reporter: Myles C. Maxfield <mmaxfield>
Component: New BugsAssignee: Myles C. Maxfield <mmaxfield>
Status: RESOLVED FIXED    
Severity: Normal CC: clopez, darin, ews-watchlist, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch darin: review+

Description Myles C. Maxfield 2022-01-24 22:31:01 PST
REGRESSION(r281419): iCloud.com Notes web app fonts render incorrectly
Comment 1 Myles C. Maxfield 2022-01-24 22:35:27 PST
<rdar://problem/87268956>
Comment 2 Myles C. Maxfield 2022-01-24 22:36:00 PST
Created attachment 449899 [details]
Patch
Comment 3 EWS Watchlist 2022-01-24 22:38:23 PST
This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess
Comment 4 Darin Adler 2022-01-24 22:58:48 PST
Comment on attachment 449899 [details]
Patch

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

> Source/WebCore/platform/graphics/WidthIterator.cpp:608
> +        // "Control characters (Unicode category Cc)âother than NULL (U+0000), tabs (U+0009), line feeds (U+000A), carriage returns (U+000D) and sequences that form a segment breakâmust be rendered as a visible glyph"

Seems like this "counts your chickens before they are hatched" since it assumes your pull request will be approved. If I was you I would be tempted to write a brief comment that more literally explains the current status of this rather than assuming the change to the spec will go through.

> Source/WebCore/platform/graphics/WidthIterator.cpp:610
> +            && u_charType(characterResponsibleForThisGlyph) == U_CONTROL_CHAR) {

Does this really reject tabs, line feeds, carriage returns, and sequences that form a segment break? The comment above lists all those special cases, and the code here doesn’t cover them. It seems like the comment might need to explain why the code below doesn’t match it?
Comment 5 Myles C. Maxfield 2022-01-24 23:14:32 PST
Comment on attachment 449899 [details]
Patch

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

>> Source/WebCore/platform/graphics/WidthIterator.cpp:610
>> +            && u_charType(characterResponsibleForThisGlyph) == U_CONTROL_CHAR) {
> 
> Does this really reject tabs, line feeds, carriage returns, and sequences that form a segment break? The comment above lists all those special cases, and the code here doesn’t cover them. It seems like the comment might need to explain why the code below doesn’t match it?

Those are handled in the stanza right above this one.
Comment 6 Myles C. Maxfield 2022-01-25 10:50:16 PST
Committed r288564 (246392@trunk): <https://commits.webkit.org/246392@trunk>