Bug 147390 – [Cocoa] Font fallback is not language-sensitive

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

[Myles C. Maxfield]

Font fallback is not language-sensitive

[Myles C. Maxfield]

Created attachment 257707 [details]
Patch

[Myles C. Maxfield]

Created attachment 257710 [details]
Patch

[Myles C. Maxfield]

Created attachment 257858 [details]
Patch

[Dean Jackson]

Comment on attachment 257858 [details]
Patch

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

> Source/WebCore/platform/graphics/Font.cpp:403
> +        : localeHash(0)
> +        , character(0)
> +        , isForPlatformFont(false)

Use initializer syntax. (There probably is an official name for that term, but I don't know it)

> Source/WebCore/platform/graphics/Font.cpp:414
> +    CharacterFallbackMapKey(WTF::HashTableDeletedValueType): character(hashTableDeletedChar()) { }

I think this should be split into four lines, and have whitespace after.

[WebKit Commit Bot]

Attachment 257858 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/Font.cpp:414:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
Total errors found: 1 in 10 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Build Bot]

Comment on attachment 257858 [details]
Patch

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

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 257889 [details]
Archive of layout-test-results from ews103 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-mavericks  Platform: Mac OS X 10.9.5

[Build Bot]

Comment on attachment 257858 [details]
Patch

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

Number of test failures exceeded the failure limit.

[Build Bot]

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

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

[Myles C. Maxfield]

Comment on attachment 257858 [details]
Patch

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

> Source/WebCore/platform/graphics/mac/FontCacheMac.mm:468
> +    NSFont *substituteFont = CFBridgingRelease(CTFontCreateForCharactersWithLanguage(reinterpret_cast<CTFontRef>(nsFont), characters, length, language.get(), &coveredLength));

This returns unique pointers for the same font each time its called, which is causing us to get all messed up

[Myles C. Maxfield]

Created attachment 257896 [details]
Patch

[Myles C. Maxfield]

Comment on attachment 257896 [details]
Patch

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

> Source/WebCore/platform/graphics/mac/FontCacheMac.mm:475
> +    RetainPtr<CFStringRef> fontString = adoptCF(CFURLGetString(fontURL.get()));

Whoops

[Myles C. Maxfield]

Created attachment 257898 [details]
Patch

[Myles C. Maxfield]

Created attachment 257901 [details]
Patch

[Myles C. Maxfield]

Created attachment 257992 [details]
Patch

[Build Bot]

Comment on attachment 257992 [details]
Patch

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

New failing tests:
css3/line-break/line-break-normal-sound-marks.html
css3/line-break/line-break-strict-hyphens.html
css3/line-break/line-break-loose-postfixes.html
css3/line-break/line-break-auto-postfixes.html
css3/line-break/line-break-strict-sound-marks.html
css3/line-break/line-break-normal-postfixes.html
css3/line-break/line-break-loose-sound-marks.html
css3/line-break/line-break-strict-centered.html
css3/line-break/line-break-loose-hyphens.html
css3/line-break/line-break-normal-hyphens.html
css3/line-break/line-break-normal-centered.html
css3/line-break/line-break-loose-centered.html
css3/line-break/line-break-auto-centered.html
css3/line-break/line-break-strict-postfixes.html

[Build Bot]

Created attachment 257994 [details]
Archive of layout-test-results from ews102 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-mavericks  Platform: Mac OS X 10.9.5

[Build Bot]

Comment on attachment 257992 [details]
Patch

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

New failing tests:
css3/line-break/line-break-normal-sound-marks.html
css3/line-break/line-break-strict-hyphens.html
css3/line-break/line-break-auto-postfixes.html
css3/line-break/line-break-strict-sound-marks.html
css3/line-break/line-break-normal-postfixes.html
css3/line-break/line-break-loose-sound-marks.html
css3/line-break/line-break-strict-centered.html
css3/line-break/line-break-loose-hyphens.html
css3/line-break/line-break-normal-centered.html
css3/line-break/line-break-normal-hyphens.html
css3/line-break/line-break-loose-postfixes.html
css3/line-break/line-break-loose-centered.html
css3/line-break/line-break-auto-centered.html
css3/line-break/line-break-strict-postfixes.html

[Build Bot]

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

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

[Myles C. Maxfield]

Created attachment 258021 [details]
Patch

[Myles C. Maxfield]

Created attachment 258023 [details]
Patch

[Myles C. Maxfield]

Committed r187729: <http://trac.webkit.org/changeset/187729>

[WebKit Commit Bot]

Re-opened since this is blocked by bug 147575

[Myles C. Maxfield]

Created attachment 258451 [details]
Patch

[Myles C. Maxfield]

Created attachment 258529 [details]
Patch

[Myles C. Maxfield]

Committed r188167: <http://trac.webkit.org/changeset/188167>

[Alexey Proskuryakov]

fast/text/fallback-language-han.html fails on bots:

https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=fast%2Ftext%2Ffallback-language-han.html

[Myles C. Maxfield]

*** Bug 143788 has been marked as a duplicate of this bug. ***

[Myles C. Maxfield]

Rolled out in r190754

[Myles C. Maxfield]

Created attachment 265355 [details]
Patch

[Myles C. Maxfield]

Committed r192372: <http://trac.webkit.org/changeset/192372>

[Myles C. Maxfield]

Created attachment 265415 [details]
Flip the switch

[Myles C. Maxfield]

Committed r192382: <http://trac.webkit.org/changeset/192382>

[Darin Adler]

Comment on attachment 265355 [details]
Patch

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

> Source/WebCore/platform/graphics/Font.cpp:432
> +        hasher.add(key.locale.isNull() ? 0 : key.locale.impl()->existingHash());

Seems like we should add an existingHash function to String. This is really awkward for no good reason.

[Myles C. Maxfield]

Comment on attachment 265355 [details]
Patch

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

>> Source/WebCore/platform/graphics/Font.cpp:432
>> +        hasher.add(key.locale.isNull() ? 0 : key.locale.impl()->existingHash());
> 
> Seems like we should add an existingHash function to String. This is really awkward for no good reason.

https://bugs.webkit.org/show_bug.cgi?id=151717

[Myles C. Maxfield]

Removing Mavericks-specific lines from platform/mac/TestExpecations due to no longer supporting Mavericks.

[Myles C. Maxfield]

Committed r193663: <http://trac.webkit.org/changeset/193663>