Skip to content

Commit d9abfdf

Browse files
AtkinsSJAtkinsSJ
committed
LibWeb/CSS: Allow non-StyleValueLists in animation properties
Reverts 51f694c and b52beb5. The animation-* properties are in an awkward place currently, where they *should* be lists, and a lot of our code acts as if they are, but actually we parse them as single values. The above commits caused a few WPT tests to crash - see link below. I've imported one of them to prevent future regressions. https://wpt.fyi/results/css/css-typed-om/the-stylepropertymap/properties?diff&filter=ADC&run_id=6293362870321152&run_id=5123272984494080
1 parent 4d27e9a commit d9abfdf

File tree

4 files changed

+83
-6
lines changed

4 files changed

+83
-6
lines changed

‎Libraries/LibWeb/CSS/ComputedProperties.cpp‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -378,11 +378,20 @@ HashMap<PropertyID, StyleValueVector> ComputedProperties::assemble_coordinated_v
378378
// - If a coordinating list property has too few values specified, its value list is repeated to add more used
379379
// values.
380380
// - The computed values of the coordinating list properties are not affected by such truncation or repetition.
381+
382+
// FIXME: This is required because our animation-* properties are not yet parsed as lists.
383+
// Once that is fixed, every value here will be a StyleValueList.
384+
auto const get_property_value_as_list = [&](PropertyID property_id) {
385+
auto const& value = property(property_id);
386+
387+
return value.is_value_list() ? value.as_value_list().values() : StyleValueVector { value };
388+
};
389+
381390
HashMap<PropertyID, StyleValueVector> coordinated_value_list;
382391

