Bug 218538

Summary: [macOS] Toggling dark mode does not update the scrollbar appearance in overflow: scroll elements
Product: WebKit Reporter: Aditya Keerthi <akeerthi>
Component: ScrollingAssignee: Aditya Keerthi <akeerthi>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, changseok, esprehn+autocc, ews-watchlist, fred.wang, glenn, kangil.han, kondapallykalyan, pdr, simon.fraser, thorton, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Safari 14   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
simon.fraser: review+
Patch for landing
none
Patch for landing none

Description Aditya Keerthi 2020-11-03 15:11:03 PST
The only way to update the scrollbar's appearance is by hovering over it.
Comment 1 Aditya Keerthi 2020-11-03 15:11:37 PST
<rdar://problem/68953006>
Comment 2 Aditya Keerthi 2020-11-03 15:13:58 PST
Created attachment 413113 [details]
Patch
Comment 3 Simon Fraser (smfr) 2020-11-03 15:54:59 PST
Comment on attachment 413113 [details]
Patch

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

> Source/WebCore/platform/ScrollableArea.cpp:370
>      m_scrollbarOverlayStyle = overlayStyle;
> +    updateScrollbarOverlayStyle();

Can you only call updateScrollbarOverlayStyle() if it changed?
Comment 4 Aditya Keerthi 2020-11-03 16:01:37 PST
(In reply to Simon Fraser (smfr) from comment #3)
> Comment on attachment 413113 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=413113&action=review
> 
> > Source/WebCore/platform/ScrollableArea.cpp:370
> >      m_scrollbarOverlayStyle = overlayStyle;
> > +    updateScrollbarOverlayStyle();
> 
> Can you only call updateScrollbarOverlayStyle() if it changed?

I think it should be fine to make that change here, but it would be unrelated to the solution.

Note that I've made RenderLayer::styleChanged unconditionally call updateScrollbarOverlayStyle(). This is because the scrollbar's overlay style is always ScrollbarOverlayStyleDefault (regardless of dark/light mode) and we need to call into `[NSScrollerImp setNeedsDisplay:]` to trigger the appearance change.
Comment 5 Simon Fraser (smfr) 2020-11-03 16:16:33 PST
Comment on attachment 413113 [details]
Patch

r- because it invalidates scrollbars on every style change which will be a perf hit.
Comment 6 Aditya Keerthi 2020-11-04 18:24:58 PST
Created attachment 413237 [details]
Patch
Comment 7 Simon Fraser (smfr) 2020-11-04 21:07:21 PST
Comment on attachment 413237 [details]
Patch

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

> Source/WebCore/platform/ScrollableArea.cpp:384
>          if (ScrollAnimator* scrollAnimator = existingScrollAnimator())

auto

> Source/WebCore/platform/ScrollableArea.cpp:390
>          if (ScrollAnimator* scrollAnimator = existingScrollAnimator())

auto
Comment 8 Aditya Keerthi 2020-11-05 07:26:59 PST
Created attachment 413295 [details]
Patch for landing
Comment 9 Aditya Keerthi 2020-11-05 08:19:41 PST
Created attachment 413301 [details]
Patch for landing
Comment 10 EWS 2020-11-05 08:58:50 PST
Committed r269437: <https://trac.webkit.org/changeset/269437>

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