Modified: trunk/LayoutTests/ChangeLog (122270 => 122271)
--- trunk/LayoutTests/ChangeLog 2012-07-10 22:14:34 UTC (rev 122270)
+++ trunk/LayoutTests/ChangeLog 2012-07-10 22:45:34 UTC (rev 122271)
@@ -1,3 +1,18 @@
+2012-07-10 Dean Jackson <[email protected]>
+
+ REGRESSION (r109610): Order of values in shorthand animation makes a difference
+ https://bugs.webkit.org/show_bug.cgi?id=84533
+ <rdar://problem/11831924>
+ <rdar://problem/11815787>
+
+ Reviewed by Simon Fraser.
+
+ A new test that exercises many different variants of the animation shorthand
+ property, putting the animation name in different spots in the list of values.
+
+ * animations/animation-shorthand-name-order-expected.txt: Added.
+ * animations/animation-shorthand-name-order.html: Added.
+
2012-07-10 Rafael Weinstein <[email protected]>
Unreviewed gardening. Removing duplicated line from TestExpectations.
Modified: trunk/Source/WebCore/ChangeLog (122270 => 122271)
--- trunk/Source/WebCore/ChangeLog 2012-07-10 22:14:34 UTC (rev 122270)
+++ trunk/Source/WebCore/ChangeLog 2012-07-10 22:45:34 UTC (rev 122271)
@@ -1,3 +1,29 @@
+2012-07-10 Dean Jackson <[email protected]>
+
+ REGRESSION (r109610): Order of values in shorthand animation makes a difference
+ https://bugs.webkit.org/show_bug.cgi?id=84533
+ <rdar://problem/11831924>
+ <rdar://problem/11815787>
+
+ Reviewed by Simon Fraser.
+
+ A previous revision (r109610) updated the parsing of the animation shorthand
+ to make sure that animation-name wouldn't clobber other styles. The side effect
+ of this was that we'd no longer find animation-name if it wasn't first in the
+ list. This commit reverts the change and fixes it in a different way, by always
+ parsing animation-name as the last property in the shorthand. This means that
+ keywords for timing functions, fill modes and iteration will match before
+ animation name. In other words, if you want an animation called "forwards"
+ you should use the longhand property, because the shorthand will first match
+ that against animation-fill-mode.
+
+ Test: animations/animation-shorthand-name-order.html
+
+ * css/CSSParser.cpp:
+ (WebCore::CSSParser::parseAnimationShorthand): make a new array of longhand
+ properties to check for, with name as the last entry rather than the first.
+ Use this array to test the properties in the shorthand.
+
2012-07-10 Huang Dongsung <[email protected]>
Fix a potential bug of BitmapImage::frameCount().
Modified: trunk/Source/WebCore/css/CSSParser.cpp (122270 => 122271)
--- trunk/Source/WebCore/css/CSSParser.cpp 2012-07-10 22:14:34 UTC (rev 122270)
+++ trunk/Source/WebCore/css/CSSParser.cpp 2012-07-10 22:45:34 UTC (rev 122271)
@@ -3038,19 +3038,42 @@
bool CSSParser::parseAnimationShorthand(bool important)
{
- const unsigned numProperties = webkitAnimationShorthand().length();
+ // When we parse the animation shorthand we need to look for animation-name
+ // last because otherwise it might match against the keywords for fill mode,
+ // timing functions and infinite iteration. This means that animation names
+ // that are the same as keywords (e.g. 'forwards') won't always match in the
+ // shorthand. In that case they should be using longhands (or reconsidering
+ // their approach). This is covered by the animations spec bug:
+ // https://www.w3.org/Bugs/Public/show_bug.cgi?id=14790
+ // And in the spec (editor's draft) at:
+ // http://dev.w3.org/csswg/css3-animations/#animation-shorthand-property
+
+ static const CSSPropertyID animationProperties[] = {
+ CSSPropertyWebkitAnimationDuration,
+ CSSPropertyWebkitAnimationTimingFunction,
+ CSSPropertyWebkitAnimationDelay,
+ CSSPropertyWebkitAnimationIterationCount,
+ CSSPropertyWebkitAnimationDirection,
+ CSSPropertyWebkitAnimationFillMode,
+ CSSPropertyWebkitAnimationName
+ };
+ const unsigned numProperties = 7;
+
+ // The list of properties in the shorthand should be the same
+ // length as the list we have here, even though they are
+ // a different order.
+ ASSERT(numProperties == webkitAnimationShorthand().length());
+
ShorthandScope scope(this, CSSPropertyWebkitAnimation);
- bool parsedProperty[] = { false, false, false, false, false, false, false };
- RefPtr<CSSValue> values[7];
+ bool parsedProperty[numProperties] = { false };
+ RefPtr<CSSValue> values[numProperties];
unsigned i;
- unsigned initialParsedPropertyIndex = 0;
while (m_valueList->current()) {
CSSParserValue* val = m_valueList->current();
if (val->unit == CSSParserValue::Operator && val->iValue == ',') {
// We hit the end. Fill in all remaining values with the initial value.
- initialParsedPropertyIndex = 0;
m_valueList->next();
for (i = 0; i < numProperties; ++i) {
if (!parsedProperty[i])
@@ -3062,13 +3085,13 @@
}
bool found = false;
- for (i = initialParsedPropertyIndex; !found && i < numProperties; ++i) {
+ for (i = 0; i < numProperties; ++i) {
if (!parsedProperty[i]) {
RefPtr<CSSValue> val;
- if (parseAnimationProperty(webkitAnimationShorthand().properties()[i], val)) {
+ if (parseAnimationProperty(animationProperties[i], val)) {
parsedProperty[i] = found = true;
- initialParsedPropertyIndex = 1;
addAnimationValue(values[i], val.release());
+ break;
}
}
}
@@ -3079,16 +3102,14 @@
return false;
}
- // Fill in any remaining properties with the initial value.
for (i = 0; i < numProperties; ++i) {
+ // If we didn't find the property, set an intial value.
if (!parsedProperty[i])
addAnimationValue(values[i], cssValuePool().createImplicitInitialValue());
+
+ addProperty(animationProperties[i], values[i].release(), important);
}
- // Now add all of the properties we found.
- for (i = 0; i < numProperties; i++)
- addProperty(webkitAnimationShorthand().properties()[i], values[i].release(), important);
-
return true;
}
