Title: [200043] trunk
Revision
200043
Author
simon.fra...@apple.com
Date
2016-04-25 13:06:09 -0700 (Mon, 25 Apr 2016)

Log Message

play-state not parsed as part of animation shorthand
https://bugs.webkit.org/show_bug.cgi?id=156959

Reviewed by Darin Adler.

Source/WebCore:

We failed to parse animation-play-state as part of the animation shorthand, contrary
to the spec and other browsers.

Fix for both the prefixed and unprefixed properties. There is some compat risk here,
but only changing unprefixed behavior will probably lead to more author confusion.

Test: animations/play-state-in-shorthand.html

* css/CSSParser.cpp:
(WebCore::CSSParser::parseAnimationShorthand):
* css/CSSPropertyNames.in:
* css/StylePropertyShorthand.cpp:
(WebCore::animationShorthandForParsing): Remove the long comment which is no longer relevant
now that the behavior has been written into the spec.

LayoutTests:

* animations/animation-shorthand-expected.txt:
* animations/animation-shorthand.html:
* animations/play-state-in-shorthand-expected.txt: Added.
* animations/play-state-in-shorthand.html: Added.
* animations/resources/animation-test-helpers.js:
(getPropertyValue):
(comparePropertyValue):

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (200042 => 200043)


--- trunk/LayoutTests/ChangeLog	2016-04-25 20:06:04 UTC (rev 200042)
+++ trunk/LayoutTests/ChangeLog	2016-04-25 20:06:09 UTC (rev 200043)
@@ -1,5 +1,20 @@
 2016-04-25  Simon Fraser  <simon.fra...@apple.com>
 
+        play-state not parsed as part of animation shorthand
+        https://bugs.webkit.org/show_bug.cgi?id=156959
+
+        Reviewed by Darin Adler.
+
+        * animations/animation-shorthand-expected.txt:
+        * animations/animation-shorthand.html:
+        * animations/play-state-in-shorthand-expected.txt: Added.
+        * animations/play-state-in-shorthand.html: Added.
+        * animations/resources/animation-test-helpers.js:
+        (getPropertyValue):
+        (comparePropertyValue):
+
+2016-04-25  Simon Fraser  <simon.fra...@apple.com>
+
         Negative animation-delay is treated as 0s
         https://bugs.webkit.org/show_bug.cgi?id=141008
 

Modified: trunk/LayoutTests/animations/animation-shorthand-expected.txt (200042 => 200043)


--- trunk/LayoutTests/animations/animation-shorthand-expected.txt	2016-04-25 20:06:04 UTC (rev 200042)
+++ trunk/LayoutTests/animations/animation-shorthand-expected.txt	2016-04-25 20:06:09 UTC (rev 200043)
@@ -5,6 +5,7 @@
 Testing webkitAnimationIterationCount on a: PASS
 Testing webkitAnimationDirection on a: PASS
 Testing webkitAnimationFillMode on a: PASS
+Testing webkitAnimationPlayState on a: PASS
 Testing webkitAnimationName on b: PASS
 Testing webkitAnimationDuration on b: PASS
 Testing webkitAnimationTimingFunction on b: PASS
@@ -12,6 +13,7 @@
 Testing webkitAnimationIterationCount on b: PASS
 Testing webkitAnimationDirection on b: PASS
 Testing webkitAnimationFillMode on b: PASS
+Testing webkitAnimationPlayState on b: PASS
 Testing webkitAnimationName on c: PASS
 Testing webkitAnimationDuration on c: PASS
 Testing webkitAnimationTimingFunction on c: PASS
@@ -19,6 +21,7 @@
 Testing webkitAnimationIterationCount on c: PASS
 Testing webkitAnimationDirection on c: PASS
 Testing webkitAnimationFillMode on c: PASS
+Testing webkitAnimationPlayState on c: PASS
 Testing webkitAnimationName on d: PASS
 Testing webkitAnimationDuration on d: PASS
 Testing webkitAnimationTimingFunction on d: PASS
@@ -26,6 +29,7 @@
 Testing webkitAnimationIterationCount on d: PASS
 Testing webkitAnimationDirection on d: PASS
 Testing webkitAnimationFillMode on d: PASS
