-
<rdar://problem/83530228>
https://github.com/w3c/csswg-drafts/commit/1c74611151b452930609627a0de2412c0cb86175
Created attachment 439273 [details] Patch to make FontCreationContext optional
Created attachment 439274 [details] Patch to implement light/dark
Comment on attachment 439273 [details] Patch to make FontCreationContext optional The FontCreationContext work is moved to https://bugs.webkit.org/show_bug.cgi?id=230800
Created attachment 439279 [details] Patch EWS will be red because this patch depends on https://bugs.webkit.org/show_bug.cgi?id=230800.
This patch needs tests.
Created attachment 439566 [details] Patch
Created attachment 439636 [details] Test font
Created attachment 439763 [details] Patch
This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess
Comment on attachment 439763 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=439763&action=review > LayoutTests/imported/w3c/ChangeLog:9 > + I will open a WPT PR for these posthaste. https://github.com/web-platform-tests/wpt/pull/31044
Created attachment 439805 [details] Patch
Comment on attachment 439805 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=439805&action=review > Source/WebCore/ChangeLog:26 > + int64_t integer; Does the spec require these be 64-bit? > Source/WebCore/platform/graphics/FontPaletteValues.h:40 > + FontPaletteIndex() = default > Source/WebCore/platform/graphics/FontPaletteValues.h:83 > + enum class Type { : uint8_t ? > Source/WebCore/platform/graphics/FontPaletteValues.h:88 > + } type { Type::String }; I like to use 'const' for members that can only ever be set in the constructor. > Source/WebCore/platform/graphics/FontPaletteValues.h:90 > + unsigned integer { 0 }; You had int64_t in the changelog > Source/WebCore/platform/graphics/cocoa/FontCacheCoreText.cpp:472 > + int64_t rawIndex = basePalette.integer; // There is no kCFNumberUIntType. > + auto number = adoptCF(CFNumberCreate(kCFAllocatorDefault, kCFNumberSInt64Type, &rawIndex)); > + CFDictionaryAddValue(attributes, kCTFontPaletteAttribute, number.get()); Can author-supplied values conflict with kCTFontPaletteLight, kCTFontPaletteDark?
Comment on attachment 439805 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=439805&action=review >> Source/WebCore/ChangeLog:26 >> + int64_t integer; > > Does the spec require these be 64-bit? Nope. In fact, our parser clamps to 32-bit. I'll update the changelog to be "unsigned" to match the implementation. >> Source/WebCore/platform/graphics/cocoa/FontCacheCoreText.cpp:472 >> + CFDictionaryAddValue(attributes, kCTFontPaletteAttribute, number.get()); > > Can author-supplied values conflict with kCTFontPaletteLight, kCTFontPaletteDark? It depends if https://github.com/w3c/csswg-drafts/issues/6627 is accepted or not. If it's accepted (which I hope it is), there won't be a collision.
Comment on attachment 439805 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=439805&action=review >> Source/WebCore/platform/graphics/FontPaletteValues.h:88 >> + } type { Type::String }; > > I like to use 'const' for members that can only ever be set in the constructor. It looks like that, because this is used as a key in a hash map, and our hash map infrastructure uses operator= to overwrite the buckets in the table, all the fields have to be assignable. :(
Committed r283398 (242402@main): <https://commits.webkit.org/242402@main>