Fix incorrect custom pseudo-scrollbar sizes caused by scroller style changes on Mac.

GetToSet · nt1m · commit 560abdc4b294 · 2023-01-25T14:52:44.000-08:00
https://bugs.webkit.org/show_bug.cgi?id=249892 Reviewed by Simon Fraser. Scroller style changes can cause custom pseudo-scrollbar sizes (set by `width` or `height` property using `::-webkit-scrollbar`) to be overridden by AppKit defaults. This could cause unexpected visual appearances, especially when the scroller is thinner than the defaults. This usually happens when the user enters or leaves the trackpad-only setup (connecting or disconnecting the mouse for laptops would be a typical case). Manually toggling the scroller appearance in the system settings would be another way to reproduce this issue. This patch resolves the above issue by explicitly checking for custom scrollbars before replacing them with standard implementations when processing scroller style updates. * LayoutTests/TestExpectations: * LayoutTests/platform/mac/TestExpectations: * LayoutTests/scrollbars/custom-scrollbar-scroller-style-change-expected.txt: Added. * LayoutTests/scrollbars/custom-scrollbar-scroller-style-change.html: Added. * Source/WebCore/platform/mac/ScrollbarsControllerMac.mm: (WebCore::ScrollbarsControllerMac::updateScrollerStyle): * Source/WebCore/testing/Internals.cpp: (WebCore::Internals::setUsesOverlayScrollbars): * Source/WebCore/testing/Internals.mm: (WebCore::Internals::setUsesOverlayScrollbars): Canonical link: https://commits.webkit.org/259389@main
diff --git a/LayoutTests/TestExpectations b/LayoutTests/TestExpectations
@@ -291,6 +291,9 @@ fast/visual-viewport/rubberbanding-viewport-rects-header-footer.html  [ Skip ]
 # DataDetectors tests only make sense on Mac
 fast/events/do-not-drag-and-drop-data-detectors-link.html [ Skip ]
 
+# Testing against scroller style changes only makes sense on Mac
+scrollbars/custom-scrollbar-scroller-style-change.html [ Skip ]
+
 # A rounding error may cause 90deg 3d-rotated elements to count as painted
 imported/w3c/web-platform-tests/paint-timing/fcp-only/fcp-invisible-3d-rotate-descendant.html
 
diff --git a/LayoutTests/platform/mac/TestExpectations b/LayoutTests/platform/mac/TestExpectations
@@ -77,6 +77,8 @@ fast/harness/uiscriptcontroller [ Pass ]
 
 fast/events/do-not-drag-and-drop-data-detectors-link.html [ Pass ]
 
+scrollbars/custom-scrollbar-scroller-style-change.html [ Pass ]
+
 http/tests/gzip-content-encoding [ Pass ]
 
 http/tests/xmlhttprequest/gzip-content-type-no-content-encoding.html [ Pass ]
diff --git a/LayoutTests/scrollbars/custom-scrollbar-scroller-style-change-expected.txt b/LayoutTests/scrollbars/custom-scrollbar-scroller-style-change-expected.txt
@@ -0,0 +1,9 @@
+layer at (0,0) size 800x600
+  RenderView at (0,0) size 800x600
+layer at (0,0) size 800x216
+  RenderBlock {HTML} at (0,0) size 800x216
+    RenderBody {BODY} at (8,8) size 784x200
+layer at (8,8) size 200x200 clip at (8,8) size 152x152 scrollWidth 400 scrollHeight 400
+  RenderBlock {DIV} at (0,0) size 200x200
+layer at (8,8) size 400x400 backgroundClip at (8,8) size 152x152 clip at (8,8) size 152x152
+  RenderBlock {DIV} at (0,0) size 400x400 [bgcolor=#008000]
diff --git a/LayoutTests/scrollbars/custom-scrollbar-scroller-style-change.html b/LayoutTests/scrollbars/custom-scrollbar-scroller-style-change.html
@@ -0,0 +1,60 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ MockScrollbarsEnabled=false ] -->
+<html>
+<head>
+    <title>Scroller style changes should not change the size of custom scrollbars.</title>
+    <style>
+        ::-webkit-scrollbar {
+            width: 48px;
+            height: 48px;
+        }
+
+        ::-webkit-scrollbar-track {
+            background-color: #E3E3E3;
+        }
+
+        ::-webkit-scrollbar-thumb {
+            background: black;
+        }
+
+        .scroll-container {
+            overflow: scroll;
+            width: 200px;
+            height: 200px;
+        }
+
+        .overflowing {
+            width: 400px;
+            height: 400px;
+            background: green;
+        }
+
+        .composited {
+            transform: translateZ(0);
+        }
+    </style>
+    <script>
+        if (window.testRunner) {
+            testRunner.waitUntilDone();
+        }
+
+        function doTest() {
+            if (window.internals)
+                internals.setUsesOverlayScrollbars(true);
+            if (window.testRunner)
+                testRunner.notifyDone();
+        }
+
+        window.addEventListener('load', () => {
+            setTimeout(() => {
+                doTest()
+            }, 0);
+        }, false);
+    </script>
+</head>
+<body>
+    <!-- The scrollbars below should always sized 48x48 regardless of any scroller style changes. -->
+    <div class="scroll-container">
+        <div class="overflowing composited"></div>
+    </div>
+</body>
+</html>
diff --git a/Source/WebCore/platform/mac/ScrollbarsControllerMac.mm b/Source/WebCore/platform/mac/ScrollbarsControllerMac.mm
@@ -954,7 +954,8 @@ - (void)invalidate
 
     NSScrollerStyle newStyle = [m_scrollerImpPair scrollerStyle];
 
