Skip to content

Commit

Permalink
[scroll-animations] make animation-timeline, `animation-range-start…
Browse files Browse the repository at this point in the history
…` and `animation-range-end` reset-only longhands of the `animation` shorthand

https://bugs.webkit.org/show_bug.cgi?id=287398

Reviewed by Tim Nguyen.

The CSS WG resolved in w3c/csswg-drafts#6946 (comment) to make the
new CSS properties introduced to support Scroll-driven Animations – `animation-timeline`, `animation-range-start`
and `animation-range-end` – reset-only longhands of the `animation` shorthand.

To that end, we add those properties to the `"longhands"` property for `animation` in `CSSProperties.json`
and deal with them when relevant:

- in `CSSPropertyParser.cpp` we set `nullptr` values for those properties to reset any value previously set
and ensure those properties have the right initial values for serialization purposes.

- in `ComputedStyleExtractor.cpp` we return an empty value whenever we attempt to serialize the `animation`
shorthand in the computed style when one of those reset-only longhands was otherwise set.

- in `ShorthandSerializer.cpp` we do similar work when serializing the specified value.

* LayoutTests/imported/w3c/web-platform-tests/css/css-animations/parsing/animation-shorthand-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/scroll-animations/css/animation-shorthand-expected.txt:
* Source/WebCore/css/CSSProperties.json:
* Source/WebCore/css/ComputedStyleExtractor.cpp:
(WebCore::animationShorthandValue):
* Source/WebCore/css/ShorthandSerializer.cpp:
(WebCore::ShorthandSerializer::serializeLayered const):
* Source/WebCore/css/parser/CSSPropertyParser.cpp:
(WebCore::initialValueForLonghand):
(WebCore::consumeAnimationValueForShorthand):
(WebCore::CSSPropertyParser::consumeAnimationShorthand):

Canonical link: https://commits.webkit.org/290168@main
  • Loading branch information
