Bug 187723 - [Cocoa] Ask platform for generic font family mappings
Summary: [Cocoa] Ask platform for generic font family mappings
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
: 167907 187895 (view as bug list)
Depends on:
Blocks:
 
Reported: 2018-07-16 19:35 PDT by Myles C. Maxfield
Modified: 2019-02-18 18:59 PST (History)
9 users (show)

See Also:


Attachments
WIP (12.06 KB, patch)
2018-07-16 19:36 PDT, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (25.19 KB, patch)
2018-07-17 15:47 PDT, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews124 for ios-simulator-wk2 (237.60 KB, application/zip)
2018-07-18 13:37 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews206 for win-future (12.97 MB, application/zip)
2018-07-18 16:11 PDT, Build Bot
no flags Details
Patch (17.87 KB, patch)
2018-07-31 22:03 PDT, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (17.54 KB, patch)
2018-07-31 22:29 PDT, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews121 for ios-simulator-wk2 (2.26 MB, application/zip)
2018-08-01 00:23 PDT, Build Bot
no flags Details
Patch (22.10 KB, patch)
2019-01-29 17:54 PST, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews103 for mac-highsierra (5.35 MB, application/zip)
2019-01-29 18:48 PST, Build Bot
no flags Details
Archive of layout-test-results from ews107 for mac-highsierra-wk2 (5.41 MB, application/zip)
2019-01-29 19:01 PST, Build Bot
no flags Details
Archive of layout-test-results from ews126 for ios-simulator-wk2 (4.94 MB, application/zip)
2019-01-29 19:46 PST, Build Bot
no flags Details
Archive of layout-test-results from ews113 for mac-highsierra (4.95 MB, application/zip)
2019-01-29 20:08 PST, Build Bot
no flags Details
Patch (21.85 KB, patch)
2019-01-30 13:33 PST, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews103 for mac-highsierra (4.69 MB, application/zip)
2019-01-30 14:41 PST, Build Bot
no flags Details
Archive of layout-test-results from ews107 for mac-highsierra-wk2 (4.81 MB, application/zip)
2019-01-30 14:54 PST, Build Bot
no flags Details
Archive of layout-test-results from ews117 for mac-highsierra (4.30 MB, application/zip)
2019-01-30 15:23 PST, Build Bot
no flags Details
Archive of layout-test-results from ews122 for ios-simulator-wk2 (4.24 MB, application/zip)
2019-01-30 15:36 PST, Build Bot
no flags Details
Patch (73.84 KB, patch)
2019-01-30 17:09 PST, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews102 for mac-highsierra (3.53 MB, application/zip)
2019-01-30 18:10 PST, Build Bot
no flags Details
Archive of layout-test-results from ews106 for mac-highsierra-wk2 (4.08 MB, application/zip)
2019-01-30 18:32 PST, Build Bot
no flags Details
Archive of layout-test-results from ews117 for mac-highsierra (3.15 MB, application/zip)
2019-01-30 18:53 PST, Build Bot
no flags Details
Archive of layout-test-results from ews122 for ios-simulator-wk2 (3.87 MB, application/zip)
2019-01-30 19:47 PST, Build Bot
no flags Details
Patch (112.99 KB, patch)
2019-02-01 13:16 PST, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews100 for mac-highsierra (2.76 MB, application/zip)
2019-02-01 14:25 PST, Build Bot
no flags Details
Archive of layout-test-results from ews106 for mac-highsierra-wk2 (2.89 MB, application/zip)
2019-02-01 14:38 PST, Build Bot
no flags Details
Archive of layout-test-results from ews115 for mac-highsierra (2.40 MB, application/zip)
2019-02-01 15:09 PST, Build Bot
no flags Details
Archive of layout-test-results from ews125 for ios-simulator-wk2 (2.46 MB, application/zip)
2019-02-01 15:19 PST, Build Bot
no flags Details
No exceptions (125.96 KB, patch)
2019-02-01 16:03 PST, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews100 for mac-highsierra (2.69 MB, application/zip)
2019-02-01 16:59 PST, Build Bot
no flags Details
Archive of layout-test-results from ews112 for mac-highsierra (2.61 MB, application/zip)
2019-02-01 17:44 PST, Build Bot
no flags Details
Archive of layout-test-results from ews104 for mac-highsierra-wk2 (2.80 MB, application/zip)
2019-02-01 17:47 PST, Build Bot
no flags Details
No exceptions (126.20 KB, patch)
2019-02-02 00:58 PST, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (33.73 KB, patch)
2019-02-02 15:12 PST, Myles C. Maxfield
ews: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews106 for mac-highsierra-wk2 (2.55 MB, application/zip)
2019-02-02 16:34 PST, Build Bot
no flags Details
Archive of layout-test-results from ews123 for ios-simulator-wk2 (4.03 MB, application/zip)
2019-02-02 17:20 PST, Build Bot
no flags Details
Patch (35.60 KB, patch)
2019-02-04 11:39 PST, Myles C. Maxfield
bfulgham: review+
Details | Formatted Diff | Diff
Patch for committing (35.91 KB, patch)
2019-02-08 19:03 PST, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch for committing (36.73 KB, patch)
2019-02-11 17:17 PST, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch for committing (36.72 KB, patch)
2019-02-11 17:19 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 2018-07-16 19:35:38 PDT
[Cocoa] Ask platform for generic font family mappings
Comment 1 Myles C. Maxfield 2018-07-16 19:36:32 PDT
Created attachment 345137 [details]
WIP
Comment 2 Myles C. Maxfield 2018-07-16 19:36:55 PDT
<rdar://problem/41892438>
Comment 3 Build Bot 2018-07-16 19:38:34 PDT
Attachment 345137 [details] did not pass style-queue:


