[webkit-reviews] review granted: [Bug 122608] Add SPI for telling WebKit to prefer pictograph glyphs over monochrome ones : [Attachment 213908] Patch

[bugzilla-daemon at webkit.org]

mitz at webkit.org <mitz at webkit.org> has granted Andy Estes <aestes at apple.com>'s
request for review:
Bug 122608: Add SPI for telling WebKit to prefer pictograph glyphs over
monochrome ones
https://bugs.webkit.org/show_bug.cgi?id=122608

Attachment 213908: Patch
https://bugs.webkit.org/attachment.cgi?id=213908&action=review

------- Additional Comments from mitz at webkit.org <mitz at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=213908&action=review


> Source/WebKit/mac/WebView/WebView.mm:3161
> +- (void)_setFontFallbackPrefersPictographs:(BOOL)flag
> +{
> +    if (_private->page)
> +	   _private->page->settings().setFontFallbackPrefersPictographs(flag);
> +}

It’s clearly wrong for a WebView instance method to update the shared Settings
object. Looking at the history of this code, I see that originally I had used
WebPreferences for this, but then for some reason external to WebKit I had to
change this into an instance method. It‘s OK to keep the code as-is for now,
but please add a FIXME here about how this is wrong, and a comment next to the
method declaration about how it affects all WebViews sharing the same
WebPreferences instance.