+Testing webkitAnimationPlayState on d: PASS
 Testing webkitAnimationName on e: PASS
 Testing webkitAnimationDuration on e: PASS
 Testing webkitAnimationTimingFunction on e: PASS
@@ -33,6 +37,7 @@
 Testing webkitAnimationIterationCount on e: PASS
 Testing webkitAnimationDirection on e: PASS
 Testing webkitAnimationFillMode on e: PASS
+Testing webkitAnimationPlayState on e: PASS
 Testing webkitAnimationName on f: PASS
 Testing webkitAnimationDuration on f: PASS
 Testing webkitAnimationTimingFunction on f: PASS
@@ -40,6 +45,7 @@
 Testing webkitAnimationIterationCount on f: PASS
 Testing webkitAnimationDirection on f: PASS
 Testing webkitAnimationFillMode on f: PASS
+Testing webkitAnimationPlayState on f: PASS
 Testing webkitAnimationName on g: PASS
 Testing webkitAnimationDuration on g: PASS
 Testing webkitAnimationTimingFunction on g: PASS
@@ -47,6 +53,7 @@
 Testing webkitAnimationIterationCount on g: PASS
 Testing webkitAnimationDirection on g: PASS
 Testing webkitAnimationFillMode on g: PASS
+Testing webkitAnimationPlayState on g: PASS
 Testing webkitAnimationName on h: PASS
 Testing webkitAnimationDuration on h: PASS
 Testing webkitAnimationTimingFunction on h: PASS
@@ -54,6 +61,7 @@
 Testing webkitAnimationIterationCount on h: PASS
 Testing webkitAnimationDirection on h: PASS
 Testing webkitAnimationFillMode on h: PASS
+Testing webkitAnimationPlayState on h: PASS
 Testing webkitAnimationName on i: PASS
 Testing webkitAnimationDuration on i: PASS
 Testing webkitAnimationTimingFunction on i: PASS
@@ -61,6 +69,7 @@
 Testing webkitAnimationIterationCount on i: PASS
 Testing webkitAnimationDirection on i: PASS
 Testing webkitAnimationFillMode on i: PASS
+Testing webkitAnimationPlayState on i: PASS
 Testing webkitAnimationName on j: PASS
 Testing webkitAnimationDuration on j: PASS
 Testing webkitAnimationTimingFunction on j: PASS
@@ -68,4 +77,13 @@
 Testing webkitAnimationIterationCount on j: PASS
 Testing webkitAnimationDirection on j: PASS
 Testing webkitAnimationFillMode on j: PASS
+Testing webkitAnimationPlayState on j: PASS
+Testing webkitAnimationName on k: PASS
+Testing webkitAnimationDuration on k: PASS
+Testing webkitAnimationTimingFunction on k: PASS
+Testing webkitAnimationDelay on k: PASS
+Testing webkitAnimationIterationCount on k: PASS
+Testing webkitAnimationDirection on k: PASS
+Testing webkitAnimationFillMode on k: PASS
+Testing webkitAnimationPlayState on k: PASS
 

Modified: trunk/LayoutTests/animations/animation-shorthand.html (200042 => 200043)


--- trunk/LayoutTests/animations/animation-shorthand.html	2016-04-25 20:06:04 UTC (rev 200042)
+++ trunk/LayoutTests/animations/animation-shorthand.html	2016-04-25 20:06:09 UTC (rev 200043)
@@ -34,8 +34,11 @@
   -webkit-animation: anim1 10s linear normal none;
 }
 #j {
-  -webkit-animation: anim1 10s linear infinite backwards, anim2 3s none, anim3 5s both;
+  -webkit-animation: anim1 10s ease infinite both paused;
 }
+#k {
+  -webkit-animation: anim1 10s linear infinite backwards, anim2 3s none paused, anim3 5s both;
+}
 
 @-webkit-keyframes anim1 { }
 @-webkit-keyframes anim2 { }
@@ -51,19 +54,21 @@
       "webkitAnimationDelay",
       "webkitAnimationIterationCount",
       "webkitAnimationDirection",
