MobileAsset fonts are broken in Reader mode in Safari
Created attachment 422913 [details] WIP
Created attachment 422917 [details] WIP
Created attachment 423003 [details] Needs test
Created attachment 423005 [details] Patch
Created attachment 423007 [details] Patch
Created attachment 423016 [details] Patch
Created attachment 423017 [details] Patch
Comment on attachment 423017 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=423017&action=review > Source/WebKit/UIProcess/API/C/WKPreferencesRefPrivate.h:550 > -// Defaults to false. > +// Defaults to true. Is this a correction to the comment, or a indicative of a behavior change? Changelog needs to explain. > Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:2242 > +- (void)_switchFromStaticFontRegistryToUserFontRegistry Since readers probably won't understand the implications of static vs. user font registries, a comment would help. > Source/WebKit/UIProcess/WebPageProxy.cpp:7868 > SandboxExtension::Handle fontMachExtensionHandle; > SandboxExtension::createHandleForMachLookup("com.apple.fonts"_s, WTF::nullopt, fontMachExtensionHandle); It's a bit odd to see this in two places? Share with switchFromStaticFontRegistryToUserFontRegistry()?
Comment on attachment 423017 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=423017&action=review > Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h:453 > +- (void)_switchFromStaticFontRegistryToUserFontRegistry WK_API_AVAILABLE(macos(WK_MAC_TBA)); Is there a way to switch back?
Comment on attachment 423017 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=423017&action=review >> Source/WebKit/UIProcess/API/C/WKPreferencesRefPrivate.h:550 >> +// Defaults to true. > > Is this a correction to the comment, or a indicative of a behavior change? Changelog needs to explain. A correction. I'll explain in the ChangeLog. >> Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:2242 >> +- (void)_switchFromStaticFontRegistryToUserFontRegistry > > Since readers probably won't understand the implications of static vs. user font registries, a comment would help. Good idea. >> Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h:453 >> +- (void)_switchFromStaticFontRegistryToUserFontRegistry WK_API_AVAILABLE(macos(WK_MAC_TBA)); > > Is there a way to switch back? Nope!
Created attachment 423068 [details] Patch for committing
Created attachment 423070 [details] Patch for committing
Committed r274374: <https://commits.webkit.org/r274374> All reviewed patches have been landed. Closing bug and clearing flags on attachment 423070 [details].
<rdar://problem/75381965>
Comment on attachment 423017 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=423017&action=review >> Source/WebKit/UIProcess/WebPageProxy.cpp:7868 >> SandboxExtension::createHandleForMachLookup("com.apple.fonts"_s, WTF::nullopt, fontMachExtensionHandle); > > It's a bit odd to see this in two places? Share with switchFromStaticFontRegistryToUserFontRegistry()? Should CTFontManagerEnableAllUserFonts(true) be called when this extension is consumed in WP? > Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm:1169 > + m_fontMachExtension->consume(); To avoid storing the extension, you could also call SandboxExtension::consumePermanently here.
Comment on attachment 423017 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=423017&action=review >>> Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:2242 >>> +- (void)_switchFromStaticFontRegistryToUserFontRegistry >> >> Since readers probably won't understand the implications of static vs. user font registries, a comment would help. > > Good idea. Is this SPI strictly needed, since the client can use the preference shouldAllowUserInstalledFonts to enable this behavior?