Changeset 269437 in webkit


Ignore:
Timestamp:
Nov 5, 2020 8:58:46 AM (21 months ago)
Author:
Aditya Keerthi
Message:

[macOS] Toggling dark mode does not update the scrollbar appearance in overflow: scroll elements
​https://bugs.webkit.org/show_bug.cgi?id=218538
<rdar://problem/68953006>

Reviewed by Simon Fraser.

Source/WebCore:

This functionality regressed in r260276 with the introduction of async
overflow scrolling on macOS. The appearance of the scrollbar is determined
by [WebScrollerImpDelegate effectiveAppearanceForScrollerImp:]. Prior to
r260276, ScrollbarThemeMac::paint was responsible for painting the scrollbar,
using [NSScrollerImp drawKnob]. Calling this method results in a call to
effectiveAppearanceForScrollerImp:, ensuring the scrollbar has the correct
appearance.

However, async overflow scrolling no longer paints the scrollbar using
drawKnob, and a call to effectiveAppearanceForScrollImp: is no longer made
when switching between light/dark appearances. Since we no longer draw the
knob ourselves, and the NSScrollerImp does not belong to an NSView, the
scrollbar does not automatically get repainted on an appearance change.
To fix, we need to ensure the scrollbar is repainted by making a call to
[NSScrollerImp setNeedsDisplay:] when the page's appearance changes. This
should be done for all scrollbars to ensure their appearance is up to date.

Note that hovering over the scrollbar still results in an update to its
appearance, since AppKit always repaints the scrollbar on hover.

Test: fast/scrolling/mac/overflow-scrollbars-toggle-dark-mode.html

  • dom/Document.cpp:

(WebCore::Document::invalidateScrollbars): Tell the FrameView to invalidate scrollbars in all its ScrollableAreas.

  • dom/Document.h:
  • page/FrameView.cpp:

(WebCore::FrameView::invalidateScrollbarsForAllScrollableAreas):

  • page/FrameView.h:
  • page/Page.cpp:

(WebCore::Page::appearanceDidChange): Tell each document to invalidate its scrollbars.

  • platform/ScrollableArea.cpp:

(WebCore::ScrollableArea::setScrollbarOverlayStyle):
(WebCore::ScrollableArea::invalidateScrollbars):

Moved scrollbar invalidation out of setScrollbarOverlayStyle and into
its own method, so that it can be called for all ScrollableAreas in a
FrameView.

  • platform/ScrollableArea.h:

LayoutTests:

Added a layout test which draws a scrollbar in the dark appearance,
switches to the light appearance, and draws the scrollbar again. The
final image should have a light appearance scrollbar.

  • fast/scrolling/mac/overflow-scrollbars-toggle-dark-mode-expected.html: Added.
  • fast/scrolling/mac/overflow-scrollbars-toggle-dark-mode.html: Added.