383-
for (size_t i = 0; i < property(base_property_id).as_value_list().size(); i++) {
392+
for (size_t i = 0; i < get_property_value_as_list(base_property_id).size(); i++) {
384393
for (auto property_id : property_ids) {
385-
auto const& list = property(property_id).as_value_list().values();
394+
auto const& list = get_property_value_as_list(property_id);
386395

387396
coordinated_value_list.ensure(property_id).append(list[i % list.size()]);
388397
}

‎Libraries/LibWeb/CSS/StyleComputer.cpp‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2807,12 +2807,18 @@ static CSSPixels snap_a_length_as_a_border_width(double device_pixels_per_css_pi
28072807

28082808
static NonnullRefPtr<StyleValue const> compute_style_value_list(NonnullRefPtr<StyleValue const> const& style_value, Function<NonnullRefPtr<StyleValue const>(NonnullRefPtr<StyleValue const> const&)> const& compute_entry)
28092809
{
2810-
StyleValueVector computed_entries;
2810+
// FIXME: This is required because our animation-* properties are not yet parsed as lists.
2811+
// Once that is fixed, every value here will be a StyleValueList.
2812+
if (style_value->is_value_list()) {
2813+
StyleValueVector computed_entries;
28112814

2812-
for (auto const& entry : style_value->as_value_list().values())
2813-
computed_entries.append(compute_entry(entry));
2815+
for (auto const& entry : style_value->as_value_list().values())
2816+
computed_entries.append(compute_entry(entry));
28142817

2815-
return StyleValueList::create(move(computed_entries), StyleValueList::Separator::Comma);
2818+
return StyleValueList::create(move(computed_entries), StyleValueList::Separator::Comma);
2819+
}
2820+
2821+
return compute_entry(style_value);
28162822
}
28172823

28182824
NonnullRefPtr<StyleValue const> StyleComputer::compute_value_of_property(

‎Tests/LibWeb/Text/expected/wpt-import/css/css-typed-om/the-stylepropertymap/properties/animation-name.txt‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
Harness status: OK
2+
3+
Found 33 tests
4+
5+
32 Pass
6+
1 Fail
7+
Pass Can set 'animation-name' to CSS-wide keywords: initial
8+
Pass Can set 'animation-name' to CSS-wide keywords: inherit
9+
Pass Can set 'animation-name' to CSS-wide keywords: unset
10+
Pass Can set 'animation-name' to CSS-wide keywords: revert
11+
Pass Can set 'animation-name' to var() references: var(--A)
12+
Pass Can set 'animation-name' to the 'none' keyword: none
13+
Fail Can set 'animation-name' to the 'custom-ident' keyword: custom-ident
14+
Pass Setting 'animation-name' to a length: 0px throws TypeError
15+
Pass Setting 'animation-name' to a length: -3.14em throws TypeError
16+
Pass Setting 'animation-name' to a length: 3.14cm throws TypeError
17+
Pass Setting 'animation-name' to a length: calc(0px + 0em) throws TypeError
18+
Pass Setting 'animation-name' to a percent: 0% throws TypeError
19+
Pass Setting 'animation-name' to a percent: -3.14% throws TypeError
20+
Pass Setting 'animation-name' to a percent: 3.14% throws TypeError
21+
Pass Setting 'animation-name' to a percent: calc(0% + 0%) throws TypeError
22+
Pass Setting 'animation-name' to a time: 0s throws TypeError
23+
Pass Setting 'animation-name' to a time: -3.14ms throws TypeError
24+
Pass Setting 'animation-name' to a time: 3.14s throws TypeError
25+
Pass Setting 'animation-name' to a time: calc(0s + 0ms) throws TypeError
26+
Pass Setting 'animation-name' to an angle: 0deg throws TypeError
27+
Pass Setting 'animation-name' to an angle: 3.14rad throws TypeError
28+
Pass Setting 'animation-name' to an angle: -3.14deg throws TypeError
29+
Pass Setting 'animation-name' to an angle: calc(0rad + 0deg) throws TypeError
30+
Pass Setting 'animation-name' to a flexible length: 0fr throws TypeError
31+
Pass Setting 'animation-name' to a flexible length: 1fr throws TypeError
32+
Pass Setting 'animation-name' to a flexible length: -3.14fr throws TypeError
33+
Pass Setting 'animation-name' to a number: 0 throws TypeError
34+
Pass Setting 'animation-name' to a number: -3.14 throws TypeError
35+
Pass Setting 'animation-name' to a number: 3.14 throws TypeError
36+
Pass Setting 'animation-name' to a number: calc(2 + 3) throws TypeError
37+
Pass Setting 'animation-name' to a transform: translate(50%, 50%) throws TypeError
38+
Pass Setting 'animation-name' to a transform: perspective(10em) throws TypeError
39+
Pass Setting 'animation-name' to a transform: translate3d(0px, 1px, 2px) translate(0px, 1px) rotate3d(1, 2, 3, 45deg) rotate(45deg) scale3d(1, 2, 3) scale(1, 2) skew(1deg, 1deg) skewX(1deg) skewY(45deg) perspective(1px) matrix3d(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16) matrix(1, 2, 3, 4, 5, 6) throws TypeError

‎Tests/LibWeb/Text/input/wpt-import/css/css-typed-om/the-stylepropertymap/properties/animation-name.html‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
<!doctype html>
2+
<meta charset="utf-8">
3+
<title>'animation-name' property</title>
4+
<link rel="help" href="https://drafts.css-houdini.org/css-typed-om-1/#dom-stylepropertymap-get">
5+
<link rel="help" href="https://drafts.css-houdini.org/css-typed-om-1/#dom-stylepropertymap-set">
6+
<link rel="help" href="https://drafts.css-houdini.org/css-typed-om-1/#property-stle-value-normalization">
7+
<script src="../../../../resources/testharness.js"></script>
8+
<script src="../../../../resources/testharnessreport.js"></script>
9+
<script src="../../resources/testhelper.js"></script>
10+
<script src="resources/testsuite.js"></script>
11+
<body>
12+
<div id="log"></div>
13+
<script>
14+
'use strict';
15+
16+
runListValuedPropertyTests('animation-name', [
17+
{ syntax: 'none' },
18+
// FIXME: This should be <custom-ident>, but the test harness doesn't
19+
// currently support it.
20+
{ syntax: 'custom-ident' },
21+
]);
22+
23+
</script>

0 commit comments

Comments
 (0)