Skip to content
Permalink
Browse files
'animation-foo' declarations in @Keyframes should be parse error
https://bugs.webkit.org/show_bug.cgi?id=243537
<rdar://98503617>

Reviewed by Antti Koivisto.

The family of animation-* properties, including the animation shorthand, are
invalid in a @Keyframes rule. We import the relevant WPT test which used to
fail but now passes as of this change. Some other existing tests need rebasing
since we changed the serialization to not output the vendor prefix, and also
because they are passing thanks to this change.

* LayoutTests/http/tests/css/shared-stylesheet-mutation-preconstruct-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-animations/parsing/keyframes-allowed-properties-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-animations/parsing/keyframes-allowed-properties.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-conditional/js/001-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/cssom/CSSKeyframesRule-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/cssom/rule-restrictions-expected.txt:
* Source/WebCore/css/CSSKeyframesRule.cpp:
(WebCore::CSSKeyframesRule::cssText const):
* Source/WebCore/css/parser/CSSPropertyParser.cpp:
(WebCore::CSSPropertyParser::parseValue):
(WebCore::CSSPropertyParser::parseKeyframeDescriptor):
* Source/WebCore/css/parser/CSSPropertyParser.h:

Canonical link: https://commits.webkit.org/254468@main
  • Loading branch information
