[webkit-reviews] review granted: [Bug 222078] experiment with using the theme-color as the scroll area background if provided : [Attachment 420730] Patch

[bugzilla-daemon at webkit.org]

Tim Horton <thorton at apple.com> has granted Devin Rousso <drousso at apple.com>'s
request for review:
Bug 222078: experiment with using the theme-color as the scroll area background
if provided
https://bugs.webkit.org/show_bug.cgi?id=222078

Attachment 420730: Patch

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




--- Comment #3 from Tim Horton <thorton at apple.com> ---
Comment on attachment 420730
  --> https://bugs.webkit.org/attachment.cgi?id=420730
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=420730&action=review

> Source/WTF/ChangeLog:3
> +	   experiment with using the theme-color as the scroll area background
if provided

Title could use some ... title casing. Or y'know at least a starting capital.

> Source/WTF/Scripts/Preferences/WebPreferencesInternal.yaml:781
> +  humanReadableName: "Use theme-color for scroll area background color"
> +  humanReadableDescription: "Use theme-color for scroll area background
color"

I hover the first string and see the second? Now I've wasted 3 seconds of my
life.

I would try shortening up the first one, it's pretty long for a menu item

> Source/WebKit/UIProcess/ViewSnapshotStore.cpp:99
> +	   backgroundColor = webPageProxy.themeColor();
> +    if (!backgroundColor.isValid())

Maybe a WebPageProxy getter to share this with WKWebViewIOS?