graouts committed Feb 10, 2025
1 parent a64f0e2 commit 91fb05a
Show file tree
Hide file tree
Showing 6 changed files with 76 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ PASS e.style['animation'] = "anim paused both reverse 4 1s -3s cubic-bezier(0, -
PASS e.style['animation'] = "anim paused both reverse 4 1s -3s cubic-bezier(0, -2, 1, 3)" should set animation-iteration-count
PASS e.style['animation'] = "anim paused both reverse 4 1s -3s cubic-bezier(0, -2, 1, 3)" should set animation-name
PASS e.style['animation'] = "anim paused both reverse 4 1s -3s cubic-bezier(0, -2, 1, 3)" should set animation-play-state
FAIL e.style['animation'] = "anim paused both reverse 4 1s -3s cubic-bezier(0, -2, 1, 3)" should set animation-range-end assert_equals: animation-range-end should be canonical expected "normal" but got ""
FAIL e.style['animation'] = "anim paused both reverse 4 1s -3s cubic-bezier(0, -2, 1, 3)" should set animation-range-start assert_equals: animation-range-start should be canonical expected "normal" but got ""
FAIL e.style['animation'] = "anim paused both reverse 4 1s -3s cubic-bezier(0, -2, 1, 3)" should set animation-timeline assert_equals: animation-timeline should be canonical expected "auto" but got ""
PASS e.style['animation'] = "anim paused both reverse 4 1s -3s cubic-bezier(0, -2, 1, 3)" should set animation-range-end
PASS e.style['animation'] = "anim paused both reverse 4 1s -3s cubic-bezier(0, -2, 1, 3)" should set animation-range-start
PASS e.style['animation'] = "anim paused both reverse 4 1s -3s cubic-bezier(0, -2, 1, 3)" should set animation-timeline
PASS e.style['animation'] = "anim paused both reverse 4 1s -3s cubic-bezier(0, -2, 1, 3)" should set animation-timing-function
PASS e.style['animation'] = "anim paused both reverse 4 1s -3s cubic-bezier(0, -2, 1, 3)" should not set unrelated longhands
PASS e.style['animation'] = "anim paused both reverse, 4 1s -3s cubic-bezier(0, -2, 1, 3)" should set animation-delay
Expand All @@ -18,9 +18,9 @@ PASS e.style['animation'] = "anim paused both reverse, 4 1s -3s cubic-bezier(0,
PASS e.style['animation'] = "anim paused both reverse, 4 1s -3s cubic-bezier(0, -2, 1, 3)" should set animation-iteration-count
PASS e.style['animation'] = "anim paused both reverse, 4 1s -3s cubic-bezier(0, -2, 1, 3)" should set animation-name
PASS e.style['animation'] = "anim paused both reverse, 4 1s -3s cubic-bezier(0, -2, 1, 3)" should set animation-play-state
FAIL e.style['animation'] = "anim paused both reverse, 4 1s -3s cubic-bezier(0, -2, 1, 3)" should set animation-range-end assert_equals: animation-range-end should be canonical expected "normal" but got ""
FAIL e.style['animation'] = "anim paused both reverse, 4 1s -3s cubic-bezier(0, -2, 1, 3)" should set animation-range-start assert_equals: animation-range-start should be canonical expected "normal" but got ""
FAIL e.style['animation'] = "anim paused both reverse, 4 1s -3s cubic-bezier(0, -2, 1, 3)" should set animation-timeline assert_equals: animation-timeline should be canonical expected "auto" but got ""
PASS e.style['animation'] = "anim paused both reverse, 4 1s -3s cubic-bezier(0, -2, 1, 3)" should set animation-range-end
PASS e.style['animation'] = "anim paused both reverse, 4 1s -3s cubic-bezier(0, -2, 1, 3)" should set animation-range-start
PASS e.style['animation'] = "anim paused both reverse, 4 1s -3s cubic-bezier(0, -2, 1, 3)" should set animation-timeline
PASS e.style['animation'] = "anim paused both reverse, 4 1s -3s cubic-bezier(0, -2, 1, 3)" should set animation-timing-function
PASS e.style['animation'] = "anim paused both reverse, 4 1s -3s cubic-bezier(0, -2, 1, 3)" should not set unrelated longhands
PASS e.style['animation'] = "4 1s -3s cubic-bezier(0, -2, 1, 3), anim paused both reverse" should set animation-delay
Expand All @@ -30,9 +30,9 @@ PASS e.style['animation'] = "4 1s -3s cubic-bezier(0, -2, 1, 3), anim paused bot
PASS e.style['animation'] = "4 1s -3s cubic-bezier(0, -2, 1, 3), anim paused both reverse" should set animation-iteration-count
PASS e.style['animation'] = "4 1s -3s cubic-bezier(0, -2, 1, 3), anim paused both reverse" should set animation-name
PASS e.style['animation'] = "4 1s -3s cubic-bezier(0, -2, 1, 3), anim paused both reverse" should set animation-play-state
FAIL e.style['animation'] = "4 1s -3s cubic-bezier(0, -2, 1, 3), anim paused both reverse" should set animation-range-end assert_equals: animation-range-end should be canonical expected "normal" but got ""
FAIL e.style['animation'] = "4 1s -3s cubic-bezier(0, -2, 1, 3), anim paused both reverse" should set animation-range-start assert_equals: animation-range-start should be canonical expected "normal" but got ""
FAIL e.style['animation'] = "4 1s -3s cubic-bezier(0, -2, 1, 3), anim paused both reverse" should set animation-timeline assert_equals: animation-timeline should be canonical expected "auto" but got ""
PASS e.style['animation'] = "4 1s -3s cubic-bezier(0, -2, 1, 3), anim paused both reverse" should set animation-range-end
PASS e.style['animation'] = "4 1s -3s cubic-bezier(0, -2, 1, 3), anim paused both reverse" should set animation-range-start
PASS e.style['animation'] = "4 1s -3s cubic-bezier(0, -2, 1, 3), anim paused both reverse" should set animation-timeline
PASS e.style['animation'] = "4 1s -3s cubic-bezier(0, -2, 1, 3), anim paused both reverse" should set animation-timing-function
PASS e.style['animation'] = "4 1s -3s cubic-bezier(0, -2, 1, 3), anim paused both reverse" should not set unrelated longhands

Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,17 @@ PASS e.style['animation'] = "1s linear 1s 2 reverse forwards paused anim1,\n 1
PASS e.style['animation'] = "1s linear 1s 2 reverse forwards paused anim1,\n 1s linear 1s 2 reverse forwards paused anim2,\n 1s linear 1s 2 reverse forwards paused anim3" should set animation-iteration-count
PASS e.style['animation'] = "1s linear 1s 2 reverse forwards paused anim1,\n 1s linear 1s 2 reverse forwards paused anim2,\n 1s linear 1s 2 reverse forwards paused anim3" should set animation-name
PASS e.style['animation'] = "1s linear 1s 2 reverse forwards paused anim1,\n 1s linear 1s 2 reverse forwards paused anim2,\n 1s linear 1s 2 reverse forwards paused anim3" should set animation-play-state
FAIL e.style['animation'] = "1s linear 1s 2 reverse forwards paused anim1,\n 1s linear 1s 2 reverse forwards paused anim2,\n 1s linear 1s 2 reverse forwards paused anim3" should set animation-range-end assert_equals: animation-range-end should be canonical expected "normal" but got ""
FAIL e.style['animation'] = "1s linear 1s 2 reverse forwards paused anim1,\n 1s linear 1s 2 reverse forwards paused anim2,\n 1s linear 1s 2 reverse forwards paused anim3" should set animation-range-start assert_equals: animation-range-start should be canonical expected "normal" but got ""
FAIL e.style['animation'] = "1s linear 1s 2 reverse forwards paused anim1,\n 1s linear 1s 2 reverse forwards paused anim2,\n 1s linear 1s 2 reverse forwards paused anim3" should set animation-timeline assert_equals: animation-timeline should be canonical expected "auto" but got ""
PASS e.style['animation'] = "1s linear 1s 2 reverse forwards paused anim1,\n 1s linear 1s 2 reverse forwards paused anim2,\n 1s linear 1s 2 reverse forwards paused anim3" should set animation-range-end
PASS e.style['animation'] = "1s linear 1s 2 reverse forwards paused anim1,\n 1s linear 1s 2 reverse forwards paused anim2,\n 1s linear 1s 2 reverse forwards paused anim3" should set animation-range-start
PASS e.style['animation'] = "1s linear 1s 2 reverse forwards paused anim1,\n 1s linear 1s 2 reverse forwards paused anim2,\n 1s linear 1s 2 reverse forwards paused anim3" should set animation-timeline
PASS e.style['animation'] = "1s linear 1s 2 reverse forwards paused anim1,\n 1s linear 1s 2 reverse forwards paused anim2,\n 1s linear 1s 2 reverse forwards paused anim3" should set animation-timing-function
PASS e.style['animation'] = "1s linear 1s 2 reverse forwards paused anim1,\n 1s linear 1s 2 reverse forwards paused anim2,\n 1s linear 1s 2 reverse forwards paused anim3" should not set unrelated longhands
FAIL Animation shorthand can not represent non-initial timelines (specified) assert_equals: expected "" but got "1s anim"
FAIL Animation shorthand can not represent non-initial timelines (computed) assert_equals: expected "" but got "1s anim --timeline"
FAIL Animation shorthand can not represent non-initial animation-range-start (specified) assert_equals: expected "" but got "1s anim"
FAIL Animation shorthand can not represent non-initial animation-range-start (computed) assert_equals: expected "" but got "1s anim"
FAIL Animation shorthand can not represent non-initial animation-range-end (specified) assert_equals: expected "" but got "1s anim"
FAIL Animation shorthand can not represent non-initial animation-range-end (computed) assert_equals: expected "" but got "1s anim"
FAIL Animation shorthand can not be represented with same list length (specified) assert_equals: expected "" but got "1s ease-in infinite forwards paused bounce, 0.2s linear 1s 2 reverse backwards roll"
PASS Animation shorthand can not represent non-initial timelines (specified)
PASS Animation shorthand can not represent non-initial timelines (computed)
PASS Animation shorthand can not represent non-initial animation-range-start (specified)
PASS Animation shorthand can not represent non-initial animation-range-start (computed)
PASS Animation shorthand can not represent non-initial animation-range-end (specified)
PASS Animation shorthand can not represent non-initial animation-range-end (computed)
PASS Animation shorthand can not be represented with same list length (specified)
PASS Animation shorthand can be represented with same list length (computed)

