Title: [124137] branches/safari-536.26-branch
Revision
124137
Author
[email protected]
Date
2012-07-30 18:26:42 -0700 (Mon, 30 Jul 2012)

Log Message

Merged r122271.  <rdar://problem/11932360>

Modified Paths

Diff

Modified: branches/safari-536.26-branch/LayoutTests/ChangeLog (124136 => 124137)


--- branches/safari-536.26-branch/LayoutTests/ChangeLog	2012-07-31 01:21:45 UTC (rev 124136)
+++ branches/safari-536.26-branch/LayoutTests/ChangeLog	2012-07-31 01:26:42 UTC (rev 124137)
@@ -1,5 +1,24 @@
 2012-07-30  Lucas Forschler  <[email protected]>
 
+    Merge 122271
+
+    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-30  Lucas Forschler  <[email protected]>
+
     Merge 122152
 
     2012-07-09  Dean Jackson  <[email protected]>

Modified: branches/safari-536.26-branch/Source/WebCore/ChangeLog (124136 => 124137)


--- branches/safari-536.26-branch/Source/WebCore/ChangeLog	2012-07-31 01:21:45 UTC (rev 124136)
+++ branches/safari-536.26-branch/Source/WebCore/ChangeLog	2012-07-31 01:26:42 UTC (rev 124137)
@@ -1,5 +1,35 @@
 2012-07-30  Lucas Forschler  <[email protected]>
 
+    Merge 122271
+
+    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-30  Lucas Forschler  <[email protected]>
+
     Merge 122228
 
     2012-07-10  Alice Cheng  <[email protected]>

Modified: branches/safari-536.26-branch/Source/WebCore/css/CSSParser.cpp (124136 => 124137)


--- branches/safari-536.26-branch/Source/WebCore/css/CSSParser.cpp	2012-07-31 01:21:45 UTC (rev 124136)
+++ branches/safari-536.26-branch/Source/WebCore/css/CSSParser.cpp	2012-07-31 01:26:42 UTC (rev 124137)
@@ -2771,19 +2771,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])
@@ -2795,13 +2818,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;
                 }
             }
         }
@@ -2812,16 +2835,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;
 }
 
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to