Bug 1140134 - Don't skip the rest of the properties in an animation after hitting one that we shouldn't apply. r=dholbert, a=lmandel
authorL. David Baron <dbaron@dbaron.org>
Fri, 06 Mar 2015 13:35:45 -0800
changeset 250310 4b50714e7419
parent 250309 116d059a1e7d
child 250311 46392e569cb6
push id4542
push userryanvm@gmail.com
push date2015-03-09 19:13 +0000
treeherdermozilla-beta@46392e569cb6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert, lmandel
bugs1140134
milestone37.0
Bug 1140134 - Don't skip the rest of the properties in an animation after hitting one that we shouldn't apply. r=dholbert, a=lmandel Both sets of new tests pass with the patch, but without the patch the "top is animating" test fails.
dom/animation/Animation.cpp
layout/style/test/test_animations.html
--- a/dom/animation/Animation.cpp
+++ b/dom/animation/Animation.cpp
@@ -289,17 +289,17 @@ Animation::ComposeStyle(nsRefPtr<css::An
     MOZ_ASSERT(prop.mSegments[prop.mSegments.Length() - 1].mToKey == 1.0,
                "incorrect last to key");
 
     if (aSetProperties.HasProperty(prop.mProperty)) {
       // Animations are composed by AnimationPlayerCollection by iterating
       // from the last animation to first. For animations targetting the
       // same property, the later one wins. So if this property is already set,
       // we should not override it.
-      return;
+      continue;
     }
 
     aSetProperties.AddProperty(prop.mProperty);
 
     MOZ_ASSERT(prop.mSegments.Length() > 0,
                "property should not be in animations if it has no segments");
 
     // FIXME: Maybe cache the current segment?
--- a/layout/style/test/test_animations.html
+++ b/layout/style/test/test_animations.html
@@ -134,16 +134,35 @@ https://bugzilla.mozilla.org/show_bug.cg
     from { margin-top: 50px;
            margin-bottom: 100px; }
     to   { margin-top: 150px !important; /* ignored */
            margin-bottom: 50px; }
   }
 
   @keyframes empty { }
   @keyframes nearlyempty { to { margin-left: 100px; } }
+
+  @keyframes lowerpriority {
+    0% {
+      top: 0px;
+      left: 0px;
+    }
+    100% {
+      top: 100px;
+      left: 100px;
+    }
+  }
+
+  @keyframes overrideleft {
+    0%, 100% { left: 0px }
+  }
+
+  @keyframes overridetop {
+    0%, 100% { top: 0px }
+  }
   </style>
 </head>
 <body>
 <a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=435442">Mozilla Bug 435442</a>
 <div id="display"></div>
 <pre id="test">
 <script type="application/javascript">
 "use strict";
@@ -1928,14 +1947,32 @@ done_div();
 new_div("animation: always_fifty 10s 1s infinite");
 advance_clock(0);
 advance_clock(2000);
 is(cs.marginLeft, "50px",
    "infinitely repeating animation with positive delay takes effect"
    + " (does not overflow)");
 done_div();
 
+/*
+ * Bug 1140134 - A property in a CSS animation being overridden by later
+ * animation causes later properties in that animation to be skipped
+ */
+new_div("position: relative; animation: lowerpriority 1s linear infinite alternate, overridetop 1s linear infinite alternate");
+advance_clock(0);
+advance_clock(500);
+is(cs.getPropertyValue("left"), "50px", "left is animating");
+is(cs.getPropertyValue("top"), "0px", "top is not animating");
+done_div();
+
+new_div("position: relative; animation: lowerpriority 1s linear infinite alternate, overrideleft 1s linear infinite alternate");
+advance_clock(0);
+advance_clock(500);
+is(cs.getPropertyValue("left"), "0px", "left is not animating");
+is(cs.getPropertyValue("top"), "50px", "top is animating");
+done_div();
+
 SpecialPowers.DOMWindowUtils.restoreNormalRefresh();
 
 </script>
 </pre>
 </body>
 </html>