Skip to content

Commit

Permalink
Fix issues with scrollbar-color invalidation with currentColor
Browse files Browse the repository at this point in the history
scrollbar-color property that uses currentColor now correctly repaints
when color value updates.

This only fixes non-viewports as viewports have a separate issue where
 scrollbar-color invalidation doesn't work at all.

Bug: 891944
Change-Id: Iac513cbadbfc589280621b864d1d0b5d68bf9e98
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4774160
Reviewed-by: Thomas Lukaszewicz <tluk@chromium.org>
Commit-Queue: Luke <lukewarlow156@gmail.com>
Cr-Commit-Position: refs/heads/main@{#1184751}
  • Loading branch information
lukewarlow authored and Chromium LUCI CQ committed Aug 17, 2023
1 parent b11cd15 commit d969c04
Show file tree
Hide file tree
Showing 20 changed files with 498 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1404,8 +1404,10 @@ void PaintLayerScrollableArea::UpdateAfterStyleChange(
old_style->UsedColorScheme() != UsedColorSchemeScrollbars() ||
old_style->ScrollbarWidth() !=
GetLayoutBox()->StyleRef().ScrollbarWidth() ||
old_style->ScrollbarColor() !=
GetLayoutBox()->StyleRef().ScrollbarColor()) {
old_style->ScrollbarThumbColorResolved() !=
GetLayoutBox()->StyleRef().ScrollbarThumbColorResolved() ||
old_style->ScrollbarTrackColorResolved() !=
GetLayoutBox()->StyleRef().ScrollbarTrackColorResolved()) {
SetScrollControlsNeedFullPaintInvalidation();
}
}
Expand Down
3 changes: 3 additions & 0 deletions third_party/blink/web_tests/TestExpectations
Original file line number Diff line number Diff line change
Expand Up @@ -1561,6 +1561,9 @@ crbug.com/1158554 external/wpt/css/css-conditional/css-supports-042.xht [ Failur
# CSS Scrollbars
crbug.com/891944 external/wpt/css/css-scrollbars/scrollbar-width-paint-004.html [ Failure ]
crbug.com/891944 external/wpt/css/css-scrollbars/scrollbar-width-paint-005.html [ Failure ]
crbug.com/891944 external/wpt/css/css-scrollbars/scrollbar-color-dynamic-1.html [ Failure ]
crbug.com/891944 external/wpt/css/css-scrollbars/scrollbar-color-dynamic-2.html [ Failure ]
crbug.com/891944 external/wpt/css/css-scrollbars/scrollbar-color-dynamic-5.html [ Failure ]

# Failures from CSS3 Text (text-justify and text-indent: hanging/each-line) not being implemented.
crbug.com/248894 external/wpt/css/css-pseudo/first-letter-allowed-properties.html [ Failure ]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<!doctype html>
<html>
<style>
:root {
scrollbar-color: blue blue;
color: blue;
}

body {
overflow: scroll;
}
</style>
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<!doctype html>
<html>
<title>CSS Scrollbars: scrollbar-color with current color works correctly</title>
<link rel="author" title="Luke Warlow" href="mailto:luke@warlow.dev" />
<link rel="match" href="scrollbar-color-011-ref.html" />
<link rel="help" href="https://drafts.csswg.org/css-scrollbars-1/" />
<style>
:root {
scrollbar-color: currentColor currentColor;
color: blue;
}

body {
overflow: scroll;
}
</style>
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
<!doctype html>
<style>
:root {
scrollbar-color: blue green;
}
body {
display: flex;
flex-wrap: wrap;
width: 200vw;
height: 200vh;
}

.container {
scrollbar-gutter: stable;
flex: 0 0;
overflow: auto;
height: 200px;
min-width: 200px;
margin: 1px;
padding: 0px;
border: none;
background: deepskyblue;
}

.content {
height: 300px;
width: 300px;
background: red;
}
</style>

<div class="container">
<div class="content"></div>
</div>
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
<!doctype html>
<html class="reftest-wait">
<title>Dynamically set scrollbar-colors and ensure scrollbars update</title>
<link rel="author" title="Luke Warlow" href="mailto:luke@warlow.dev" />
<link rel="help" href="https://drafts.csswg.org/css-scrollbars" />
<link rel="match" href="scrollbar-color-dynamic-1-ref.html" />
<script src="/common/reftest-wait.js"></script>
<style>
:root {
scrollbar-color: red yellow;
}
body {
display: flex;
flex-wrap: wrap;
width: 200vw;
height: 200vh;
}

.container {
scrollbar-gutter: stable;
overflow: auto;
flex: 0 0;
height: 200px;
min-width: 200px;
margin: 1px;
padding: 0px;
border: none;
background: deepskyblue;
}

.content {
height: 300px;
width: 300px;
background: red;
}
</style>
<div class="container">
<div class="content"></div>
</div>
<script>
requestAnimationFrame(() => requestAnimationFrame(() => {
document.documentElement.style.scrollbarColor = 'blue green';

takeScreenshot();
}));
</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[scrollbar-color-dynamic-1.html]
expected:
FAIL
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<!doctype html>
<style>
:root {
scrollbar-color: blue green;
}
body {
display: flex;
flex-wrap: wrap;
overflow: scroll;
}
.container {
scrollbar-gutter: stable;
overflow: scroll;
flex: 0 0;
height: 200px;
min-width: 200px;
margin: 1px;
padding: 0px;
border: none;
background: deepskyblue;
}
</style>
<div class="container"></div>
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
<!doctype html>
<html class="reftest-wait">
<title>Dynamically set scrollbar-color and ensure overflow scroll scrollbars update</title>
<link rel="author" title="Luke Warlow" href="mailto:luke@warlow.dev" />
<link rel="help" href="https://drafts.csswg.org/css-scrollbars/#valdef-scrollbar-color-auto" />
<link rel="match" href="scrollbar-color-dynamic-2-ref.html" />
<script src="/common/reftest-wait.js"></script>
<style>
:root {
scrollbar-color: red yellow;
}
body {
display: flex;
flex-wrap: wrap;
overflow: scroll;
}
.container {
scrollbar-gutter: stable;
overflow: scroll;
flex: 0 0;
height: 200px;
min-width: 200px;
margin: 1px;
padding: 0px;
border: none;
background: deepskyblue;
}
</style>
<div class="container"></div>
<script>
requestAnimationFrame(() => requestAnimationFrame(() => {
document.documentElement.style.scrollbarColor = 'blue green';

takeScreenshot();
}));
</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[scrollbar-color-dynamic-2.html]
expected:
FAIL
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
<!doctype html>
<style>
:root {
scrollbar-color: red yellow;
}
body {
display: flex;
flex-wrap: wrap;
width: 200vw;
height: 200vh;
}

.container {
scrollbar-color: blue green;
scrollbar-gutter: stable;
flex: 0 0;
overflow: auto;
height: 200px;
min-width: 200px;
margin: 1px;
padding: 0px;
border: none;
background: deepskyblue;
}

.content {
height: 300px;
width: 300px;
background: red;
}
</style>

<div class="container">
<div class="content"></div>
</div>
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
<!doctype html>
<html class="reftest-wait">
<title>Dynamically set scrollbar-color on container and ensure scrollbars update</title>
<link rel="author" title="Luke Warlow" href="mailto:luke@warlow.dev" />
<link rel="help" href="https://drafts.csswg.org/css-scrollbars/#valdef-scrollbar-color-auto" />
<link rel="match" href="scrollbar-color-dynamic-3-ref.html" />
<script src="/common/reftest-wait.js"></script>
<style>
:root {
scrollbar-color: red yellow;
}
body {
display: flex;
flex-wrap: wrap;
width: 200vw;
height: 200vh;
}

.container {
scrollbar-gutter: stable;
overflow: auto;
flex: 0 0;
height: 200px;
min-width: 200px;
margin: 1px;
padding: 0px;
border: none;
background: deepskyblue;
}

.content {
height: 300px;
width: 300px;
background: red;
}
</style>
<div class="container">
<div class="content"></div>
</div>
<script>
requestAnimationFrame(() => requestAnimationFrame(() => {
document.querySelector(".container").style.scrollbarColor = 'blue green';

takeScreenshot();
}));
</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<!doctype html>
<style>
:root {
scrollbar-color: red yellow;
}
body {
display: flex;
flex-wrap: wrap;
overflow: scroll;
}
.container {
scrollbar-color: blue green;
scrollbar-gutter: stable;
overflow: scroll;
flex: 0 0;
height: 200px;
min-width: 200px;
margin: 1px;
padding: 0px;
border: none;
background: deepskyblue;
}
</style>
<div class="container"></div>
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
<!doctype html>
<html class="reftest-wait">
<title>Dynamically set scrollbar-color on container with overflow scroll and ensure scrollbars update</title>
<link rel="author" title="Luke Warlow" href="mailto:luke@warlow.dev" />
<link rel="help" href="https://drafts.csswg.org/css-scrollbars/#valdef-scrollbar-color-auto" />
<link rel="match" href="scrollbar-color-dynamic-4-ref.html" />
<script src="/common/reftest-wait.js"></script>
<style>
:root {
scrollbar-color: red yellow;
}
body {
display: flex;
flex-wrap: wrap;
overflow: scroll;
}
.container {
scrollbar-gutter: stable;
overflow: scroll;
flex: 0 0;
height: 200px;
min-width: 200px;
margin: 1px;
padding: 0px;
border: none;
background: deepskyblue;
}
</style>
<div class="container"></div>
<script>
requestAnimationFrame(() => requestAnimationFrame(() => {
document.querySelector(".container").style.scrollbarColor = 'blue green';

takeScreenshot();
}));
</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
<!doctype html>
<style>
:root {
scrollbar-color: green green;
}
body {
display: flex;
flex-wrap: wrap;
width: 200vw;
height: 200vh;
}

.container {
scrollbar-gutter: stable;
flex: 0 0;
overflow: auto;
height: 200px;
min-width: 200px;
margin: 1px;
padding: 0px;
border: none;
background: deepskyblue;
}

.content {
height: 300px;
width: 300px;
background: red;
}
</style>

<div class="container">
<div class="content"></div>
</div>

0 comments on commit d969c04

Please sign in to comment.