Bug 147390 - [Cocoa] Font fallback is not language-sensitive
Summary: [Cocoa] Font fallback is not language-sensitive
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Myles C. Maxfield
URL:
Keywords:
: 143788 (view as bug list)
Depends on: 147457 147483 147530 147532 147575
Blocks:
  Show dependency treegraph
 
Reported: 2015-07-28 17:16 PDT by Myles C. Maxfield
Modified: 2015-12-07 16:10 PST (History)
3 users (show)

See Also:


Attachments
Patch (10.04 KB, patch)
2015-07-28 17:18 PDT, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (10.03 KB, patch)
2015-07-28 17:20 PDT, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (12.64 KB, patch)
2015-07-30 15:08 PDT, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews103 for mac-mavericks (1.33 MB, application/zip)
2015-07-30 17:44 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews105 for mac-mavericks-wk2 (1.28 MB, application/zip)
2015-07-30 17:59 PDT, Build Bot
no flags Details
Patch (10.63 KB, patch)
2015-07-30 20:17 PDT, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (12.21 KB, patch)
2015-07-30 21:41 PDT, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (9.73 KB, patch)
2015-07-30 22:29 PDT, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (10.85 KB, patch)
2015-07-31 21:13 PDT, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews102 for mac-mavericks (1.13 MB, application/zip)
2015-07-31 21:48 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews105 for mac-mavericks-wk2 (1.56 MB, application/zip)
2015-07-31 22:15 PDT, Build Bot
no flags Details
Patch (73.55 KB, patch)
2015-08-01 15:06 PDT, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (74.25 KB, patch)
2015-08-01 15:18 PDT, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (74.55 KB, patch)
2015-08-06 21:40 PDT, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (11.14 KB, patch)
2015-08-07 14:35 PDT, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (9.53 KB, patch)
2015-11-11 20:16 PST, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Flip the switch (1.96 KB, patch)
2015-11-12 11:49 PST, Myles C. Maxfield
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Myles C. Maxfield 2015-07-28 17:16:42 PDT
Font fallback is not language-sensitive
Comment 1 Myles C. Maxfield 2015-07-28 17:18:53 PDT
Created attachment 257707 [details]
Patch
Comment 2 Myles C. Maxfield 2015-07-28 17:20:00 PDT
Created attachment 257710 [details]
Patch
Comment 3 Myles C. Maxfield 2015-07-30 15:08:52 PDT
Created attachment 257858 [details]
Patch
Comment 4 Dean Jackson 2015-07-30 15:44:41 PDT
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.
Comment 5 WebKit Commit Bot 2015-07-30 17:14:48 PDT
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.
Comment 6 Build Bot 2015-07-30 17:44:30 PDT
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.
Comment 7 Build Bot 2015-07-30 17:44:33 PDT
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
Comment 8 Build Bot 2015-07-30 17:59:07 PDT
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.
Comment 9 Build Bot 2015-07-30 17:59:09 PDT
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
Comment 10 Myles C. Maxfield 2015-07-30 19:02:30 PDT
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
Comment 11 Myles C. Maxfield 2015-07-30 20:17:33 PDT
Created attachment 257896 [details]
Patch
Comment 12 Myles C. Maxfield 2015-07-30 20:59:53 PDT
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
Comment 13 Myles C. Maxfield 2015-07-30 21:41:53 PDT
Created attachment 257898 [details]
Patch
Comment 14 Myles C. Maxfield 2015-07-30 22:29:03 PDT
Created attachment 257901 [details]
Patch
Comment 15 Myles C. Maxfield 2015-07-31 21:13:59 PDT
Created attachment 257992 [details]
Patch
Comment 16 Build Bot 2015-07-31 21:48:40 PDT
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
Comment 17 Build Bot 2015-07-31 21:48:44 PDT
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
Comment 18 Build Bot 2015-07-31 22:14:56 PDT
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
Comment 19 Build Bot 2015-07-31 22:15:00 PDT
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
Comment 20 Myles C. Maxfield 2015-08-01 15:06:58 PDT
Created attachment 258021 [details]
Patch
Comment 21 Myles C. Maxfield 2015-08-01 15:18:32 PDT
Created attachment 258023 [details]
Patch
Comment 22 Myles C. Maxfield 2015-08-01 17:48:57 PDT
Committed r187729: <http://trac.webkit.org/changeset/187729>
Comment 23 WebKit Commit Bot 2015-08-03 11:28:40 PDT
Re-opened since this is blocked by bug 147575
Comment 24 Myles C. Maxfield 2015-08-06 21:40:46 PDT
Created attachment 258451 [details]
Patch
Comment 25 Myles C. Maxfield 2015-08-07 14:35:12 PDT
Created attachment 258529 [details]
Patch
Comment 26 Myles C. Maxfield 2015-08-07 14:50:00 PDT
Committed r188167: <http://trac.webkit.org/changeset/188167>
Comment 27 Alexey Proskuryakov 2015-08-07 15:36:21 PDT
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
Comment 28 Myles C. Maxfield 2015-08-26 23:52:32 PDT
*** Bug 143788 has been marked as a duplicate of this bug. ***
Comment 29 Myles C. Maxfield 2015-11-11 20:11:12 PST
Rolled out in r190754
Comment 30 Myles C. Maxfield 2015-11-11 20:16:25 PST
Created attachment 265355 [details]
Patch
Comment 31 Myles C. Maxfield 2015-11-12 11:37:54 PST
Committed r192372: <http://trac.webkit.org/changeset/192372>
Comment 32 Myles C. Maxfield 2015-11-12 11:49:16 PST
Created attachment 265415 [details]
Flip the switch
Comment 33 Myles C. Maxfield 2015-11-12 13:29:40 PST
Committed r192382: <http://trac.webkit.org/changeset/192382>
Comment 34 Darin Adler 2015-11-30 16:10:14 PST
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.
Comment 35 Myles C. Maxfield 2015-12-01 14:45:55 PST
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
Comment 36 Myles C. Maxfield 2015-12-07 16:05:12 PST
Removing Mavericks-specific lines from platform/mac/TestExpecations due to no longer supporting Mavericks.
Comment 37 Myles C. Maxfield 2015-12-07 16:10:44 PST
Committed r193663: <http://trac.webkit.org/changeset/193663>