-      "webkitAnimationFillMode"
+      "webkitAnimationFillMode",
+      "webkitAnimationPlayState",
     ];
     const kExpectedResults = [
-      { id: 'a',  values: [ "none", "0s", "ease", "0s", "1", "normal", "none" ] },
-      { id: 'b',  values: [ "none", "0s", "ease", "0s", "1", "normal", "none" ] },
-      { id: 'c',  values: [ "anim1", "10s", "ease", "0s", "1", "normal", "none" ] },
-      { id: 'd',  values: [ "anim1", "10s", "linear", "0s", "1", "normal", "none" ] },
-      { id: 'e',  values: [ "anim1", "10s", "linear", "5s", "1", "normal", "none" ] },
-      { id: 'f',  values: [ "anim1", "10s", "linear", "5s", "3", "normal", "none" ] },
-      { id: 'g',  values: [ "anim1", "10s", "linear", "5s", "infinite", "alternate", "none" ] },
-      { id: 'h',  values: [ "anim1", "10s", "linear", "5s", "infinite", "alternate", "forwards" ] },
-      { id: 'i',  values: [ "anim1", "10s", "linear", "0s", "1", "normal", "none" ] },
-      { id: 'j',  values: [ "anim1, anim2, anim3", "10s, 3s, 5s", "linear, ease, ease", "0s, 0s, 0s", "infinite, 1, 1", "normal, normal, normal", "backwards, none, both" ] }
+      { id: 'a',  values: [ "none", "0s", "ease", "0s", "1", "normal", "none", "running" ] },
+      { id: 'b',  values: [ "none", "0s", "ease", "0s", "1", "normal", "none", "running" ] },
+      { id: 'c',  values: [ "anim1", "10s", "ease", "0s", "1", "normal", "none", "running" ] },
+      { id: 'd',  values: [ "anim1", "10s", "linear", "0s", "1", "normal", "none", "running" ] },
+      { id: 'e',  values: [ "anim1", "10s", "linear", "5s", "1", "normal", "none", "running" ] },
+      { id: 'f',  values: [ "anim1", "10s", "linear", "5s", "3", "normal", "none", "running" ] },
+      { id: 'g',  values: [ "anim1", "10s", "linear", "5s", "infinite", "alternate", "none", "running" ] },
+      { id: 'h',  values: [ "anim1", "10s", "linear", "5s", "infinite", "alternate", "forwards", "running" ] },
+      { id: 'i',  values: [ "anim1", "10s", "linear", "0s", "1", "normal", "none", "running" ] },
+      { id: 'j',  values: [ "anim1", "10s", "ease", "0s", "infinite", "normal", "both", "paused" ] },
+      { id: 'k',  values: [ "anim1, anim2, anim3", "10s, 3s, 5s", "linear, ease, ease", "0s, 0s, 0s", "infinite, 1, 1", "normal, normal, normal", "backwards, none, both", "running, paused, running" ] },
     ];
     
     function start()
@@ -102,6 +107,7 @@
 <div id="h" class="box"></div>
 <div id="i" class="box"></div>
 <div id="j" class="box"></div>
+<div id="k" class="box"></div>
 <div id="result">
 </div>
 </body>

Added: trunk/LayoutTests/animations/play-state-in-shorthand-expected.txt (0 => 200043)


--- trunk/LayoutTests/animations/play-state-in-shorthand-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/animations/play-state-in-shorthand-expected.txt	2016-04-25 20:06:09 UTC (rev 200043)
@@ -0,0 +1,4 @@
+PASS - "transform" property for "box" element at 0.5s saw something close to: 1,0,0,1,75,0
+PASS - "transform" property for "box" element at 1s saw something close to: 1,0,0,1,150,0
+PASS - "transform" property for "box" element at 2.5s saw something close to: 1,0,0,1,150,0
+

Added: trunk/LayoutTests/animations/play-state-in-shorthand.html (0 => 200043)


