Bug 173962 – REGRESSION(r216944): Font loads can cause Chinese characters to draw as .notdef

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Myles C. Maxfield]

Font loads can cause Chinese characters to draw as .notdef

[Myles C. Maxfield]

Created attachment 314116 [details]
Patch

[Myles C. Maxfield]

<rdar://problem/32925318>

[Myles C. Maxfield]

Comment on attachment 314116 [details]
Patch

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

> Source/WebCore/ChangeLog:3
> +        Font loads can cause Chinese characters to draw as .notdef

REGRESSION(r216944):

> LayoutTests/ChangeLog:3
> +        Font loads can cause Chinese characters to draw as .notdef

REGRESSION(r216944):

[Build Bot]

Comment on attachment 314116 [details]
Patch

Attachment 314116 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/4018382

New failing tests:
fast/text/font-load-fallback-chinese.html

[Build Bot]

Created attachment 314120 [details]
Archive of layout-test-results from ews122 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews122  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.5

[Myles C. Maxfield]

Created attachment 314412 [details]
Patch

[Build Bot]

Comment on attachment 314412 [details]
Patch

Attachment 314412 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/4039266

New failing tests:
workers/bomb.html

[Build Bot]

Created attachment 314426 [details]
Archive of layout-test-results from ews105 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Myles C. Maxfield]

Failure is unrelated.

[Build Bot]

Comment on attachment 314412 [details]
Patch

Attachment 314412 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/4046281

New failing tests:
fast/text/font-loading-system-fallback.html
fast/text/font-loading-system-fallback-visibility.html

[Build Bot]

Created attachment 314511 [details]
Archive of layout-test-results from ews121 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews121  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.5

[Myles C. Maxfield]

Created attachment 314695 [details]
Patch

[Build Bot]

Comment on attachment 314695 [details]
Patch

Attachment 314695 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/4061000

New failing tests:
fast/text/font-loading-system-fallback.html

[Build Bot]

Created attachment 314699 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.5

[Myles C. Maxfield]

Created attachment 314728 [details]
Patch

[Myles C. Maxfield]

Created attachment 314756 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 314756 [details]
Patch

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

> Source/WebCore/platform/graphics/Font.cpp:289
> +    if (!m_derivedFontData)
> +        m_derivedFontData = std::make_unique<DerivedFonts>();

This is repeated 3 times. Maybe make an ensureDerivedFontData() function?

> Source/WebCore/platform/graphics/Font.h:137
> +    const Font& invisibleFont() const;

This is a little mysterious. Is this the font you use if you don't want the characters to display?

> Source/WebCore/platform/graphics/Font.h:284
> +        RefPtr<Font> invisible;

I would have said "Font" after the name. "invisibleFont". Same for the preceding.

> Source/WebCore/platform/graphics/FontCascadeFonts.cpp:333
> +

remove this blank line.

[Myles C. Maxfield]

Comment on attachment 314756 [details]
Patch

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

>> Source/WebCore/platform/graphics/Font.h:137
>> +    const Font& invisibleFont() const;
> 
> This is a little mysterious. Is this the font you use if you don't want the characters to display?

Yep! For loading purposes.

[Myles C. Maxfield]

Committed r219221: <http://trac.webkit.org/changeset/219221>