5 changes: 4 additions & 1 deletion Source/WebCore/css/CSSProperties.json
Original file line number Diff line number Diff line change
Expand Up @@ -1470,7 +1470,10 @@
"animation-direction",
"animation-fill-mode",
"animation-play-state",
"animation-name"
"animation-name",
"animation-timeline",
"animation-range-start",
"animation-range-end"
]
},
"specification": {
Expand Down
9 changes: 7 additions & 2 deletions Source/WebCore/css/ComputedStyleExtractor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1894,9 +1894,14 @@ static Ref<CSSValue> animationShorthandValue(const RenderStyle& style, const Ani
return CSSPrimitiveValue::create(CSSValueNone);

CSSValueListBuilder list;
for (auto& animation : *animations)
for (auto& animation : *animations) {
// If any of the reset-only longhands are set, we cannot serialize this value.
if (animation->isTimelineSet() || animation->isRangeStartSet() || animation->isRangeEndSet()) {
list.clear();
break;
}
list.append(singleAnimationValue(style, animation));
ASSERT(!list.isEmpty());
}
return CSSValueList::createCommaSeparated(WTFMove(list));
}

Expand Down
15 changes: 15 additions & 0 deletions Source/WebCore/css/ShorthandSerializer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -656,6 +656,7 @@ String ShorthandSerializer::serializeLayered() const
}
}