--- trunk/LayoutTests/animations/play-state-in-shorthand.html	                        (rev 0)
+++ trunk/LayoutTests/animations/play-state-in-shorthand.html	2016-04-25 20:06:09 UTC (rev 200043)
@@ -0,0 +1,55 @@
+<!DOCTYPE html>
+<html>
+<head>
+  <style>
+    body {
+      margin: 0;
+    }
+
+    #box {
+      position: absolute;
+      left: 0px;
+      top: 100px;
+      height: 100px;
+      width: 100px;
+      background-color: blue;
+      animation: move1 linear 2s;
+    }
+
+    #box.paused {
+        animation: move1 linear 2s paused;
+    }
+
+    @keyframes move1 {
+      from { transform: translateX(0px); }
+      to   { transform: translateX(300px); }
+    }
+  </style>
+  <script src="" type="text/_javascript_"></script>
+  <script type="text/_javascript_">
+    const expectedValues = [
+      // [animation-name, time, element-id, property, expected-value, tolerance]
+      ["move1", 0.5, "box", "transform", [1,0,0,1,75,0], 20],
+      ["move1", 1.0, "box", "transform", [1,0,0,1,150,0], 20],
+      ["move1", 2.5, "box", "transform", [1,0,0,1,150,0], 20],
+    ];
+
+    function pauseAnimation()
+    {
+        document.getElementById("box").classList.add('paused');
+    }
+
+    function setTimers()
+    {
+        setTimeout(pauseAnimation, 1000);
+    }
+
+    runAnimationTest(expectedValues, setTimers, null, true);
+  </script>
+</head>
+<body>
+<div id="box"></div>
+<div id="result"></div>
+</div>
+</body>
+</html>

Modified: trunk/LayoutTests/animations/resources/animation-test-helpers.js (200042 => 200043)


--- trunk/LayoutTests/animations/resources/animation-test-helpers.js	2016-04-25 20:06:04 UTC (rev 200042)
+++ trunk/LayoutTests/animations/resources/animation-test-helpers.js	2016-04-25 20:06:09 UTC (rev 200043)
@@ -399,7 +399,8 @@
                || property == "webkitClipPath"
                || property == "webkitShapeInside"
                || property == "webkitShapeOutside"
