play-state not parsed as part of animation shorthand
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 25 Apr 2016 20:06:09 +0000 (20:06 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 25 Apr 2016 20:06:09 +0000 (20:06 +0000)
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):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@200043 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/animations/animation-shorthand-expected.txt
LayoutTests/animations/animation-shorthand.html
LayoutTests/animations/play-state-in-shorthand-expected.txt [new file with mode: 0644]
LayoutTests/animations/play-state-in-shorthand.html [new file with mode: 0644]
LayoutTests/animations/resources/animation-test-helpers.js
Source/WebCore/ChangeLog
Source/WebCore/css/CSSParser.cpp
Source/WebCore/css/CSSPropertyNames.in
Source/WebCore/css/StylePropertyShorthand.cpp

index 8f14f53..3e86c81 100644 (file)
@@ -1,5 +1,20 @@
 2016-04-25  Simon Fraser  <simon.fraser@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.fraser@apple.com>
+
         Negative animation-delay is treated as 0s
         https://bugs.webkit.org/show_bug.cgi?id=141008
 
index f2610f5..9c4b1d3 100644 (file)
@@ -5,6 +5,7 @@ Testing webkitAnimationDelay on a: PASS
 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 webkitAnimationDelay on b: PASS
 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 webkitAnimationDelay on c: PASS
 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 webkitAnimationDelay on d: PASS
 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 webkitAnimationDelay on e: PASS
 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 webkitAnimationDelay on f: PASS
 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 webkitAnimationDelay on g: PASS
 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 webkitAnimationDelay on h: PASS
 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 webkitAnimationDelay on i: PASS
 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 webkitAnimationDelay on j: PASS
 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
 
index 805c835..40ff143 100644 (file)
   -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 { }
       "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()
 <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>
diff --git a/LayoutTests/animations/play-state-in-shorthand-expected.txt b/LayoutTests/animations/play-state-in-shorthand-expected.txt
new file mode 100644 (file)
index 0000000..0df044b
--- /dev/null
@@ -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
+
diff --git a/LayoutTests/animations/play-state-in-shorthand.html b/LayoutTests/animations/play-state-in-shorthand.html
new file mode 100644 (file)
index 0000000..eae2fb3
--- /dev/null
@@ -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="resources/animation-test-helpers.js" 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>
index e0eac14..b044545 100644 (file)
@@ -399,7 +399,8 @@ function getPropertyValue(property, elementId, iframeId)
                || 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 @@ function comparePropertyValue(property, computedValue, expectedValue, tolerance)
 {
     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") {
index 047af58..dda64f2 100644 (file)
@@ -1,5 +1,27 @@
 2016-04-25  Simon Fraser  <simon.fraser@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.fraser@apple.com>
+
         Negative animation-delay is treated as 0s
         https://bugs.webkit.org/show_bug.cgi?id=141008
 
index b6bebcb..4e3d6cb 100644 (file)
@@ -3785,7 +3785,7 @@ bool CSSParser::parseAnimationShorthand(CSSPropertyID propId, bool important)
 {
     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
index 6f755aa..fdffe5d 100644 (file)
@@ -132,7 +132,7 @@ font-synthesis [Inherited, FontProperty, Converter=FontSynthesis]
 // 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 @@ y [Initial=initialZeroLength, Converter=Length]
 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]
index c384295..48d4a04 100644 (file)
@@ -35,15 +35,8 @@ StylePropertyShorthand borderAbridgedShorthand()
 
 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 @@ StylePropertyShorthand animationShorthandForParsing(CSSPropertyID propId)
         CSSPropertyAnimationIterationCount,
         CSSPropertyAnimationDirection,
         CSSPropertyAnimationFillMode,
+        CSSPropertyAnimationPlayState,
         CSSPropertyAnimationName
     };
 
@@ -61,6 +55,7 @@ StylePropertyShorthand animationShorthandForParsing(CSSPropertyID propId)
         CSSPropertyWebkitAnimationIterationCount,
         CSSPropertyWebkitAnimationDirection,
         CSSPropertyWebkitAnimationFillMode,
+        CSSPropertyWebkitAnimationPlayState,
         CSSPropertyWebkitAnimationName
     };