bool hasUnskippedValue = false;
for (unsigned j = 0; j < length(); j++) {
auto longhand = longhandProperty(j);

Expand Down Expand Up @@ -703,9 +704,23 @@ String ShorthandSerializer::serializeLayered() const
}
}

// If we get to "animation-timeline" and yet haven't encountered an unskipped value,
// this means that this "animation" shorthand only has initial values for the non
// reset-only longhands and so we cannot serialize it. We only do this when we deal
// with multiple layers since the single "none" case would be caught otherwise.
if (numLayers > 1 && longhand == CSSPropertyAnimationTimeline && !hasUnskippedValue)
return String();

if (layerValues.skip(j))
continue;

hasUnskippedValue = true;

// If we encounter one of the reset-only "animation" longhands and the value was not skipped,
// then it was set to a non-initial value and we cannot serialize it.
if (longhand == CSSPropertyAnimationTimeline || longhand == CSSPropertyAnimationRangeStart || longhand == CSSPropertyAnimationRangeEnd)
return String();

// The syntax for background-size means that if it is present, background-position must be too.
// The syntax for mask-size means that if it is present, mask-position must be too.
if (longhand == CSSPropertyBackgroundSize || longhand == CSSPropertyMaskSize) {
Expand Down
41 changes: 31 additions & 10 deletions Source/WebCore/css/parser/CSSPropertyParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -976,6 +976,7 @@ static constexpr InitialValue initialValueForLonghand(CSSPropertyID longhand)
case CSSPropertyAccentColor:
case CSSPropertyAlignSelf:
case CSSPropertyAnimationDuration:
case CSSPropertyAnimationTimeline:
case CSSPropertyAspectRatio:
case CSSPropertyBackgroundSize:
case CSSPropertyBlockSize:
Expand Down Expand Up @@ -1057,6 +1058,8 @@ static constexpr InitialValue initialValueForLonghand(CSSPropertyID longhand)
case CSSPropertyAlignContent:
case CSSPropertyAlignItems:
case CSSPropertyAnimationDirection:
case CSSPropertyAnimationRangeEnd:
case CSSPropertyAnimationRangeStart:
case CSSPropertyBackgroundBlendMode:
case CSSPropertyColumnGap:
case CSSPropertyContainerType:
Expand Down Expand Up @@ -1098,7 +1101,6 @@ static constexpr InitialValue initialValueForLonghand(CSSPropertyID longhand)
return InitialNumericValue { 0, CSSUnitType::CSS_S };
case CSSPropertyAnimationFillMode:
case CSSPropertyAnimationName:
case CSSPropertyAnimationTimeline:
case CSSPropertyAppearance:
case CSSPropertyBackgroundImage:
case CSSPropertyBlockEllipsis:
Expand Down Expand Up @@ -1880,10 +1882,12 @@ static RefPtr<CSSValue> consumeAnimationValueForShorthand(CSSPropertyID property
return CSSPropertyParsing::consumeSingleAnimationPlayState(range);
case CSSPropertyAnimationComposition:
return CSSPropertyParsing::consumeSingleAnimationComposition(range);
case CSSPropertyAnimationTimeline:
case CSSPropertyAnimationRangeStart:
case CSSPropertyAnimationRangeEnd:
return nullptr; // reset-only longhands
case CSSPropertyTransitionProperty:
return consumeSingleTransitionPropertyOrNone(range, context);
case CSSPropertyAnimationTimeline:
return consumeAnimationTimeline(range, context);
case CSSPropertyAnimationTimingFunction:
case CSSPropertyTransitionTimingFunction:
return consumeEasingFunction(range, context);
Expand All @@ -1898,12 +1902,24 @@ static RefPtr<CSSValue> consumeAnimationValueForShorthand(CSSPropertyID property
bool CSSPropertyParser::consumeAnimationShorthand(const StylePropertyShorthand& shorthand, bool important)
{
auto shorthandProperties = shorthand.properties();
const unsigned longhandCount = shorthand.length();
std::array<CSSValueListBuilder, 8> longhands;
ASSERT(longhandCount <= 8);
const size_t longhandCount = shorthand.length();
const size_t maxLonghandCount = 11;
std::array<CSSValueListBuilder, maxLonghandCount> longhands;
ASSERT(longhandCount <= maxLonghandCount);

auto isResetOnlyLonghand = [](CSSPropertyID longhand) {
switch (longhand) {
case CSSPropertyAnimationTimeline:
case CSSPropertyAnimationRangeStart:
case CSSPropertyAnimationRangeEnd:
return true;
default:
return false;
}
};

do {
std::array<bool, 8> parsedLonghand = { };
std::array<bool, maxLonghandCount> parsedLonghand = { };
do {
bool foundProperty = false;
for (size_t i = 0; i < longhandCount; ++i) {
Expand All @@ -1922,7 +1938,7 @@ bool CSSPropertyParser::consumeAnimationShorthand(const StylePropertyShorthand&
} while (!m_range.atEnd() && m_range.peek().type() != CommaToken);

for (size_t i = 0; i < longhandCount; ++i) {
if (!parsedLonghand[i])
if (!parsedLonghand[i] && !isResetOnlyLonghand(shorthandProperties[i]))
longhands[i].append(Ref { CSSPrimitiveValue::implicitInitialValue() });
parsedLonghand[i] = false;
}
Expand All @@ -1933,8 +1949,13 @@ bool CSSPropertyParser::consumeAnimationShorthand(const StylePropertyShorthand&
return false;
}

for (size_t i = 0; i < longhandCount; ++i)
addProperty(shorthandProperties[i], shorthand.id(), CSSValueList::createCommaSeparated(WTFMove(longhands[i])), important);
for (size_t i = 0; i < longhandCount; ++i) {
auto& list = longhands[i];
if (list.isEmpty()) // reset-only property
addProperty(shorthandProperties[i], shorthand.id(), nullptr, important);
else
addProperty(shorthandProperties[i], shorthand.id(), CSSValueList::createCommaSeparated(WTFMove(list)), important);
}

return m_range.atEnd();
}
Expand Down

0 comments on commit 91fb05a

Please sign in to comment.