Bug 191559 - Use a light scrollbar for transparent web views in dark mode
Summary: Use a light scrollbar for transparent web views in dark mode
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Timothy Hatcher
URL:
Keywords: InRadar
Depends on: 191556
Blocks: 191607
  Show dependency treegraph
 
Reported: 2018-11-12 13:03 PST by Timothy Hatcher
Modified: 2018-11-13 17:15 PST (History)
8 users (show)

See Also:


Attachments
Patch (12.58 KB, patch)
2018-11-12 16:07 PST, Timothy Hatcher
no flags Details | Formatted Diff | Diff
Patch (24.02 KB, patch)
2018-11-13 15:38 PST, Timothy Hatcher
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Timothy Hatcher 2018-11-12 13:03:29 PST
Also need to treat a transparent background in dark mode as dark so the scrollbar thumb will be white.
Comment 1 Radar WebKit Bug Importer 2018-11-12 13:03:56 PST
<rdar://problem/46000489>
Comment 2 Timothy Hatcher 2018-11-12 13:05:43 PST Comment hidden (obsolete)
Comment 3 Timothy Hatcher 2018-11-12 16:07:01 PST Comment hidden (obsolete)
Comment 4 Timothy Hatcher 2018-11-13 15:38:06 PST
Created attachment 354713 [details]
Patch
Comment 5 EWS Watchlist 2018-11-13 15:40:22 PST
Attachment 354713 [details] did not pass style-queue:


ERROR: Source/WebCore/rendering/style/RenderStyle.cpp:962:  Line contains only semicolon. If this should be an empty statement, use { } instead.  [whitespace/semicolon] [5]
Total errors found: 1 in 19 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 6 Dean Jackson 2018-11-13 16:50:13 PST
Comment on attachment 354713 [details]
Patch

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

> Source/WebCore/page/FrameView.cpp:2058
> +    auto* document = frame().document();
> +    auto* documentElement = document ? document->documentElement() : nullptr;
> +    auto* documentElementRenderer = documentElement ? documentElement->renderer() : nullptr;
> +    if (documentElementRenderer && documentElementRenderer->style().hasExplicitlySetSupportedColorSchemes())
> +        return documentElementRenderer;
> +    auto* bodyElement = document ? document->bodyOrFrameset() : nullptr;
> +    return bodyElement ? bodyElement->renderer() : nullptr;

Don't we normally do early returns? A null document would go through 5 extra checks before returning null here.

> Source/WebCore/testing/Internals.cpp:2507
> +        return "default"_str;

First time I've seen someone else use it!
Comment 7 Timothy Hatcher 2018-11-13 16:56:38 PST
Comment on attachment 354713 [details]
Patch

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

>> Source/WebCore/page/FrameView.cpp:2058
>> +    return bodyElement ? bodyElement->renderer() : nullptr;
> 
> Don't we normally do early returns? A null document would go through 5 extra checks before returning null here.

Yes, I could return early for !document. I moved this out of another function, and forgot to change it.

>> Source/WebCore/testing/Internals.cpp:2507
>> +        return "default"_str;
> 
> First time I've seen someone else use it!

Haha.
Comment 8 WebKit Commit Bot 2018-11-13 17:15:31 PST
Comment on attachment 354713 [details]
Patch

Clearing flags on attachment: 354713

Committed r238155: <https://trac.webkit.org/changeset/238155>
Comment 9 WebKit Commit Bot 2018-11-13 17:15:33 PST
All reviewed patches have been landed.  Closing bug.