[webkit-reviews] review granted: [Bug 223062] [macOS] MobileAsset fonts are broken in Reader mode in Safari : [Attachment 423017] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Myles C. Maxfield
<mmaxfield at apple.com>'s request for review:
Bug 223062: [macOS] MobileAsset fonts are broken in Reader mode in Safari
https://bugs.webkit.org/show_bug.cgi?id=223062

Attachment 423017: Patch

https://bugs.webkit.org/attachment.cgi?id=423017&action=review




--- Comment #8 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 423017
  --> https://bugs.webkit.org/attachment.cgi?id=423017
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()?