8000 Fix incorrect custom pseudo-scrollbar sizes caused by scroller style … · WebKit/WebKit@560abdc · GitHub
[go: up one dir, main page]

Skip to content

Commit 560abdc

Browse files
GetToSetnt1m
authored andcommitted
Fix incorrect custom pseudo-scrollbar sizes caused by scroller style changes on Mac.
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
1 parent 34f0ba4 commit 560abdc

File tree

7 files changed

+104
-12
lines changed

7 files changed

+104
-12
lines changed

LayoutTests/TestExpectations

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,9 @@ fast/visual-viewport/rubberbanding-viewport-rects-header-footer.html [ Skip ]
291291
# DataDetectors tests only make sense on Mac
292292
fast/events/do-not-drag-and-drop-data-detectors-link.html [ Skip ]
293293

294+
# Testing against scroller style changes only makes sense on Mac
295+
scrollbars/custom-scrollbar-scroller-style-change.html [ Skip ]
296+
294297
# A rounding error may cause 90deg 3d-rotated elements to count as painted
295298
imported/w3c/web-platform-tests/paint-timing/fcp-only/fcp-invisible-3d-rotate-descendant.html
296299

LayoutTests/platform/mac/TestExpectations

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,8 @@ fast/harness/uiscriptcontroller [ Pass ]
7777

7878
fast/events/do-not-drag-and-drop-data-detectors-link.html [ Pass ]
7979

80+
scrollbars/custom-scrollbar-scroller-style-change.html [ Pass ]
81+
8082
http/tests/gzip-content-encoding [ Pass ]
8183

