Bug 222078 - Experiment with using the theme-color as the scroll area background if provided
Summary: Experiment with using the theme-color as the scroll area background if provided
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Devin Rousso
URL:
Keywords: InRadar
Depends on:
Blocks: 224455
  Show dependency tree / graph
 
Reported: 2021-02-17 15:09 PST by Devin Rousso
Modified: 2021-04-12 14:47 PDT (History)
14 users (show)

See Also:

Attachments
Patch (8.67 KB, patch)
2021-02-17 15:10 PST, Devin Rousso 		thorton: review+
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (8.85 KB, patch)
2021-02-17 15:49 PST, Devin Rousso 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (8.85 KB, patch)
2021-02-17 15:52 PST, Devin Rousso 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (8.86 KB, patch)
2021-02-17 15:57 PST, Devin Rousso 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (8.81 KB, patch)
2021-02-17 16:11 PST, Devin Rousso 		no flags Details | Formatted Diff | Diff
Patch (8.82 KB, patch)
2021-02-17 16:12 PST, Devin Rousso 		no flags Details | Formatted Diff | Diff
Patch (8.82 KB, patch)
2021-02-17 16:13 PST, Devin Rousso 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (8.82 KB, patch)
2021-02-17 16:24 PST, Devin Rousso 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (8.79 KB, patch)
2021-02-17 17:38 PST, Devin Rousso 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (8.83 KB, patch)
2021-02-17 17:59 PST, Devin Rousso 		no flags Details | Formatted Diff | Diff
Show Obsolete (9) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Devin Rousso 2021-02-17 15:09:19 PST 
`<meta name="theme-color" content="...">` would be used (if provided) instead of the `pageExtendedBackgroundColor`
Comment 1 Devin Rousso 2021-02-17 15:09:36 PST 
<rdar://problem/74158818>
Comment 2 Devin Rousso 2021-02-17 15:10:21 PST 
Created attachment 420730 [details]
Patch
Comment 3 Tim Horton 2021-02-17 15:20:10 PST 
Comment on attachment 420730 [details]
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?
Comment 4 Devin Rousso 2021-02-17 15:48:02 PST 
Comment on attachment 420730 [details]
Patch

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

>> Source/WTF/Scripts/Preferences/WebPreferencesInternal.yaml:781
>> +  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

talked offline about this and since it's an internal setting (and the existing title is already pretty succinct) i'll leave this as is

>> Source/WebKit/UIProcess/ViewSnapshotStore.cpp:99
>> +    if (!backgroundColor.isValid())
> 
> Maybe a WebPageProxy getter to share this with WKWebViewIOS?

smart
Comment 5 Devin Rousso 2021-02-17 15:49:14 PST 
Created attachment 420737 [details]
Patch
Comment 6 Devin Rousso 2021-02-17 15:52:21 PST 
Created attachment 420742 [details]
Patch
Comment 7 Devin Rousso 2021-02-17 15:57:27 PST 
Created attachment 420743 [details]
Patch
Comment 8 Devin Rousso 2021-02-17 16:11:17 PST 
Created attachment 420751 [details]
Patch
Comment 9 Devin Rousso 2021-02-17 16:12:30 PST 
Created attachment 420752 [details]
Patch
Comment 10 Devin Rousso 2021-02-17 16:13:11 PST 
Created attachment 420753 [details]
Patch
Comment 11 Devin Rousso 2021-02-17 16:24:59 PST 
Created attachment 420755 [details]
Patch
Comment 12 Devin Rousso 2021-02-17 17:38:25 PST 
Created attachment 420766 [details]
Patch
Comment 13 Devin Rousso 2021-02-17 17:59:30 PST 
Created attachment 420774 [details]
Patch
Comment 14 EWS 2021-02-18 10:38:19 PST 
Committed r273083: <https://commits.webkit.org/r273083>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 420774 [details].