ERROR: Source/WebCore/page/FrameView.cpp:4142:  Should have a space between // and comment  [whitespace/comments] [4]
ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 2 in 10 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Myles C. Maxfield 2018-07-17 15:47:31 PDT
Created attachment 345200 [details]
Patch
Comment 5 Build Bot 2018-07-18 12:17:15 PDT
Attachment 345200 [details] did not pass style-queue:


ERROR: Source/WebCore/page/FrameView.cpp:4142:  Should have a space between // and comment  [whitespace/comments] [4]
ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 2 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 6 Build Bot 2018-07-18 13:37:24 PDT
Comment on attachment 345200 [details]
Patch

Attachment 345200 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/8577754

Number of test failures exceeded the failure limit.
Comment 7 Build Bot 2018-07-18 13:37:25 PDT
Created attachment 345277 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.4
Comment 8 Build Bot 2018-07-18 16:11:29 PDT
Comment on attachment 345200 [details]
Patch

Attachment 345200 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/8580189

New failing tests:
http/tests/preload/onload_event.html
fast/css/beforeSelectorOnCodeElement.html
Comment 9 Build Bot 2018-07-18 16:11:40 PDT
Created attachment 345301 [details]
Archive of layout-test-results from ews206 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews206  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Comment 10 Myles C. Maxfield 2018-07-20 19:02:41 PDT
The iOS failures are because of kCTFontUIFontDesignTrait.

I think this patch is the wrong approach, because the whole point of FontFamilySpecificationCoreText is that CoreText can give is exactly the right CTFontRef to use, and we shouldn't use the CSS font selection algorithm. This is right for system-ui, but it isn't right for the other generic font families. For those, we still need to run the font selection algorithm in order to find the font with the best weight/width/slope. Therefore, we should simply pull out the family name from the return of CTFontDescriptorCreateForCSSFamily() and continue down the normal path.

We still need to figure out how to determine which signal to listen to:
1) https://developer.apple.com/documentation/webkit/webpreferences/1536182-cursivefontfamily API clients can set the default font families
2) Core Text can tell us what the default families should be
Comment 11 Myles C. Maxfield 2018-07-20 19:15:39 PDT
WebKit sets all of these generic font families to default values (see WebPreferencesDefaultValues.h)
Comment 12 Myles C. Maxfield 2018-07-20 19:18:58 PDT
The current design is that the API changes the "default" value of the generic family, but SettingsBaseCocoa sets the script-specific value of the generic family, which overrides the default. Therefore, web content in CJK will see standard/fixed/serif/sans-serif from SettingsBaseCocoa, and never will see anything the API sets.

This is pretty unfortunate because, for scripts other than CJK, CoreText can give us better choices than these hardcoded font names inside WebPreferencesDefaultValues.h.

