[Cocoa] Ask platform for generic font family mappings
Created attachment 345137 [details] WIP
<rdar://problem/41892438>
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.
Created attachment 345200 [details] Patch
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 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.
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 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
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
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
WebKit sets all of these generic font families to default values (see WebPreferencesDefaultValues.h)
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.
+[NSFont userFixedPitchFontOfSize] :(
Looks like WebKit2 doesn't have API for changing these generic font families. (Though it does have some SPI: WKPreferencesSetSansSerifFontFamily)
Created attachment 346259 [details] Patch
*** Bug 187895 has been marked as a duplicate of this bug. ***
Created attachment 346260 [details] Patch
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
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
Created attachment 360532 [details] Patch
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
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 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
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 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
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 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
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
Created attachment 360610 [details] Patch
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 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
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?
(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?
See the comment in the next section: // There is no serif Chinese font in default iOS installation.
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
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 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
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 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
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 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
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
Created attachment 360654 [details] Patch
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
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 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
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 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
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 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
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
*** Bug 167907 has been marked as a duplicate of this bug. ***
Created attachment 360887 [details] Patch
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
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 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
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 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
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 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
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
Created attachment 360917 [details] No exceptions
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
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 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
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 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
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
Created attachment 360968 [details] No exceptions
Created attachment 360982 [details] Patch
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
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 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
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
Created attachment 361078 [details] Patch
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 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 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 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
(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.
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&)
(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 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?
> 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.
Created attachment 361580 [details] Patch for committing
Comment on attachment 361580 [details] Patch for committing Clearing flags on attachment: 361580 Committed r241229: <https://trac.webkit.org/changeset/241229>
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 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 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 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.
Reverted r241229 for reason: Revision broke internal builds for watchOS. Committed r241268: <https://trac.webkit.org/changeset/241268>
Created attachment 361737 [details] Patch for committing
Created attachment 361738 [details] Patch for committing
Comment on attachment 361738 [details] Patch for committing Clearing flags on attachment: 361738 Committed r241288: <https://trac.webkit.org/changeset/241288>
(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.
*** Bug 152322 has been marked as a duplicate of this bug. ***