Skip to content

Commit 76dadd4

Browse files
Calme1709AtkinsSJ
authored andcommitted
LibWeb: Favour !important property values over animated values
1 parent c8f3453 commit 76dadd4

File tree

3 files changed

+159
-1
lines changed

3 files changed

+159
-1
lines changed

Libraries/LibWeb/CSS/ComputedProperties.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,8 @@ StyleValue const& ComputedProperties::property(PropertyID property_id, WithAnima
169169
{
170170
VERIFY(property_id >= first_longhand_property_id && property_id <= last_longhand_property_id);
171171

172-
if (return_animated_value == WithAnimationsApplied::Yes) {
172+
// Important properties override animated properties
173+
if (!is_property_important(property_id) && return_animated_value == WithAnimationsApplied::Yes) {
173174
if (auto animated_value = m_animated_property_values.get(property_id); animated_value.has_value())
174175
return *animated_value.value();
175176
}
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
Harness status: OK
2+
3+
Found 7 tests
4+
5+
5 Pass
6+
2 Fail
7+
Pass Interpolated value is observable
8+
Pass Important rules override animations
9+
Pass Non-overriden interpolations are observable
10+
Pass Important rules override animations (::before)
11+
Pass Important rules do not override animations on :visited as seen from JS
12+
Fail Standard property animations appearing via setKeyframes do not override important declarations
13+
Fail Custom property animations appearing via setKeyframes do not override important declarations
Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
<!DOCTYPE html>
2+
<title>Test !important declarations vs. animation effects</title>
3+
<link rel="help" href="https://drafts.csswg.org/css-cascade/#cascade-origin">
4+
<script src="../../resources/testharness.js"></script>
5+
<script src="../../resources/testharnessreport.js"></script>
6+
<script>
7+
CSS.registerProperty({
8+
name: "--length-1",
9+
syntax: "<length>",
10+
initialValue: "0px",
11+
inherits: false
12+
});
13+
CSS.registerProperty({
14+
name: "--length-2",
15+
syntax: "<length>",
16+
initialValue: "0px",
17+
inherits: false
18+
});
19+
</script>
20+
<style>
21+
@keyframes bgcolor_animation {
22+
from { background-color: rgb(10, 10, 10); }
23+
to { background-color: rgb(20, 20, 20); }
24+
}
25+
@keyframes color_and_bg_animation {
26+
from { background-color: rgb(10, 10, 10); color: rgb(10, 10, 10); }
27+
to { background-color: rgb(20, 20, 20); color: rgb(20, 20, 20); }
28+
}
29+
a, div, ::before{
30+
animation-duration: 1000s;
31+
animation-delay: -500s;
32+
animation-timing-function: steps(2, end);
33+
}
34+
#target1 {
35+
animation-name: bgcolor_animation;
36+
}
37+
#target2 {
38+
background-color: rgb(0, 128, 0) !important;
39+
animation-name: bgcolor_animation;
40+
}
41+
#target3 {
42+
color: rgb(0, 128, 0) !important;
43+
animation-name: color_and_bg_animation;
44+
}
45+
#target4::before {
46+
color: rgb(0, 128, 0) !important;
47+
animation-name: color_and_bg_animation;
48+
content: " ";
49+
}
50+
#target5 {
51+
animation-name: color_and_bg_animation;
52+
}
53+
#target5:visited {
54+
color: rgb(0, 128, 0) !important;
55+
}
56+
#target6 {
57+
background-color: white;
58+
color: white !important;
59+
}
60+
#target7 {
61+
--length-1: 10px;
62+
--length-2: 10px !important;
63+
}
64+
</style>
65+
<div id="target1"></div>
66+
<div id="target2"></div>
67+
<div id="target3"></div>
68+
<div id="target4"></div>
69+
<a href="" id="target5"></a>
70+
<span id="target6"></span>
71+
<span id="target7"></span>
72+
<script>
73+
test(() => {
74+
assert_equals(getComputedStyle(target1).backgroundColor, 'rgb(15, 15, 15)');
75+
}, 'Interpolated value is observable');
76+
77+
test(() => {
78+
assert_equals(getComputedStyle(target2).backgroundColor, 'rgb(0, 128, 0)');
79+
}, 'Important rules override animations');
80+
81+
test(() => {
82+
assert_equals(getComputedStyle(target3).color, 'rgb(0, 128, 0)');
83+
assert_equals(getComputedStyle(target3).backgroundColor, 'rgb(15, 15, 15)');
84+
}, 'Non-overriden interpolations are observable');
85+
86+
test(() => {
87+
assert_equals(getComputedStyle(target4, '::before').color, 'rgb(0, 128, 0)');
88+
assert_equals(getComputedStyle(target4, '::before').backgroundColor, 'rgb(15, 15, 15)');
89+
}, 'Important rules override animations (::before)');
90+
91+
test(() => {
92+
assert_equals(getComputedStyle(target5).color, 'rgb(15, 15, 15)');
93+
assert_equals(getComputedStyle(target5).backgroundColor, 'rgb(15, 15, 15)');
94+
}, 'Important rules do not override animations on :visited as seen from JS');
95+
96+
test(() => {
97+
getComputedStyle(target6).backgroundColor;
98+
99+
let animation = target6.animate([
100+
{ backgroundColor: 'rgb(10, 10, 10)' },
101+
{ backgroundColor: 'rgb(20, 20, 20)' },
102+
], {
103+
duration: 1000000,
104+
delay: -500000,
105+
easing: 'steps(2, end)'
106+
});
107+
108+
assert_equals(getComputedStyle(target6).backgroundColor, 'rgb(15, 15, 15)');
109+
assert_equals(getComputedStyle(target6).color, 'rgb(255, 255, 255)');
110+
111+
animation.effect.setKeyframes([
112+
{ color: 'rgb(10, 10, 10)' },
113+
{ color: 'rgb(20, 20, 20)' },
114+
]);
115+
116+
assert_equals(getComputedStyle(target6).backgroundColor, 'rgb(255, 255, 255)');
117+
assert_equals(getComputedStyle(target6).color, 'rgb(255, 255, 255)');
118+
}, 'Standard property animations appearing via setKeyframes do not override important declarations');
119+
120+
test(() => {
121+
getComputedStyle(target7).getPropertyValue('--length-1');
122+
123+
let animation = target7.animate([
124+
{ '--length-1': '100px' },
125+
{ '--length-1': '200px' },
126+
], {
127+
duration: 1000000,
128+
delay: -500000,
129+
easing: 'steps(2, end)'
130+
});
131+
132+
assert_equals(getComputedStyle(target7).getPropertyValue('--length-1'), '150px');
133+
assert_equals(getComputedStyle(target7).getPropertyValue('--length-2'), '10px');
134+
135+
animation.effect.setKeyframes([
136+
{ '--length-2': '100px' },
137+
{ '--length-2': '200px' },
138+
]);
139+
140+
assert_equals(getComputedStyle(target7).getPropertyValue('--length-1'), '10px');
141+
assert_equals(getComputedStyle(target7).getPropertyValue('--length-2'), '10px');
142+
}, 'Custom property animations appearing via setKeyframes do not override important declarations');
143+
144+
</script>

0 commit comments

Comments
 (0)