-    if (Scrollbar* verticalScrollbar = scrollableArea().verticalScrollbar()) {
+    Scrollbar* verticalScrollbar = scrollableArea().verticalScrollbar();
+    if (verticalScrollbar && !verticalScrollbar->isCustomScrollbar()) {
         verticalScrollbar->invalidate();
 
         NSScrollerImp *oldVerticalPainter = [m_scrollerImpPair verticalScrollerImp];
@@ -971,7 +972,8 @@ - (void)invalidate
         verticalScrollbar->setFrameRect(IntRect(0, 0, thickness, thickness));
     }
 
-    if (Scrollbar* horizontalScrollbar = scrollableArea().horizontalScrollbar()) {
+    Scrollbar* horizontalScrollbar = scrollableArea().horizontalScrollbar();
+    if (horizontalScrollbar && !horizontalScrollbar->isCustomScrollbar()) {
         horizontalScrollbar->invalidate();
 
         NSScrollerImp *oldHorizontalPainter = [m_scrollerImpPair horizontalScrollerImp];
diff --git a/Source/WebCore/testing/Internals.cpp b/Source/WebCore/testing/Internals.cpp
@@ -352,8 +352,6 @@
 
 #if PLATFORM(MAC)
 #include "GraphicsChecksMac.h"
-#include "NSScrollerImpDetails.h"
-#include "ScrollbarThemeMac.h"
 #endif
 
 #if PLATFORM(IOS_FAMILY)
@@ -3984,18 +3982,12 @@ JSC::JSValue Internals::evaluateInWorldIgnoringException(const String& name, con
     return scriptController.executeScriptInWorldIgnoringException(world, source);
 }
 
+#if !PLATFORM(MAC)
 void Internals::setUsesOverlayScrollbars(bool enabled)
 {
     WebCore::DeprecatedGlobalSettings::setUsesOverlayScrollbars(enabled);
-#if PLATFORM(MAC)
-    ScrollerStyle::setUseOverlayScrollbars(enabled);
-    ScrollbarTheme& theme = ScrollbarTheme::theme();
-    if (theme.isMockTheme())
-        return;
-
-    static_cast<ScrollbarThemeMac&>(theme).preferencesChanged();
-#endif
 }
+#endif
 
 void Internals::forceReload(bool endToEnd)
 {
diff --git a/Source/WebCore/testing/Internals.mm b/Source/WebCore/testing/Internals.mm
@@ -28,6 +28,7 @@
 
 #import "AGXCompilerService.h"
 #import "DOMURL.h"
+#import "DeprecatedGlobalSettings.h"
 #import "DictionaryLookup.h"
 #import "Document.h"
 #import "EventHandler.h"
@@ -39,6 +40,13 @@
 #import "SimpleRange.h"
 #import "UTIUtilities.h"
 #import <AVFoundation/AVPlayer.h>
+
+#if PLATFORM(MAC)
+#import "NSScrollerImpDetails.h"
+#import "ScrollbarThemeMac.h"
+#import <pal/spi/mac/NSScrollerImpSPI.h>
+#endif
+
 #if PLATFORM(COCOA)
 #import <Metal/Metal.h>
 #endif
@@ -130,6 +138,22 @@ - (NSAttributedString *)_attributedStringForRange:(NSRange)range
     return RefPtr<Range> { createLiveRange(std::get<SimpleRange>(*range)) };
 }
 
+void Internals::setUsesOverlayScrollbars(bool enabled)
+{
+    WebCore::DeprecatedGlobalSettings::setUsesOverlayScrollbars(enabled);
+
+    ScrollerStyle::setUseOverlayScrollbars(enabled);
+
+    ScrollbarTheme& theme = ScrollbarTheme::theme();
+    if (theme.isMockTheme())
+        return;
+
+    static_cast<ScrollbarThemeMac&>(theme).preferencesChanged();
+
+    NSScrollerStyle style = enabled ? NSScrollerStyleOverlay : NSScrollerStyleLegacy;
+    [NSScrollerImpPair _updateAllScrollerImpPairsForNewRecommendedScrollerStyle:style];
+}
+
 #endif
 
 #if ENABLE(VIDEO)