8284
http/tests/xmlhttprequest/gzip-content-type-no-content-encoding.html [ Pass ]
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
layer at (0,0) size 800x600
2+
RenderView at (0,0) size 800x600
3+
layer at (0,0) size 800x216
4+
RenderBlock {HTML} at (0,0) size 800x216
5+
RenderBody {BODY} at (8,8) size 784x200
6+
layer at (8,8) size 200x200 clip at (8,8) size 152x152 scrollWidth 400 scrollHeight 400
7+
RenderBlock {DIV} at (0,0) size 200x200
8+
layer at (8,8) size 400x400 backgroundClip at (8,8) size 152x152 clip at (8,8) size 152x152
9+
RenderBlock {DIV} at (0,0) size 400x400 [bgcolor=#008000]
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
<!DOCTYPE html> <!-- webkit-test-runner [ MockScrollbarsEnabled=false ] -->
2+
<html>
3+
<head>
4+
<title>Scroller style changes should not change the size of custom scrollbars.</title>
5+
<style>
6+
::-webkit-scrollbar {
7+
width: 48px;
8+
height: 48px;
9+
}
10+
11+
::-webkit-scrollbar-track {
12+
background-color: #E3E3E3;
13+
}
14+
15+
::-webkit-scrollbar-thumb {
16+
background: black;
17+
}
18+
19+
.scroll-container {
20+
overflow: scroll;
21+
width: 200px;
22+
height: 200px;
23+
}
24+
25+
.overflowing {
26+
width: 400px;
27+
height: 400px;
28+
background: green;
29+
}
30+
31+
.composited {
32+
transform: translateZ(0);
33+
}
34+
</style>
35+
<script>
36+
if (window.testRunner) {
37+
testRunner.waitUntilDone();
38+
}
39+
40+
function doTest() {
41+
if (window.internals)
42+
internals.setUsesOverlayScrollbars(true);
43+
if (window.testRunner)
44+
testRunner.notifyDone();
45+
}
46+
47+
window.addEventListener('load', () => {
48+
setTimeout(() => {
49+
doTest()
50+
}, 0);
51+
}, false);
52+
</script>
53+
</head>
54+
<body>
55+
<!-- The scrollbars below should always sized 48x48 regardless of any scroller style changes. -->
56+
<div class="scroll-container">
57+
<div class="overflowing composited"></div>
58+
</div>
59+
</body>
60+
</html>

Source/WebCore/platform/mac/ScrollbarsControllerMac.mm

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -954,7 +954,8 @@ - (void)invalidate
954954

955955
NSScrollerStyle newStyle = [m_scrollerImpPair scrollerStyle];
956956

957-
if (Scrollbar* verticalScrollbar = scrollableArea().verticalScrollbar()) {
957+
Scrollbar* verticalScrollbar = scrollableArea().verticalScrollbar();
958+
if (verticalScrollbar && !verticalScrollbar->isCustomScrollbar()) {
958959
verticalScrollbar->invalidate();
959960

960961
NSScrollerImp *oldVerticalPainter = [m_scrollerImpPair verticalScrollerImp];
@@ -971,7 +972,8 @@ - (void)invalidate
971972
verticalScrollbar->setFrameRect(IntRect(0, 0, thickness, thickness));
972973
}
973974

974-
if (Scrollbar* horizontalScrollbar = scrollableArea().horizontalScrollbar()) {
975+
Scrollbar* horizontalScrollbar = scrollableArea().horizontalScrollbar();
976+
if (horizontalScrollbar && !horizontalScrollbar->isCustomScrollbar()) {
975977
horizontalScrollbar->invalidate();
976978

977979
NSScrollerImp *oldHorizontalPainter = [m_scrollerImpPair horizontalScrollerImp];

Source/WebCore/testing/Internals.cpp

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -352,8 +352,6 @@
352352

353353
#if PLATFORM(MAC)
354354
#include "GraphicsChecksMac.h"
355-
#include "NSScrollerImpDetails.h"
356-
#include "ScrollbarThemeMac.h"
357355
#endif
358356

359357
#if PLATFORM(IOS_FAMILY)
@@ -3984,18 +3982,12 @@ JSC::JSValue Internals::evaluateInWorldIgnoringException(const String& name, con
39843982
return scriptController.executeScriptInWorldIgnoringException(world, source);
39853983
}
39863984

3985+
#if !PLATFORM(MAC)
39873986
void Internals::setUsesOverlayScrollbars(bool enabled)
39883987
{
39893988
WebCore::DeprecatedGlobalSettings::setUsesOverlayScrollbars(enabled);
3990-
#if PLATFORM(MAC)
3991-
ScrollerStyle::setUseOverlayScrollbars(enabled);
3992-
ScrollbarTheme& theme = ScrollbarTheme::theme();
3993-
if (theme.isMockTheme())
3994-
return;
3995-
3996-
static_cast<ScrollbarThemeMac&>(theme).preferencesChanged();
3997-
#endif
39983989
}
3990+
#endif
39993991

40003992
void Internals::forceReload(bool endToEnd)
40013993
{

Source/WebCore/testing/Internals.mm

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828

2929
#import "AGXCompilerService.h"
3030
#import "DOMURL.h"
31+
#import "DeprecatedGlobalSettings.h"
3132
#import "DictionaryLookup.h"
3233
#import "Document.h"
3334
#import "EventHandler.h"
@@ -39,6 +40,13 @@
3940
#import "SimpleRange.h"
4041
#import "UTIUtilities.h"
4142
#import <AVFoundation/AVPlayer.h>
43+
44+
#if PLATFORM(MAC)
45+
#import "NSScrollerImpDetails.h"
46+
#import "ScrollbarThemeMac.h"
47+
#import <pal/spi/mac/NSScrollerImpSPI.h>
48+
#endif
49+
4250
#if PLATFORM(COCOA)
4351
#import <Metal/Metal.h>
4452
#endif
@@ -130,6 +138,22 @@ - (NSAttributedString *)_attributedStringForRange:(NSRange)range
130138
return RefPtr<Range> { createLiveRange(std::get<SimpleRange>(*range)) };
131139
}
132140

141+
void Internals::setUsesOverlayScrollbars(bool enabled)
142+
{
143+
WebCore::DeprecatedGlobalSettings::setUsesOverlayScrollbars(enabled);
144+
145+
ScrollerStyle::setUseOverlayScrollbars(enabled);
146+
147+
ScrollbarTheme& theme = ScrollbarTheme::theme();
148+
if (theme.isMockTheme())
149+
return;
150+
151+
static_cast<ScrollbarThemeMac&>(theme).preferencesChanged();
152+
153+
NSScrollerStyle style = enabled ? NSScrollerStyleOverlay : NSScrollerStyleLegacy;
154+
[NSScrollerImpPair _updateAllScrollerImpPairsForNewRecommendedScrollerStyle:style];
155+
}
156+
133157
#endif
134158

135159
#if ENABLE(VIDEO)

0 commit comments

Comments
 (0)
0