Location:
trunk
Files:
2 added
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r269436 r269437  
     12020-11-05  Aditya Keerthi  <akeerthi@apple.com>
     2
     3        [macOS] Toggling dark mode does not update the scrollbar appearance in overflow: scroll elements
     4        https://bugs.webkit.org/show_bug.cgi?id=218538
     5        <rdar://problem/68953006>
     6
     7        Reviewed by Simon Fraser.
     8
     9        Added a layout test which draws a scrollbar in the dark appearance,
     10        switches to the light appearance, and draws the scrollbar again. The
     11        final image should have a light appearance scrollbar.
     12
     13        * fast/scrolling/mac/overflow-scrollbars-toggle-dark-mode-expected.html: Added.
     14        * fast/scrolling/mac/overflow-scrollbars-toggle-dark-mode.html: Added.
     15
    1162020-11-05  Youenn Fablet  <youenn@apple.com>
    217

  • trunk/Source/WebCore/ChangeLog

    r269435 r269437  
     12020-11-05  Aditya Keerthi  <akeerthi@apple.com>
     2
     3        [macOS] Toggling dark mode does not update the scrollbar appearance in overflow: scroll elements
     4        https://bugs.webkit.org/show_bug.cgi?id=218538
     5        <rdar://problem/68953006>
     6
     7        Reviewed by Simon Fraser.
     8
     9        This functionality regressed in r260276 with the introduction of async
     10        overflow scrolling on macOS. The appearance of the scrollbar is determined
     11        by [WebScrollerImpDelegate effectiveAppearanceForScrollerImp:]. Prior to
     12        r260276, ScrollbarThemeMac::paint was responsible for painting the scrollbar,
     13        using [NSScrollerImp drawKnob]. Calling this method results in a call to
     14        effectiveAppearanceForScrollerImp:, ensuring the scrollbar has the correct
     15        appearance.
     16
     17        However, async overflow scrolling no longer paints the scrollbar using
     18        drawKnob, and a call to effectiveAppearanceForScrollImp: is no longer made
     19        when switching between light/dark appearances. Since we no longer draw the
     20        knob ourselves, and the NSScrollerImp does not belong to an NSView, the
     21        scrollbar does not automatically get repainted on an appearance change.
     22        To fix, we need to ensure the scrollbar is repainted by making a call to
     23        [NSScrollerImp setNeedsDisplay:] when the page's appearance changes. This
     24        should be done for all scrollbars to ensure their appearance is up to date.
     25
     26        Note that hovering over the scrollbar still results in an update to its
     27        appearance, since AppKit always repaints the scrollbar on hover.
     28
     29        Test: fast/scrolling/mac/overflow-scrollbars-toggle-dark-mode.html
     30
     31        * dom/Document.cpp:
     32        (WebCore::Document::invalidateScrollbars): Tell the FrameView to invalidate scrollbars in all its ScrollableAreas.
     33        * dom/Document.h:
     34        * page/FrameView.cpp:
     35        (WebCore::FrameView::invalidateScrollbarsForAllScrollableAreas):
     36        * page/FrameView.h:
     37        * page/Page.cpp:
     38        (WebCore::Page::appearanceDidChange): Tell each document to invalidate its scrollbars.
     39        * platform/ScrollableArea.cpp:
     40        (WebCore::ScrollableArea::setScrollbarOverlayStyle):
     41        (WebCore::ScrollableArea::invalidateScrollbars):
     42
     43        Moved scrollbar invalidation out of setScrollbarOverlayStyle and into
     44        its own method, so that it can be called for all ScrollableAreas in a
     45        FrameView.
     46
     47        * platform/ScrollableArea.h:
     48
    1492020-11-05  Alex Christensen  <achristensen@webkit.org>
    250

  • trunk/Source/WebCore/dom/Document.cpp

    r269435 r269437  
    41574157}
    41584158
     4159void Document::invalidateScrollbars()
     4160{
     4161    if (auto* frameView = view())
     4162        frameView->invalidateScrollbarsForAllScrollableAreas();
     4163}
     4164
    41594165void Document::addAudioProducer(MediaProducer& audioProducer)
    41604166{

  • trunk/Source/WebCore/dom/Document.h

    r269435 r269437  
    13701370    void runScrollSteps();
    13711371
     1372    void invalidateScrollbars();
     1373
    13721374    WEBCORE_EXPORT void addAudioProducer(MediaProducer&);
    13731375    WEBCORE_EXPORT void removeAudioProducer(MediaProducer&);

  • trunk/Source/WebCore/page/FrameView.cpp

    r269160 r269437  
    817817}
    818818
     819void FrameView::invalidateScrollbarsForAllScrollableAreas()
     820{
     821    if (!m_scrollableAreas)
     822        return;
     823
     824    for (auto* scrollableArea : *m_scrollableAreas)
     825        scrollableArea->invalidateScrollbars();
     826}
     827
    819828GraphicsLayer* FrameView::layerForHorizontalScrollbar() const
    820829{

  • trunk/Source/WebCore/page/FrameView.h

    r268031 r269437  
    669669    void invalidateImagesWithAsyncDecodes() { traverseForPaintInvalidation(GraphicsContext::PaintInvalidationReasons::InvalidatingImagesWithAsyncDecodes); }
    670670
     671    void invalidateScrollbarsForAllScrollableAreas();
     672
    671673    GraphicsLayer* layerForHorizontalScrollbar() const final;
    672674    GraphicsLayer* layerForVerticalScrollbar() const final;

  • trunk/Source/WebCore/page/Page.cpp

    r269348 r269437  
    30123012        document.updateElementsAffectedByMediaQueries();
    30133013        document.scheduleRenderingUpdate(RenderingUpdateStep::MediaQueryEvaluation);
     3014        document.invalidateScrollbars();
    30143015    });
    30153016}

  • trunk/Source/WebCore/platform/ScrollableArea.cpp

    r269373 r269437  
    369369    m_scrollbarOverlayStyle = overlayStyle;
    370370
    371     if (horizontalScrollbar()) {
    372         ScrollbarTheme::theme().updateScrollbarOverlayStyle(*horizontalScrollbar());
    373         horizontalScrollbar()->invalidate();
    374         if (ScrollAnimator* scrollAnimator = existingScrollAnimator())
    375             scrollAnimator->invalidateScrollbarPartLayers(horizontalScrollbar());
    376     }
    377    
    378     if (verticalScrollbar()) {
    379         ScrollbarTheme::theme().updateScrollbarOverlayStyle(*verticalScrollbar());
    380         verticalScrollbar()->invalidate();
    381         if (ScrollAnimator* scrollAnimator = existingScrollAnimator())
    382             scrollAnimator->invalidateScrollbarPartLayers(verticalScrollbar());
     371    if (auto* scrollbar = horizontalScrollbar())
     372        ScrollbarTheme::theme().updateScrollbarOverlayStyle(*scrollbar);
     373
     374    if (auto* scrollbar = verticalScrollbar())
     375        ScrollbarTheme::theme().updateScrollbarOverlayStyle(*scrollbar);
     376
     377    invalidateScrollbars();
     378}
     379
     380void ScrollableArea::invalidateScrollbars()
     381{
     382    if (auto* scrollbar = horizontalScrollbar()) {
     383        scrollbar->invalidate();
     384        if (auto* scrollAnimator = existingScrollAnimator())
     385            scrollAnimator->invalidateScrollbarPartLayers(scrollbar);
     386    }
     387
     388    if (auto* scrollbar = verticalScrollbar()) {
     389        scrollbar->invalidate();
     390        if (auto* scrollAnimator = existingScrollAnimator())
     391            scrollAnimator->invalidateScrollbarPartLayers(scrollbar);
    383392    }
    384393}

  • trunk/Source/WebCore/platform/ScrollableArea.h

    r269373 r269437  
    172172    WEBCORE_EXPORT virtual void setScrollbarOverlayStyle(ScrollbarOverlayStyle);
    173173    ScrollbarOverlayStyle scrollbarOverlayStyle() const { return static_cast<ScrollbarOverlayStyle>(m_scrollbarOverlayStyle); }
     174    void invalidateScrollbars();
    174175    bool useDarkAppearanceForScrollbars() const;
    175176
Note: See TracChangeset for help on using the changeset viewer.