Skip to content

Commit 50fde0f

Browse files
Calme1709Calme1709
committed
LibWeb: Parse the scroll-timeline shorthand CSS property
The remaining failing tests in scroll-timeline-shorthand.html are due to either: a) incorrect tests, see web-platform-tests/wpt#56181 or; b) a wider issue where we collapse coordinating value list longhand properties to a single value when we shouldn't.
1 parent 5374917 commit 50fde0f

File tree

8 files changed

+261
-3
lines changed

8 files changed

+261
-3
lines changed

‎Libraries/LibWeb/CSS/Parser/Parser.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -499,6 +499,7 @@ class Parser {
499499
RefPtr<StyleValue const> parse_position_visibility_value(TokenStream<ComponentValue>&);
500500
RefPtr<StyleValue const> parse_quotes_value(TokenStream<ComponentValue>&);
501501
RefPtr<StyleValue const> parse_single_repeat_style_value(PropertyID, TokenStream<ComponentValue>&);
502+
RefPtr<StyleValue const> parse_scroll_timeline_value(TokenStream<ComponentValue>&);
502503
RefPtr<StyleValue const> parse_scrollbar_color_value(TokenStream<ComponentValue>&);
503504
RefPtr<StyleValue const> parse_scrollbar_gutter_value(TokenStream<ComponentValue>&);
504505
RefPtr<StyleValue const> parse_shadow_value(TokenStream<ComponentValue>&, ShadowStyleValue::ShadowType);

‎Libraries/LibWeb/CSS/Parser/PropertyParsing.cpp‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -745,6 +745,8 @@ Parser::ParseErrorOr<NonnullRefPtr<StyleValue const>> Parser::parse_css_value(Pr
745745
return parse_all_as(tokens, [this](auto& tokens) { return parse_translate_value(tokens); });
746746
case PropertyID::Scale:
747747
return parse_all_as(tokens, [this](auto& tokens) { return parse_scale_value(tokens); });
748+
case PropertyID::ScrollTimeline:
749+
return parse_all_as(tokens, [this](auto& tokens) { return parse_scroll_timeline_value(tokens); });
748750
case PropertyID::ScrollTimelineAxis:
749751
case PropertyID::ScrollTimelineName:
750752
return parse_all_as(tokens, [this, property_id](auto& tokens) { return parse_simple_comma_separated_value_list(property_id, tokens); });
@@ -5182,6 +5184,73 @@ RefPtr<StyleValue const> Parser::parse_scale_value(TokenStream<ComponentValue>&
51825184
return TransformationStyleValue::create(PropertyID::Scale, TransformFunction::Scale3d, { maybe_x.release_nonnull(), maybe_y.release_nonnull(), maybe_z.release_nonnull() });
51835185
}
51845186

5187+
// https://drafts.csswg.org/scroll-animations-1/#scroll-timeline-shorthand
5188+
RefPtr<StyleValue const> Parser::parse_scroll_timeline_value(TokenStream<ComponentValue>& tokens)
5189+
{
5190+
// [ <'scroll-timeline-name'> <'scroll-timeline-axis'>? ]#
5191+
StyleValueVector names;
5192+
StyleValueVector axes;
5193+
5194+
auto transaction = tokens.begin_transaction();
5195+
5196+
do {
5197+
tokens.discard_whitespace();
5198+
5199+
auto name_value = parse_css_value_for_property(PropertyID::ScrollTimelineName, tokens);
5200+
5201+
if (!name_value)
5202+
return nullptr;
5203+
5204+
names.append(name_value.release_nonnull());
5205+
5206+
tokens.discard_whitespace();
5207+
5208+
if (tokens.next_token().is(Token::Type::Comma)) {
5209+
axes.append(KeywordStyleValue::create(Keyword::Block));
5210+
tokens.discard_a_token();
5211+
5212+
// Disallow trailing commas
5213+
if (!tokens.has_next_token())
5214+
return nullptr;
5215+
5216+
continue;
5217+
}
5218+
5219+
if (!tokens.has_next_token()) {
5220+
axes.append(KeywordStyleValue::create(Keyword::Block));
5221+
break;
5222+
}
5223+
5224+
auto axis_value = parse_css_value_for_property(PropertyID::ScrollTimelineAxis, tokens);
5225+
5226+
if (!axis_value)
5227+
return nullptr;
5228+
5229+
axes.append(axis_value.release_nonnull());
5230+
5231+
tokens.discard_whitespace();
5232+
5233+
if (tokens.next_token().is(Token::Type::Comma)) {
5234+
tokens.discard_a_token();
5235+
5236+
// Disallow trailing commas
5237+
if (!tokens.has_next_token())
5238+
return nullptr;
5239+
5240+
continue;
5241+
}
5242+
5243+
if (tokens.has_next_token())
5244+
return nullptr;
5245+
} while (tokens.has_next_token());
5246+
5247+
transaction.commit();
5248+
5249+
return ShorthandStyleValue::create(PropertyID::ScrollTimeline,
5250+
{ PropertyID::ScrollTimelineName, PropertyID::ScrollTimelineAxis },
5251+
{ StyleValueList::create(move(names), StyleValueList::Separator::Comma), StyleValueList::create(move(axes), StyleValueList::Separator::Comma) });
5252+
}
5253+
51855254
// https://drafts.csswg.org/css-scrollbars/#propdef-scrollbar-color
51865255
RefPtr<StyleValue const> Parser::parse_scrollbar_color_value(TokenStream<ComponentValue>& tokens)
51875256
{

‎Libraries/LibWeb/CSS/Properties.json‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3440,6 +3440,16 @@
34403440
"affects-layout": false,
34413441
"affects-stacking-context": true
34423442
},
3443+
"scroll-timeline": {
3444+
"affects-layout": false,
3445+
"inherited": false,
3446+
"initial": "none block",
3447+
"multiplicity": "coordinating-list",
3448+
"longhands": [
3449+
"scroll-timeline-name",
3450+
"scroll-timeline-axis"
3451+
]
3452+
},
34433453
"scroll-timeline-axis": {
34443454
"affects-layout": false,
34453455
"animation-type": "none",

‎Libraries/LibWeb/CSS/StyleValues/ShorthandStyleValue.cpp‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,18 +78,23 @@ String ShorthandStyleValue::to_string(SerializationMode mode) const
7878
return { value.release_nonnull() };
7979
};
8080

81-
auto const coordinating_value_list_shorthand_to_string = [&](StringView entry_when_all_longhands_initial) {
81+
auto const coordinating_value_list_shorthand_to_string = [&](StringView entry_when_all_longhands_initial, Vector<PropertyID> required_longhands = {}) {
8282
auto entry_count = style_value_as_value_list(longhand(m_properties.sub_properties[0])).size();
8383

8484
// If we don't have the same number of values for each longhand, we can't serialize this shorthand.
8585
if (any_of(m_properties.sub_properties, [&](auto longhand_id) { return style_value_as_value_list(longhand(longhand_id)).size() != entry_count; }))
8686
return ""_string;
8787

8888
// We should serialize a longhand if:
89+
// - The longhand is required
8990
// - The value is not the initial value
9091
// - Another longhand value which will be included later in the serialization is valid for this longhand.
9192
auto should_serialize_longhand = [&](size_t entry_index, size_t longhand_index) {
9293
auto longhand_id = m_properties.sub_properties[longhand_index];
94+
95+
if (required_longhands.contains_slow(longhand_id))
96+
return true;
97+
9398
auto longhand_value = style_value_as_value_list(longhand(longhand_id))[entry_index];
9499

95100
if (!longhand_value->equals(style_value_as_value_list(property_initial_value(longhand_id))[0]))
@@ -815,6 +820,10 @@ String ShorthandStyleValue::to_string(SerializationMode mode) const
815820
case PropertyID::PlaceItems:
816821
case PropertyID::PlaceSelf:
817822
return positional_value_list_shorthand_to_string(m_properties.values);
823+
case PropertyID::ScrollTimeline:
824+
// NB: We don't need to specify a value to use when the entry is empty as all values are initial since
825+
// scroll-timeline-name is always included
826+
return coordinating_value_list_shorthand_to_string(""sv, { PropertyID::ScrollTimelineName });
818827
case PropertyID::TextDecoration: {
819828
// The rule here seems to be, only print what's different from the default value,
820829
// but if they're all default, print the line.

‎Tests/LibWeb/Text/expected/css/CSSStyleProperties-all-supported-properties-and-default-values.txt‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -694,6 +694,8 @@ All supported properties and their default values exposed from CSSStylePropertie
694694
'rx': 'auto'
695695
'ry': 'auto'
696696
'scale': 'none'
697+
'scrollTimeline': 'none'
698+
'scroll-timeline': 'none'
697699
'scrollTimelineAxis': 'block'
698700
'scroll-timeline-axis': 'block'
699701
'scrollTimelineName': 'none'

‎Tests/LibWeb/Text/expected/wpt-import/css/css-cascade/all-prop-revert-layer.txt‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
Harness status: OK
22

3-
Found 272 tests
3+
Found 274 tests
44

5-
266 Pass
5+
268 Pass
66
6 Fail
77
Pass accent-color
88
Pass border-collapse
@@ -241,6 +241,8 @@ Pass row-gap
241241
Pass rx
242242
Pass ry
243243
Pass scale
244+
Pass scroll-timeline-axis
245+
Pass scroll-timeline-name
244246
Pass scrollbar-color
245247
Pass scrollbar-gutter
246248
Pass scrollbar-width

‎Tests/LibWeb/Text/expected/wpt-import/scroll-animations/css/scroll-timeline-shorthand.txt‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
Harness status: OK
2+
3+
Found 50 tests
4+
5+
47 Pass
6+
3 Fail
7+
Pass e.style['scroll-timeline'] = "none block" should set the property value
8+
Pass e.style['scroll-timeline'] = "none inline" should set the property value
9+
Pass e.style['scroll-timeline'] = "--abc x" should set the property value
10+
Pass e.style['scroll-timeline'] = "--abc inline" should set the property value
11+
Pass e.style['scroll-timeline'] = "--aBc inline" should set the property value
12+
Pass e.style['scroll-timeline'] = "--inline inline" should set the property value
13+
Pass e.style['scroll-timeline'] = "--abc" should set the property value
14+
Pass e.style['scroll-timeline'] = "--inline block" should set the property value
15+
Pass e.style['scroll-timeline'] = "--block block" should set the property value
16+
Pass e.style['scroll-timeline'] = "--y block" should set the property value
17+
Pass e.style['scroll-timeline'] = "--x block" should set the property value
18+
Pass e.style['scroll-timeline'] = "--a, --b, --c" should set the property value
19+
Pass e.style['scroll-timeline'] = "--a inline, --b block, --c y" should set the property value
20+
Pass e.style['scroll-timeline'] = "--auto" should set the property value
21+
Pass e.style['scroll-timeline'] = "" should not set the property value
22+
Pass e.style['scroll-timeline'] = "--abc --abc" should not set the property value
23+
Pass e.style['scroll-timeline'] = "block none" should not set the property value
24+
Pass e.style['scroll-timeline'] = "inline --abc" should not set the property value
25+
Pass e.style['scroll-timeline'] = "default" should not set the property value
26+
Pass e.style['scroll-timeline'] = "," should not set the property value
27+
Pass e.style['scroll-timeline'] = ",,block,," should not set the property value
28+
Pass Property scroll-timeline value 'none block'
29+
Pass Property scroll-timeline value '--abc inline'
30+
Pass Property scroll-timeline value 'none y'
31+
Pass Property scroll-timeline value '--abc x'
32+
Pass Property scroll-timeline value '--y y'
33+
Pass Property scroll-timeline value '--abc'
34+
Pass Property scroll-timeline value '--inline block'
35+
Pass Property scroll-timeline value '--block block'
36+
Pass Property scroll-timeline value '--y block'
37+
Pass Property scroll-timeline value '--x block'
38+
Pass Property scroll-timeline value '--a, --b, --c'
39+
Pass Property scroll-timeline value '--a inline, --b block, --c y'
40+
Pass e.style['scroll-timeline'] = "--abc y" should set scroll-timeline-axis
41+
Pass e.style['scroll-timeline'] = "--abc y" should set scroll-timeline-name
42+
Pass e.style['scroll-timeline'] = "--abc y" should not set unrelated longhands
43+
Pass e.style['scroll-timeline'] = "--inline x" should set scroll-timeline-axis
44+
Pass e.style['scroll-timeline'] = "--inline x" should set scroll-timeline-name
45+
Pass e.style['scroll-timeline'] = "--inline x" should not set unrelated longhands
46+
Pass e.style['scroll-timeline'] = "--abc y, --def" should set scroll-timeline-axis
47+
Pass e.style['scroll-timeline'] = "--abc y, --def" should set scroll-timeline-name
48+
Pass e.style['scroll-timeline'] = "--abc y, --def" should not set unrelated longhands
49+
Fail e.style['scroll-timeline'] = "--abc, --def" should set scroll-timeline-axis
50+
Pass e.style['scroll-timeline'] = "--abc, --def" should set scroll-timeline-name
51+
Pass e.style['scroll-timeline'] = "--abc, --def" should not set unrelated longhands
52+
Pass Shorthand contraction of scroll-timeline-name:--abc:undefined;scroll-timeline-axis:inline:undefined
53+
Pass Shorthand contraction of scroll-timeline-name:--a, --b:undefined;scroll-timeline-axis:inline, block:undefined
54+
Pass Shorthand contraction of scroll-timeline-name:none, none:undefined;scroll-timeline-axis:block, block:undefined
55+
Fail Shorthand contraction of scroll-timeline-name:--a, --b, --c:undefined;scroll-timeline-axis:inline, inline:undefined
56+
Fail Shorthand contraction of scroll-timeline-name:--a, --b:undefined;scroll-timeline-axis:inline, inline, inline:undefined

‎Tests/LibWeb/Text/input/wpt-import/scroll-animations/css/scroll-timeline-shorthand.html‎

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
<!DOCTYPE html>
2+
<link rel="help" href="https://drafts.csswg.org/scroll-animations-1/#scroll-timeline-shorthand">
3+
<script src="../../resources/testharness.js"></script>
4+
<script src="../../resources/testharnessreport.js"></script>
5+
<script src="../../css/support/computed-testcommon.js"></script>
6+
<script src="../../css/support/parsing-testcommon.js"></script>
7+
<script src="../../css/support/shorthand-testcommon.js"></script>
8+
<div id="target"></div>
9+
<script>
10+
test_valid_value('scroll-timeline', 'none block', 'none');
11+
test_valid_value('scroll-timeline', 'none inline');
12+
test_valid_value('scroll-timeline', '--abc x');
13+
test_valid_value('scroll-timeline', '--abc inline');
14+
test_valid_value('scroll-timeline', '--aBc inline');
15+
test_valid_value('scroll-timeline', '--inline inline');
16+
test_valid_value('scroll-timeline', '--abc');
17+
18+
test_valid_value('scroll-timeline', '--inline block', '--inline');
19+
test_valid_value('scroll-timeline', '--block block', '--block');
20+
test_valid_value('scroll-timeline', '--y block', '--y');
21+
test_valid_value('scroll-timeline', '--x block', '--x');
22+
23+
test_valid_value('scroll-timeline', '--a, --b, --c');
24+
test_valid_value('scroll-timeline', '--a inline, --b block, --c y', '--a inline, --b, --c y');
25+
test_valid_value('scroll-timeline', '--auto');
26+
27+
test_invalid_value('scroll-timeline', '');
28+
test_invalid_value('scroll-timeline', '--abc --abc');
29+
test_invalid_value('scroll-timeline', 'block none');
30+
test_invalid_value('scroll-timeline', 'inline --abc');
31+
test_invalid_value('scroll-timeline', 'default');
32+
test_invalid_value('scroll-timeline', ',');
33+
test_invalid_value('scroll-timeline', ',,block,,');
34+
35+
test_computed_value('scroll-timeline', 'none block', 'none');
36+
test_computed_value('scroll-timeline', '--abc inline');
37+
test_computed_value('scroll-timeline', 'none y');
38+
test_computed_value('scroll-timeline', '--abc x');
39+
test_computed_value('scroll-timeline', '--y y');
40+
test_computed_value('scroll-timeline', '--abc');
41+
test_computed_value('scroll-timeline', '--inline block', '--inline');
42+
test_computed_value('scroll-timeline', '--block block', '--block');
43+
test_computed_value('scroll-timeline', '--y block', '--y');
44+
test_computed_value('scroll-timeline', '--x block', '--x');
45+
test_computed_value('scroll-timeline', '--a, --b, --c');
46+
test_computed_value('scroll-timeline', '--a inline, --b block, --c y', '--a inline, --b, --c y');
47+
48+
test_shorthand_value('scroll-timeline', '--abc y',
49+
{
50+
'scroll-timeline-name': '--abc',
51+
'scroll-timeline-axis': 'y',
52+
});
53+
test_shorthand_value('scroll-timeline', '--inline x',
54+
{
55+
'scroll-timeline-name': '--inline',
56+
'scroll-timeline-axis': 'x',
57+
});
58+
test_shorthand_value('scroll-timeline', '--abc y, --def',
59+
{
60+
'scroll-timeline-name': '--abc, --def',
61+
'scroll-timeline-axis': 'y, block',
62+
});
63+
test_shorthand_value('scroll-timeline', '--abc, --def',
64+
{
65+
'scroll-timeline-name': '--abc, --def',
66+
'scroll-timeline-axis': 'block, block',
67+
});
68+
69+
function test_shorthand_contraction(shorthand, longhands, expected) {
70+
let longhands_fmt = Object.entries(longhands).map((e) => `${e[0]}:${e[1]}:${e[2]}`).join(';');
71+
test((t) => {
72+
t.add_cleanup(() => {
73+
for (let shorthand of Object.keys(longhands))
74+
target.style.removeProperty(shorthand);
75+
});
76+
for (let [shorthand, value] of Object.entries(longhands))
77+
target.style.setProperty(shorthand, value);
78+
assert_equals(target.style.getPropertyValue(shorthand), expected, 'Declared value');
79+
assert_equals(getComputedStyle(target).getPropertyValue(shorthand), expected, 'Computed value');
80+
}, `Shorthand contraction of ${longhands_fmt}`);
81+
}
82+
83+
test_shorthand_contraction('scroll-timeline', {
84+
'scroll-timeline-name': '--abc',
85+
'scroll-timeline-axis': 'inline',
86+
}, '--abc inline');
87+
88+
test_shorthand_contraction('scroll-timeline', {
89+
'scroll-timeline-name': '--a, --b',
90+
'scroll-timeline-axis': 'inline, block',
91+
}, '--a inline, --b');
92+
93+
test_shorthand_contraction('scroll-timeline', {
94+
'scroll-timeline-name': 'none, none',
95+
'scroll-timeline-axis': 'block, block',
96+
}, 'none, none');
97+
98+
// Longhands with different lengths:
99+
100+
test_shorthand_contraction('scroll-timeline', {
101+
'scroll-timeline-name': '--a, --b, --c',
102+
'scroll-timeline-axis': 'inline, inline',
103+
}, '--a inline, --b inline, --c inline');
104+
105+
test_shorthand_contraction('scroll-timeline', {
106+
'scroll-timeline-name': '--a, --b',
107+
'scroll-timeline-axis': 'inline, inline, inline',
108+
}, '--a inline, --b inline');
109+
</script>

0 commit comments

Comments
 (0)