graouts committed Sep 14, 2022
1 parent 64c4c2c commit edad56b68bd20b2e91cc54b6ad04c6d33d97124e
Show file tree
Hide file tree
Showing 9 changed files with 82 additions and 20 deletions.
@@ -13,7 +13,7 @@ PASS getComputedStyle(testElement, null).getPropertyValue('background-color') is
#testdiv { background-color: red; }
}
@page :right { margin-left: 3cm; }
@-webkit-keyframes bounce {
@keyframes bounce {
0% { left: 0px; }
100% { left: 200px; }
}
@@ -29,7 +29,7 @@ PASS getComputedStyle(testElement, null).getPropertyValue('background-color') is
}
@page :right { margin-left: 3cm; }
#testdiv { background-color: green; }
@-webkit-keyframes bounce {
@keyframes bounce {
0% { left: 0px; }
100% { left: 200px; }
}
@@ -41,7 +41,7 @@ PASS getComputedStyle(testElement, null).getPropertyValue('background-color') is

#testdiv { background-color: green; }
@page :right { margin-left: 3cm; }
@-webkit-keyframes bounce {
@keyframes bounce {
0% { left: 0px; }
100% { left: 200px; }
}
@@ -57,7 +57,7 @@ PASS getComputedStyle(testElement, null).getPropertyValue('background-color') is
#testdiv { background-color: green; }
}
@page :right { margin-left: 3cm; }
@-webkit-keyframes bounce {
@keyframes bounce {
0% { left: 0px; }
100% { left: 200px; }
}
@@ -71,7 +71,7 @@ PASS getComputedStyle(testElement, null).getPropertyValue('background-color') is
@media all {
}
@page :right { margin-left: 3cm; }
@-webkit-keyframes bounce {
@keyframes bounce {
0% { left: 0px; }
100% { left: 200px; }
}
@@ -86,7 +86,7 @@ PASS getComputedStyle(testElement, null).getPropertyValue('background-color') is
#testdiv { background-color: green; }
}
@page :right { margin-left: 3cm; }
@-webkit-keyframes bounce {
@keyframes bounce {
0% { left: 0px; }
100% { left: 200px; }
}
@@ -101,7 +101,7 @@ PASS getComputedStyle(testElement, null).getPropertyValue('background-color') is
#testdiv { }
}
@page :right { margin-left: 3cm; }
@-webkit-keyframes bounce {
@keyframes bounce {
0% { left: 0px; }
100% { left: 200px; }
}
@@ -116,7 +116,7 @@ PASS getComputedStyle(testElement, null).getPropertyValue('background-color') is
#testdiv { background-color: green; }
}
@page :right { margin-left: 3cm; }
@-webkit-keyframes bounce {
@keyframes bounce {
0% { left: 0px; }
100% { left: 200px; }
}
@@ -131,7 +131,7 @@ PASS getComputedStyle(testElement, null).getPropertyValue('background-color') is
#dontmatch { background-color: red; }
}
@page :right { margin-left: 3cm; }
@-webkit-keyframes bounce {
@keyframes bounce {
0% { left: 0px; }
100% { left: 200px; }
}
@@ -146,7 +146,7 @@ PASS getComputedStyle(testElement, null).getPropertyValue('background-color') is
#testdiv { background-color: red; }
}
@page :right { margin-left: 3cm; }
@-webkit-keyframes bounce {
@keyframes bounce {
0% { left: 0px; }
100% { left: 200px; }
}
@@ -161,7 +161,7 @@ PASS getComputedStyle(testElement, null).getPropertyValue('background-color') is
#testdiv { background-color: red; }
}
@page :right { margin-left: 3cm; }
@-webkit-keyframes bounce {
@keyframes bounce {
0% { left: 0px; }
100% { left: 200px; }
}
@@ -183,7 +183,7 @@ PASS getComputedStyle(testElement, null).getPropertyValue('background-color') is
#testdiv { background-color: red; }
}
@page :right { margin-left: 3cm; }
@-webkit-keyframes bounce {
@keyframes bounce {
0% { left: 0px; }
100% { left: 200px; }
}
@@ -198,7 +198,7 @@ PASS getComputedStyle(testElement, null).getPropertyValue('background-color') is
#testdiv { background-color: red; }
}
@page foo { margin-left: 3cm; }
@-webkit-keyframes bounce {
@keyframes bounce {
0% { left: 0px; }
100% { left: 200px; }
}
@@ -213,7 +213,7 @@ PASS getComputedStyle(testElement, null).getPropertyValue('background-color') is
#testdiv { background-color: red; }
}
@page :right { margin-left: 3cm; }
@-webkit-keyframes bounce {
@keyframes bounce {
0% { left: 0px; }
100% { left: 200px; }
40% { left: 40px; }
@@ -229,7 +229,7 @@ PASS getComputedStyle(testElement, null).getPropertyValue('background-color') is
#testdiv { background-color: red; }
}
@page :right { margin-left: 3cm; }
@-webkit-keyframes bounce {
@keyframes bounce {
0% { left: 0px; }
}
@font-face { font-family: Foo; }
@@ -243,7 +243,7 @@ PASS getComputedStyle(testElement, null).getPropertyValue('background-color') is
#testdiv { background-color: red; }
}
@page :right { margin-left: 3cm; }
@-webkit-keyframes bounce {
@keyframes bounce {
0% { left: 0px; }
100% { left: 200px; }
}
@@ -0,0 +1,3 @@

PASS @keyframes allows all non-animation properties and animation-timing-function

@@ -0,0 +1,35 @@
<!DOCTYPE html>
<title>Tests which properties are allowed in @keyframes</title>
<link rel="help" href="https://drafts.csswg.org/css-animations/#typedef-keyframe-block">
<link rel="author" href="mailto:xiaochengh@chromium.org">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<style id=sheet>
@keyframes foo {
from {
/* Non-animation properties are allowed */
margin-top: 10px;
/* animation-timing-function is specially allowed */
animation-timing-function: ease;
/* All other animation properties are not allowed */
animation-name: none;
animation-duration: 1s;
animation-iteration-count: 1;
animation-direction: normal;
animation-play-state: running;
animation-delay: 0s;
animation-fill-mode: none;
/* The animation shorthand is also not allowed */
animation: bar 1s infinite;
}
}
</style>
<script>
test(() => {
const keyframe = sheet.sheet.cssRules[0].cssRules[0];
const style = keyframe.style;
assert_equals(style.length, 2);
assert_equals(style.marginTop, '10px');
assert_equals(style.animationTimingFunction, 'ease');
}, '@keyframes allows all non-animation properties and animation-timing-function');
</script>
@@ -1,6 +1,6 @@

PASS nested @supports serialize properly
FAIL @keyframes nested in @supports serialize properly assert_equals: expected "@media all { @supports (padding: 0) { @keyframes foo { 0% { top: 0px; left: 0px; } 100% { top: 100px; left: 100px; } } } }" but got "@media all { @supports (padding: 0) { @-webkit-keyframes foo { 0% { top: 0px; left: 0px; } 100% { top: 100px; left: 100px; } } } }"
PASS @keyframes nested in @supports serialize properly
PASS The style sheet structure is properly represented
PASS Deleting the top level of a nested structue works
PASS Rule insertion works in nested @supports rules
@@ -1,3 +1,3 @@

FAIL CSSOM - CSSKeyframesRule interface assert_equals: CSSKeyframesRule cssText attribute expected "@keyframesbar{}" but got "@-webkit-keyframesbar{}"
FAIL CSSOM - CSSKeyframesRule interface assert_equals: CSSKeyframesRule cssText attribute with CSS-wide keyword name expected "@keyframes\"initial\"{}" but got "@keyframesinitial{}"

@@ -1,4 +1,4 @@

FAIL @page assert_equals: Transform doesn't quite apply to pages expected 1 but got 2
FAIL @keyframe assert_equals: animation-name doesn't apply to keyframes expected 1 but got 2
PASS @keyframe

@@ -169,7 +169,7 @@ CSSKeyframeRule* CSSKeyframesRule::findRule(const String& s)
String CSSKeyframesRule::cssText() const
{
StringBuilder result;
result.append("@-webkit-keyframes ", name(), " { \n");
result.append("@keyframes ", name(), " { \n");
for (unsigned i = 0, size = length(); i < size; ++i)
result.append(" ", m_keyframesRule->keyframes()[i]->cssText(), '\n');
result.append('}');
@@ -256,6 +256,8 @@ bool CSSPropertyParser::parseValue(CSSPropertyID propertyID, bool important, con
parseSuccess = parser.parseFontPaletteValuesDescriptor(propertyID);
else if (ruleType == StyleRuleType::CounterStyle)
parseSuccess = parser.parseCounterStyleDescriptor(propertyID, context);
else if (ruleType == StyleRuleType::Keyframe)
parseSuccess = parser.parseKeyframeDescriptor(propertyID, important);
else
parseSuccess = parser.parseValueStart(propertyID, important);

@@ -5171,6 +5173,27 @@ bool CSSPropertyParser::parseFontFaceDescriptor(CSSPropertyID propId)
return true;
}

bool CSSPropertyParser::parseKeyframeDescriptor(CSSPropertyID propertyID, bool important)
{
// https://www.w3.org/TR/css-animations-1/#keyframes
// The <declaration-list> inside of <keyframe-block> accepts any CSS property except those
// defined in this specification, but does accept the animation-timing-function property and
// interprets it specially.
switch (propertyID) {
case CSSPropertyAnimation:
case CSSPropertyAnimationDelay:
case CSSPropertyAnimationDirection:
case CSSPropertyAnimationDuration:
case CSSPropertyAnimationFillMode:
case CSSPropertyAnimationIterationCount:
case CSSPropertyAnimationName:
case CSSPropertyAnimationPlayState:
return false;
default:
return parseValueStart(propertyID, important);
}
}

static RefPtr<CSSPrimitiveValue> consumeBasePaletteDescriptor(CSSParserTokenRange& range)
{
if (auto result = consumeIdent<CSSValueLight, CSSValueDark>(range))
@@ -75,6 +75,7 @@ class CSSPropertyParser {
bool parseFontFaceDescriptor(CSSPropertyID);
bool parseFontPaletteValuesDescriptor(CSSPropertyID);
bool parseCounterStyleDescriptor(CSSPropertyID, const CSSParserContext&);
bool parseKeyframeDescriptor(CSSPropertyID, bool important);

void addProperty(CSSPropertyID, CSSPropertyID, Ref<CSSValue>&&, bool important, bool implicit = false);
void addPropertyWithImplicitDefault(CSSPropertyID, CSSPropertyID, RefPtr<CSSValue>&&, Ref<CSSValue>&& implicitDefault, bool important);

0 comments on commit edad56b

Please sign in to comment.