-               || !property.indexOf("webkitTransform")) {
+               || !property.indexOf("webkitTransform")
+               || !property.indexOf("transform")) {
         computedValue = window.getComputedStyle(element)[property.split(".")[0]];
     } else {
         var computedStyle = window.getComputedStyle(element).getPropertyCSSValue(property);
@@ -413,7 +414,7 @@
 {
     var result = true;
 
-    if (!property.indexOf("webkitTransform")) {
+    if (!property.indexOf("webkitTransform") || !property.indexOf("transform")) {
         if (typeof expectedValue == "string")
             result = (computedValue == expectedValue);
         else if (typeof expectedValue == "number") {

Modified: trunk/Source/WebCore/ChangeLog (200042 => 200043)


--- trunk/Source/WebCore/ChangeLog	2016-04-25 20:06:04 UTC (rev 200042)
+++ trunk/Source/WebCore/ChangeLog	2016-04-25 20:06:09 UTC (rev 200043)
@@ -1,5 +1,27 @@
 2016-04-25  Simon Fraser  <simon.fra...@apple.com>
 
+        play-state not parsed as part of animation shorthand
+        https://bugs.webkit.org/show_bug.cgi?id=156959
+
+        Reviewed by Darin Adler.
+
+        We failed to parse animation-play-state as part of the animation shorthand, contrary
+        to the spec and other browsers.
+
+        Fix for both the prefixed and unprefixed properties. There is some compat risk here,
+        but only changing unprefixed behavior will probably lead to more author confusion.
+
+        Test: animations/play-state-in-shorthand.html
+
+        * css/CSSParser.cpp:
+        (WebCore::CSSParser::parseAnimationShorthand):
+        * css/CSSPropertyNames.in:
+        * css/StylePropertyShorthand.cpp:
+        (WebCore::animationShorthandForParsing): Remove the long comment which is no longer relevant
+        now that the behavior has been written into the spec.
+
+2016-04-25  Simon Fraser  <simon.fra...@apple.com>
+
         Negative animation-delay is treated as 0s
         https://bugs.webkit.org/show_bug.cgi?id=141008
 

Modified: trunk/Source/WebCore/css/CSSParser.cpp (200042 => 200043)


--- trunk/Source/WebCore/css/CSSParser.cpp	2016-04-25 20:06:04 UTC (rev 200042)
+++ trunk/Source/WebCore/css/CSSParser.cpp	2016-04-25 20:06:09 UTC (rev 200043)
@@ -3785,7 +3785,7 @@
 {
     ASSERT(propId == CSSPropertyAnimation || propId == CSSPropertyWebkitAnimation);
 
-    const unsigned numProperties = 7;
+    const unsigned numProperties = 8;
     const StylePropertyShorthand& shorthand = animationShorthandForParsing(propId);
 
     // The list of properties in the shorthand should be the same

Modified: trunk/Source/WebCore/css/CSSPropertyNames.in (200042 => 200043)


--- trunk/Source/WebCore/css/CSSPropertyNames.in	2016-04-25 20:06:04 UTC (rev 200042)
+++ trunk/Source/WebCore/css/CSSPropertyNames.in	2016-04-25 20:06:09 UTC (rev 200043)
@@ -132,7 +132,7 @@
 // The remaining properties are listed in alphabetical order
 alignment-baseline [SVG]
 all [Longhands=all]
-animation [Longhands=animation-name|animation-duration|animation-timing-function|animation-delay|animation-iteration-count|animation-direction|animation-fill-mode]
+animation [Longhands=animation-name|animation-duration|animation-timing-function|animation-delay|animation-iteration-count|animation-direction|animation-fill-mode|animation-play-state]
 animation-delay [AnimationProperty, NameForMethods=Delay]
 animation-direction [AnimationProperty, NameForMethods=Direction]
 animation-duration [AnimationProperty, NameForMethods=Duration]
@@ -372,7 +372,7 @@
 z-index [AutoFunctions]
 alt [NameForMethods=ContentAltText, Custom=Value]
 -webkit-alt = alt
--webkit-animation [Longhands=-webkit-animation-name|-webkit-animation-duration|-webkit-animation-timing-function|-webkit-animation-delay|-webkit-animation-iteration-count|-webkit-animation-direction|-webkit-animation-fill-mode]
+-webkit-animation [Longhands=-webkit-animation-name|-webkit-animation-duration|-webkit-animation-timing-function|-webkit-animation-delay|-webkit-animation-iteration-count|-webkit-animation-direction|-webkit-animation-fill-mode|-webkit-animation-play-state]
 -webkit-animation-delay [AnimationProperty, NameForMethods=Delay]
 -webkit-animation-direction [AnimationProperty, NameForMethods=Direction]
 -webkit-animation-duration [AnimationProperty, NameForMethods=Duration]

Modified: trunk/Source/WebCore/css/StylePropertyShorthand.cpp (200042 => 200043)


--- trunk/Source/WebCore/css/StylePropertyShorthand.cpp	2016-04-25 20:06:04 UTC (rev 200042)
+++ trunk/Source/WebCore/css/StylePropertyShorthand.cpp	2016-04-25 20:06:09 UTC (rev 200043)
@@ -35,15 +35,8 @@
 
 StylePropertyShorthand animationShorthandForParsing(CSSPropertyID propId)
 {
-    // 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 the authors 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
+    // Animation-name must come last, so that keywords for other properties in the shorthand
+    // preferentially match those properties.
     static const CSSPropertyID animationPropertiesForParsing[] = {
         CSSPropertyAnimationDuration,
         CSSPropertyAnimationTimingFunction,
@@ -51,6 +44,7 @@
         CSSPropertyAnimationIterationCount,
         CSSPropertyAnimationDirection,
         CSSPropertyAnimationFillMode,
+        CSSPropertyAnimationPlayState,
         CSSPropertyAnimationName
     };
 
@@ -61,6 +55,7 @@
         CSSPropertyWebkitAnimationIterationCount,
         CSSPropertyWebkitAnimationDirection,
         CSSPropertyWebkitAnimationFillMode,
+        CSSPropertyWebkitAnimationPlayState,
         CSSPropertyWebkitAnimationName
     };
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to