It seems like this API wasn't designed well.
Comment 13 Myles C. Maxfield 2018-07-20 19:26:39 PDT
+[NSFont userFixedPitchFontOfSize] :(
Comment 14 Myles C. Maxfield 2018-07-20 19:33:56 PDT
Looks like WebKit2 doesn't have API for changing these generic font families. (Though it does have some SPI: WKPreferencesSetSansSerifFontFamily)
Comment 15 Myles C. Maxfield 2018-07-31 22:03:44 PDT
Created attachment 346259 [details]
Patch
Comment 16 Myles C. Maxfield 2018-07-31 22:24:25 PDT
*** Bug 187895 has been marked as a duplicate of this bug. ***
Comment 17 Myles C. Maxfield 2018-07-31 22:29:15 PDT
Created attachment 346260 [details]
Patch
Comment 18 Build Bot 2018-08-01 00:23:45 PDT
Comment on attachment 346260 [details]
Patch

Attachment 346260 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/8720839

New failing tests:
fast/text/jp-sans-serif.html
Comment 19 Build Bot 2018-08-01 00:23:46 PDT
Created attachment 346264 [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.13.4
Comment 20 Myles C. Maxfield 2019-01-29 17:54:48 PST
Created attachment 360532 [details]
Patch
Comment 21 Build Bot 2019-01-29 18:48:52 PST
Comment on attachment 360532 [details]
Patch

Attachment 360532 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/10946086

New failing tests:
fast/overflow/007.html
fast/text/hyphenate-avoid-orphaned-word.html
fast/text/international/002.html
imported/w3c/web-platform-tests/css/css-text/word-break/word-break-normal-lo-000.html
fast/css-generated-content/014.html
fast/text/international/lang-sensitive-fonts-xml.xhtml
fast/text/hangul-generic-font-families.html
fast/text/international/generic-font-family-language-traditional.html
fast/text/international/locale-sensitive-fonts.html
fast/ruby/nested-ruby.html
fast/text/hyphens.html
fast/text/international/lang-sensitive-fonts.html
tables/mozilla/bugs/bug139524-2.html
tables/mozilla/bugs/bug10296-1.html
svg/custom/object-sizing.xhtml
fast/css/line-height-font-order.html
fast/text/international/font-fallback-to-common-script.html
fast/text/hyphenate-character.html
tables/mozilla/bugs/bug10269-2.html
tables/mozilla_expected_failures/bugs/bug2479-5.html
Comment 22 Build Bot 2019-01-29 18:48:54 PST
Created attachment 360536 [details]
Archive of layout-test-results from ews103 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-highsierra  Platform: Mac OS X 10.13.6
Comment 23 Build Bot 2019-01-29 19:01:47 PST
Comment on attachment 360532 [details]
Patch

Attachment 360532 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/10946096

New failing tests:
fast/overflow/007.html
fast/text/hyphenate-avoid-orphaned-word.html
fast/text/international/002.html
imported/w3c/web-platform-tests/css/css-text/word-break/word-break-normal-lo-000.html
fast/css-generated-content/014.html
fast/text/international/lang-sensitive-fonts-xml.xhtml
fast/text/hangul-generic-font-families.html
fast/text/international/generic-font-family-language-traditional.html
fast/text/international/locale-sensitive-fonts.html
fast/ruby/nested-ruby.html
fast/text/hyphens.html
fast/text/international/lang-sensitive-fonts.html
tables/mozilla/bugs/bug139524-2.html
tables/mozilla/bugs/bug10296-1.html
svg/custom/object-sizing.xhtml
fast/css/line-height-font-order.html
fast/text/international/font-fallback-to-common-script.html
fast/text/hyphenate-character.html
tables/mozilla/bugs/bug10269-2.html
tables/mozilla_expected_failures/bugs/bug2479-5.html
Comment 24 Build Bot 2019-01-29 19:01:49 PST
Created attachment 360538 [details]
Archive of layout-test-results from ews107 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6
Comment 25 Build Bot 2019-01-29 19:46:34 PST
Comment on attachment 360532 [details]
Patch

Attachment 360532 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/10946223

New failing tests:
fast/block/float/016.html
tables/mozilla_expected_failures/bugs/bug2479-5.html
fast/overflow/007.html
fast/text/hyphenate-avoid-orphaned-word.html
fast/text/jp-sans-serif.html
fast/css-generated-content/014.html
fast/text/international/lang-sensitive-fonts-xml.xhtml
fast/text/hangul-generic-font-families.html
fast/text/international/generic-font-family-language-traditional.html
fast/text/international/locale-sensitive-fonts.html
fast/text/international/lang-sensitive-fonts.html
tables/mozilla/bugs/bug139524-2.html
tables/mozilla/bugs/bug10296-1.html
svg/custom/object-sizing.xhtml
fast/css/line-height-font-order.html
fast/text/international/font-fallback-to-common-script.html
tables/mozilla/bugs/bug10269-2.html
fast/ruby/nested-ruby.html
Comment 26 Build Bot 2019-01-29 19:46:36 PST
Created attachment 360539 [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.13.6
Comment 27 Build Bot 2019-01-29 20:08:29 PST
Comment on attachment 360532 [details]
Patch

Attachment 360532 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/10946373

New failing tests:
fast/overflow/007.html
fast/text/hyphenate-avoid-orphaned-word.html
fast/text/international/002.html
imported/w3c/web-platform-tests/css/css-text/word-break/word-break-normal-lo-000.html
fast/css-generated-content/014.html
fast/text/international/lang-sensitive-fonts-xml.xhtml
fast/text/hangul-generic-font-families.html
fast/text/international/generic-font-family-language-traditional.html
fast/text/international/locale-sensitive-fonts.html
fast/ruby/nested-ruby.html
fast/text/hyphens.html
fast/text/international/lang-sensitive-fonts.html
tables/mozilla/bugs/bug139524-2.html
tables/mozilla/bugs/bug10296-1.html
svg/custom/object-sizing.xhtml
fast/css/line-height-font-order.html
fast/text/international/font-fallback-to-common-script.html
fast/text/hyphenate-character.html
tables/mozilla/bugs/bug10269-2.html
tables/mozilla_expected_failures/bugs/bug2479-5.html
Comment 28 Build Bot 2019-01-29 20:08:31 PST
Created attachment 360540 [details]
Archive of layout-test-results from ews113 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113  Port: mac-highsierra  Platform: Mac OS X 10.13.6
Comment 29 Myles C. Maxfield 2019-01-30 13:33:11 PST
Created attachment 360610 [details]
Patch
Comment 30 Jiang Jiang 2019-01-30 13:39:28 PST
Comment on attachment 360610 [details]
Patch

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

> Source/WebCore/page/cocoa/SettingsBaseCocoa.mm:53
>      setStandardFontFamily("Songti TC", USCRIPT_TRADITIONAL_HAN);

This is interesting, why is standard family Songti for MAC platforms?
Comment 31 Myles C. Maxfield 2019-01-30 13:57:57 PST
Comment on attachment 360610 [details]
Patch

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

>> Source/WebCore/page/cocoa/SettingsBaseCocoa.mm:53
>>      setStandardFontFamily("Songti TC", USCRIPT_TRADITIONAL_HAN);
> 
> This is interesting, why is standard family Songti for MAC platforms?

https://bugs.webkit.org/show_bug.cgi?id=117568
Comment 32 Jiang Jiang 2019-01-30 14:11:54 PST
I guess what I'm trying to say is: why is standard font family for PLATFORM(MAC) serif instead of sans-serif like other platforms?
Comment 33 Myles C. Maxfield 2019-01-30 14:15:36 PST
(In reply to Jiang Jiang from comment #32)
> I guess what I'm trying to say is: why is standard font family for
> PLATFORM(MAC) serif instead of sans-serif like other platforms?

I don't know; that code is older than my tenure at Apple.

My guess is that, since the standard font family is serif for en, we assumed it should be serif for other languages too. But I don't know.

Do you have a suggestion for a better choice?
Comment 34 Myles C. Maxfield 2019-01-30 14:17:34 PST
See the comment in the next section: // There is no serif Chinese font in default iOS installation.
Comment 35 Build Bot 2019-01-30 14:41:55 PST
Comment on attachment 360610 [details]
Patch

Attachment 360610 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/10956926

New failing tests:
fast/overflow/007.html
fast/text/hyphenate-avoid-orphaned-word.html
imported/w3c/web-platform-tests/css/css-text/word-break/word-break-normal-lo-000.html
fast/css-generated-content/014.html
fast/text/international/lang-sensitive-fonts-xml.xhtml
fast/text/international/generic-font-family-language-traditional.html
fast/text/international/locale-sensitive-fonts.html
fast/text/hyphens.html
fast/text/international/lang-sensitive-fonts.html
tables/mozilla/bugs/bug139524-2.html
tables/mozilla/bugs/bug10296-1.html
svg/custom/object-sizing.xhtml
fast/css/line-height-font-order.html
fast/text/international/font-fallback-to-common-script.html
fast/text/hyphenate-character.html
tables/mozilla/bugs/bug10269-2.html
tables/mozilla_expected_failures/bugs/bug2479-5.html
Comment 36 Build Bot 2019-01-30 14:41:57 PST
Created attachment 360624 [details]
Archive of layout-test-results from ews103 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-highsierra  Platform: Mac OS X 10.13.6
Comment 37 Build Bot 2019-01-30 14:54:18 PST
Comment on attachment 360610 [details]
Patch

Attachment 360610 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/10956972

New failing tests:
fast/overflow/007.html
fast/text/hyphenate-avoid-orphaned-word.html
fast/text/international/font-fallback-to-common-script.html
fast/css-generated-content/014.html
fast/text/international/lang-sensitive-fonts-xml.xhtml
fast/text/international/generic-font-family-language-traditional.html
fast/text/international/locale-sensitive-fonts.html
fast/text/hyphens.html
fast/text/international/lang-sensitive-fonts.html
tables/mozilla/bugs/bug139524-2.html
tables/mozilla/bugs/bug10296-1.html
svg/custom/object-sizing.xhtml
fast/css/line-height-font-order.html
imported/w3c/web-platform-tests/css/css-text/word-break/word-break-normal-lo-000.html
fast/text/hyphenate-character.html
tables/mozilla/bugs/bug10269-2.html
tables/mozilla_expected_failures/bugs/bug2479-5.html
Comment 38 Build Bot 2019-01-30 14:54:21 PST
Created attachment 360628 [details]
Archive of layout-test-results from ews107 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6
Comment 39 Build Bot 2019-01-30 15:23:06 PST
Comment on attachment 360610 [details]
Patch

Attachment 360610 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/10957015

New failing tests:
fast/overflow/007.html
fast/text/hyphenate-avoid-orphaned-word.html
fast/text/international/font-fallback-to-common-script.html
fast/css-generated-content/014.html
fast/text/international/lang-sensitive-fonts-xml.xhtml
fast/text/international/generic-font-family-language-traditional.html
fast/text/international/locale-sensitive-fonts.html
fast/text/hyphens.html
fast/text/international/lang-sensitive-fonts.html
tables/mozilla/bugs/bug139524-2.html
tables/mozilla/bugs/bug10296-1.html
svg/custom/object-sizing.xhtml
fast/css/line-height-font-order.html
imported/w3c/web-platform-tests/css/css-text/word-break/word-break-normal-lo-000.html
fast/text/hyphenate-character.html
tables/mozilla/bugs/bug10269-2.html
tables/mozilla_expected_failures/bugs/bug2479-5.html
Comment 40 Build Bot 2019-01-30 15:23:08 PST
Created attachment 360631 [details]
Archive of layout-test-results from ews117 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117  Port: mac-highsierra  Platform: Mac OS X 10.13.6
Comment 41 Build Bot 2019-01-30 15:36:46 PST
Comment on attachment 360610 [details]
Patch

Attachment 360610 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/10957036

New failing tests:
fast/block/float/016.html
fast/overflow/007.html
fast/text/hyphenate-avoid-orphaned-word.html
fast/text/jp-sans-serif.html
fast/css-generated-content/014.html
fast/text/international/lang-sensitive-fonts-xml.xhtml
fast/text/international/generic-font-family-language-traditional.html
fast/text/international/locale-sensitive-fonts.html
fast/text/international/lang-sensitive-fonts.html
tables/mozilla/bugs/bug139524-2.html
tables/mozilla/bugs/bug10296-1.html
svg/custom/object-sizing.xhtml
fast/css/line-height-font-order.html
fast/text/international/font-fallback-to-common-script.html
tables/mozilla/bugs/bug10269-2.html
tables/mozilla_expected_failures/bugs/bug2479-5.html
Comment 42 Build Bot 2019-01-30 15:36:48 PST
Created attachment 360633 [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.13.6
Comment 43 Myles C. Maxfield 2019-01-30 17:09:30 PST
Created attachment 360654 [details]
Patch
Comment 44 Build Bot 2019-01-30 18:10:17 PST
Comment on attachment 360654 [details]
Patch

Attachment 360654 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/10960833

New failing tests:
fast/text/han-generic-font-families.html
imported/w3c/web-platform-tests/css/css-text/word-break/word-break-normal-lo-000.html
tables/mozilla_expected_failures/bugs/bug2479-5.html
Comment 45 Build Bot 2019-01-30 18:10:19 PST
Created attachment 360664 [details]
Archive of layout-test-results from ews102 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-highsierra  Platform: Mac OS X 10.13.6
Comment 46 Build Bot 2019-01-30 18:32:13 PST
Comment on attachment 360654 [details]
Patch

Attachment 360654 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/10961188

New failing tests:
fast/text/han-generic-font-families.html
imported/w3c/web-platform-tests/css/css-text/word-break/word-break-normal-lo-000.html
tables/mozilla_expected_failures/bugs/bug2479-5.html
Comment 47 Build Bot 2019-01-30 18:32:15 PST
Created attachment 360665 [details]
Archive of layout-test-results from ews106 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6
Comment 48 Build Bot 2019-01-30 18:53:06 PST
Comment on attachment 360654 [details]
Patch

Attachment 360654 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/10961183

New failing tests:
fast/text/han-generic-font-families.html
imported/w3c/web-platform-tests/css/css-text/word-break/word-break-normal-lo-000.html
tables/mozilla_expected_failures/bugs/bug2479-5.html
Comment 49 Build Bot 2019-01-30 18:53:08 PST
Created attachment 360671 [details]
Archive of layout-test-results from ews117 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117  Port: mac-highsierra  Platform: Mac OS X 10.13.6
Comment 50 Build Bot 2019-01-30 19:47:31 PST
Comment on attachment 360654 [details]
Patch

Attachment 360654 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/10961916

New failing tests:
fast/block/float/016.html
fast/overflow/007.html
fast/text/hyphenate-avoid-orphaned-word.html
fast/css-generated-content/014.html
tables/mozilla/bugs/bug139524-2.html
tables/mozilla/bugs/bug10296-1.html
svg/custom/object-sizing.xhtml
fast/css/line-height-font-order.html
fast/text/jp-sans-serif.html
tables/mozilla/bugs/bug10269-2.html
tables/mozilla_expected_failures/bugs/bug2479-5.html
Comment 51 Build Bot 2019-01-30 19:47:33 PST
Created attachment 360679 [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.13.6
Comment 52 Myles C. Maxfield 2019-02-01 12:45:02 PST
*** Bug 167907 has been marked as a duplicate of this bug. ***
Comment 53 Myles C. Maxfield 2019-02-01 13:16:11 PST
Created attachment 360887 [details]
Patch
Comment 54 Build Bot 2019-02-01 14:25:31 PST
Comment on attachment 360887 [details]
Patch

Attachment 360887 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/10998253

New failing tests:
tables/mozilla_expected_failures/bugs/bug2479-5.html
Comment 55 Build Bot 2019-02-01 14:25:33 PST
Created attachment 360901 [details]
Archive of layout-test-results from ews100 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-highsierra  Platform: Mac OS X 10.13.6
Comment 56 Build Bot 2019-02-01 14:38:27 PST
Comment on attachment 360887 [details]
Patch

Attachment 360887 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/10998294

New failing tests:
tables/mozilla_expected_failures/bugs/bug2479-5.html
Comment 57 Build Bot 2019-02-01 14:38:29 PST
Created attachment 360905 [details]
Archive of layout-test-results from ews106 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6
Comment 58 Build Bot 2019-02-01 15:09:22 PST
Comment on attachment 360887 [details]
Patch

Attachment 360887 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/10998386

New failing tests:
tables/mozilla_expected_failures/bugs/bug2479-5.html
Comment 59 Build Bot 2019-02-01 15:09:24 PST
Created attachment 360909 [details]
Archive of layout-test-results from ews115 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-highsierra  Platform: Mac OS X 10.13.6
Comment 60 Build Bot 2019-02-01 15:19:42 PST
Comment on attachment 360887 [details]
Patch

Attachment 360887 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/10998435

New failing tests:
fast/block/float/016.html
Comment 61 Build Bot 2019-02-01 15:19:45 PST
Created attachment 360911 [details]
Archive of layout-test-results from ews125 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6
Comment 62 Myles C. Maxfield 2019-02-01 16:03:13 PST
Created attachment 360917 [details]
No exceptions
Comment 63 Build Bot 2019-02-01 16:59:10 PST
Comment on attachment 360917 [details]
No exceptions

Attachment 360917 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/11000874

New failing tests:
fast/text/ja-sans-serif.html
Comment 64 Build Bot 2019-02-01 16:59:12 PST
Created attachment 360924 [details]
Archive of layout-test-results from ews100 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-highsierra  Platform: Mac OS X 10.13.6
Comment 65 Build Bot 2019-02-01 17:43:58 PST
Comment on attachment 360917 [details]
No exceptions

Attachment 360917 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/11000970

New failing tests:
fast/text/ja-sans-serif.html
Comment 66 Build Bot 2019-02-01 17:44:00 PST
Created attachment 360932 [details]
Archive of layout-test-results from ews112 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112  Port: mac-highsierra  Platform: Mac OS X 10.13.6
Comment 67 Build Bot 2019-02-01 17:47:47 PST
Comment on attachment 360917 [details]
No exceptions

Attachment 360917 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/11001124

New failing tests:
fast/text/ja-sans-serif.html
Comment 68 Build Bot 2019-02-01 17:47:50 PST
Created attachment 360935 [details]
Archive of layout-test-results from ews104 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6
Comment 69 Myles C. Maxfield 2019-02-02 00:58:08 PST
Created attachment 360968 [details]
No exceptions
Comment 70 Myles C. Maxfield 2019-02-02 15:12:04 PST
Created attachment 360982 [details]
Patch
Comment 71 Build Bot 2019-02-02 16:34:28 PST
Comment on attachment 360982 [details]
Patch

Attachment 360982 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/11010047

New failing tests:
http/wpt/mediarecorder/MediaRecorder-AV-audio-video-dataavailable.html
Comment 72 Build Bot 2019-02-02 16:34:31 PST
Created attachment 360985 [details]
Archive of layout-test-results from ews106 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6
Comment 73 Build Bot 2019-02-02 17:20:01 PST
Comment on attachment 360982 [details]
Patch

Attachment 360982 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/11010106

New failing tests:
fast/block/float/016.html
fast/overflow/007.html
fast/text/hyphenate-avoid-orphaned-word.html
fast/css-generated-content/014.html
tables/mozilla/bugs/bug139524-2.html
tables/mozilla/bugs/bug10296-1.html
svg/custom/object-sizing.xhtml
fast/css/line-height-font-order.html
tables/mozilla/bugs/bug10269-2.html
tables/mozilla_expected_failures/bugs/bug2479-5.html
Comment 74 Build Bot 2019-02-02 17:20:04 PST
Created attachment 360987 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6
Comment 75 Myles C. Maxfield 2019-02-04 11:39:05 PST
Created attachment 361078 [details]
Patch
Comment 76 Build Bot 2019-02-04 11:42:38 PST
Attachment 361078 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/cocoa/SystemFontDatabaseCoreText.cpp:217:  Misplaced OS version check. Please use a named macro in wtf/Platform.h, wtf/FeatureDefines.h, or an appropriate internal file.  [build/version_check] [5]
ERROR: Source/WebCore/platform/graphics/cocoa/SystemFontDatabaseCoreText.cpp:222:  Misplaced OS version check. Please use a named macro in wtf/Platform.h, wtf/FeatureDefines.h, or an appropriate internal file.  [build/version_check] [5]
Total errors found: 2 in 17 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 77 Jiang Jiang 2019-02-04 11:46:19 PST
Comment on attachment 361078 [details]
Patch

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

> Source/WebCore/page/cocoa/SettingsBaseCocoa.mm:53
>      setStandardFontFamily("Songti TC", USCRIPT_TRADITIONAL_HAN);

Do we still need this? Can it be from CTFontDescriptorCreateWithCSSFamily()?

> Source/WebCore/page/cocoa/SettingsBaseCocoa.mm:54
>      setStandardFontFamily("Songti SC", USCRIPT_SIMPLIFIED_HAN);

Do we still need this? Can it be from CTFontDescriptorCreateWithCSSFamily()?

> Source/WebCore/page/cocoa/SettingsBaseCocoa.mm:55
>      setStandardFontFamily("Hiragino Mincho ProN", USCRIPT_KATAKANA_OR_HIRAGANA);

Do we still need this? Can it be from CTFontDescriptorCreateWithCSSFamily()?

> Source/WebCore/page/cocoa/SettingsBaseCocoa.mm:56
>      setStandardFontFamily("AppleMyungjo", USCRIPT_HANGUL);

Do we still need this? Can it be from CTFontDescriptorCreateWithCSSFamily()?
Comment 78 Myles C. Maxfield 2019-02-04 13:36:25 PST
Comment on attachment 361078 [details]
Patch

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

>> Source/WebCore/page/cocoa/SettingsBaseCocoa.mm:53
>>      setStandardFontFamily("Songti TC", USCRIPT_TRADITIONAL_HAN);
> 
> Do we still need this? Can it be from CTFontDescriptorCreateWithCSSFamily()?

CTFontDescriptorCreateWithCSSFamily() doesn't have a "standard family" concept.
Comment 79 Myles C. Maxfield 2019-02-04 15:02:20 PST
Comment on attachment 361078 [details]
Patch

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

> Source/WebCore/platform/graphics/cocoa/SystemFontDatabaseCoreText.cpp:217
> +#if PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED < 101500

This is supposed to be in platform.h

> Source/WebCore/platform/graphics/cocoa/SystemFontDatabaseCoreText.cpp:222
> +#elif PLATFORM(IOS_FAMILY) && __IPHONE_OS_VERSION_MIN_REQUIRED < 130000

Ditto
Comment 80 Jiang Jiang 2019-02-04 15:08:59 PST
(In reply to Myles C. Maxfield from comment #78)
> Comment on attachment 361078 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=361078&action=review
> 
> >> Source/WebCore/page/cocoa/SettingsBaseCocoa.mm:53
> >>      setStandardFontFamily("Songti TC", USCRIPT_TRADITIONAL_HAN);
> > 
> > Do we still need this? Can it be from CTFontDescriptorCreateWithCSSFamily()?
> 
> CTFontDescriptorCreateWithCSSFamily() doesn't have a "standard family"
> concept.

But even if we do, it won't be helpful. The fact that Safari/WebKit picks serif fonts as standard family is purely artificial, the system won't pick that anyway.
Comment 81 Shawn Roberts 2019-02-05 11:01:58 PST
Test is a flaky failure and crash on Mac WK2

https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=http%2Fwpt%2Fmediarecorder%2FMediaRecorder-AV-audio-video-dataavailable.html

Reproducible locally with:

run-webkit-tests d240977 http/wpt/mediarecorder/MediaRecorder-AV-audio-video-dataavailable.html --iterations 500 -f --debug --exit-after-n-failures=1 --no-retry-failures

Crash log : Assertion failure
https://build.webkit.org/results/Apple%20Mojave%20Debug%20WK2%20(Tests)/r240972%20(1465)/http/wpt/mediarecorder/MediaRecorder-AV-audio-video-dataavailable-crash-log.txt

ASSERTION FAILED: dlsym(0x7fcdf7532a50, AVEncoderBitRateKey): symbol not found
constant
./platform/mediarecorder/cocoa/MediaRecorderPrivateWriterCocoa.mm(55) : NSString *initAVEncoderBitRateKey()
1   0x11b590e09 WTFCrash
2   0x10bca0aba initAVEncoderBitRateKey()
3   0x10bc9a887 WebCore::MediaRecorderPrivateWriter::setAudioInput()
4   0x10bc9a434 WebCore::MediaRecorderPrivateWriter::create(WebCore::MediaStreamTrackPrivate const*, WebCore::MediaStreamTrackPrivate const*)
5   0x10e51f33a WebCore::MediaRecorderPrivateAVFImpl::create(WebCore::MediaStreamPrivate const&)
Comment 82 Myles C. Maxfield 2019-02-05 13:09:53 PST
(In reply to Shawn Roberts from comment #81)
> Test is a flaky failure and crash on Mac WK2
> 
> https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.
> html#showAllRuns=true&tests=http%2Fwpt%2Fmediarecorder%2FMediaRecorder-AV-
> audio-video-dataavailable.html

This patch has nothing to do with MediaRecorder.
Comment 83 Brent Fulgham 2019-02-08 08:38:12 PST
Comment on attachment 361078 [details]
Patch

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

> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:-1723
> -

Doesn't seem like we need this change.

> Source/WebCore/css/parser/CSSPropertyParser.cpp:1070
> +        if (RefPtr<CSSValue> parsedValue = consumeGenericFamily(range))

Could this be auto?

>> Source/WebCore/platform/graphics/cocoa/SystemFontDatabaseCoreText.cpp:217
>> +#if PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED < 101500
> 
> This is supposed to be in platform.h

I don't understand this statement. What is? The PLATFORM macro?
Comment 84 Alexey Proskuryakov 2019-02-08 09:04:05 PST
> I don't understand this statement. What is? The PLATFORM macro?

As style checker said, "Please use a named macro in wtf/Platform.h, wtf/FeatureDefines.h, or an appropriate internal file."

> 2   0x10bca0aba initAVEncoderBitRateKey()

Bug 193724. Crazy stuff.
Comment 85 Myles C. Maxfield 2019-02-08 19:03:49 PST
Created attachment 361580 [details]
Patch for committing
Comment 86 WebKit Commit Bot 2019-02-08 19:42:28 PST
Comment on attachment 361580 [details]
Patch for committing

Clearing flags on attachment: 361580

Committed r241229: <https://trac.webkit.org/changeset/241229>
Comment 87 Darin Adler 2019-02-09 12:36:04 PST
Comment on attachment 361078 [details]
Patch

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

>>> Source/WebCore/platform/graphics/cocoa/SystemFontDatabaseCoreText.cpp:217
>>> +#if PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED < 101500
>> 
>> This is supposed to be in platform.h
> 
> I don't understand this statement. What is? The PLATFORM macro?

Alexey recently established a WebKit coding style rule that we don’t put specific iOS or macOS version checks into source files. Instead, all such checks are named HAVE() or ENABLE() definitions at the bottom of Platform.h.

We still have a couple hundred old call sites to clean up, but I think we’re pretty solid on this being the new strategy for many reasons. I don’t know if we have done a good enough job announcing and documenting this.
Comment 88 Darin Adler 2019-02-09 12:36:58 PST
Comment on attachment 361078 [details]
Patch

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

>>>> Source/WebCore/platform/graphics/cocoa/SystemFontDatabaseCoreText.cpp:217
>>>> +#if PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED < 101500
>>> 
>>> This is supposed to be in platform.h
>> 
>> I don't understand this statement. What is? The PLATFORM macro?
> 
> Alexey recently established a WebKit coding style rule that we don’t put specific iOS or macOS version checks into source files. Instead, all such checks are named HAVE() or ENABLE() definitions at the bottom of Platform.h.
> 
> We still have a couple hundred old call sites to clean up, but I think we’re pretty solid on this being the new strategy for many reasons. I don’t know if we have done a good enough job announcing and documenting this.

Oh, I see Alexey already replied about this, but I didn’t see it since it was a bug comment rather than a patch review comment response.
Comment 89 Darin Adler 2019-02-09 12:38:12 PST
Comment on attachment 361580 [details]
Patch for committing

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

> Source/WebCore/platform/graphics/cocoa/FontDescriptionCocoa.cpp:171
> +        static int dummy;

Maybe char instead of int will save a tiny bit of memory?
Comment 90 Darin Adler 2019-02-09 12:39:53 PST
Comment on attachment 361580 [details]
Patch for committing

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

> Source/WebCore/platform/graphics/cocoa/SystemFontDatabaseCoreText.cpp:185
> +    return m_serifFamilies.ensure(locale, [&] {
> +        auto descriptor = adoptCF(CTFontDescriptorCreateForCSSFamily(kCTFontCSSFamilySerif, locale.createCFString().get()));
> +        return adoptCF(static_cast<CFStringRef>(CTFontDescriptorCopyAttribute(descriptor.get(), kCTFontFamilyNameAttribute))).get();
> +    }).iterator->value;

Seems like we could write a helper so these 4 almost identical functions weren’t so repetitive.
Comment 91 Truitt Savell 2019-02-11 10:22:23 PST
Reverted r241229 for reason:

Revision broke internal builds for watchOS.

Committed r241268: <https://trac.webkit.org/changeset/241268>
Comment 92 Myles C. Maxfield 2019-02-11 17:17:12 PST
Created attachment 361737 [details]
Patch for committing
Comment 93 Myles C. Maxfield 2019-02-11 17:19:12 PST
Created attachment 361738 [details]
Patch for committing
Comment 94 WebKit Commit Bot 2019-02-11 18:19:25 PST
Comment on attachment 361738 [details]
Patch for committing

Clearing flags on attachment: 361738

Committed r241288: <https://trac.webkit.org/changeset/241288>
Comment 95 Shawn Roberts 2019-02-12 11:08:53 PST
(In reply to Myles C. Maxfield from comment #82)
> (In reply to Shawn Roberts from comment #81)
> > Test is a flaky failure and crash on Mac WK2
> > 
> > https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.
> > html#showAllRuns=true&tests=http%2Fwpt%2Fmediarecorder%2FMediaRecorder-AV-
> > audio-video-dataavailable.html
> 
> This patch has nothing to do with MediaRecorder.

Sorry, I filed on the wrong bug.