Bug 1536688 - Allow animating the 'all' property from Web Animations;…

… r=emilio Differential Revision: https://phabricator.services.mozilla.com/D28763
mozilla-spidermonkey · Apr 26, 2019 · 8894214 · 8894214
1 parent f904761
commit 8894214
Show file tree

Hide file tree

Showing 5 changed files with 81 additions and 2 deletions.
diff --git a/dom/animation/test/chrome.ini b/dom/animation/test/chrome.ini
@@ -17,6 +17,7 @@ support-files =
 [chrome/test_animation_observers_sync.html]
 [chrome/test_animation_performance_warning.html]
 [chrome/test_animation_properties.html]
+[chrome/test_animation_properties_display.html]
 [chrome/test_cssanimation_missing_keyframes.html]
 [chrome/test_generated_content_getAnimations.html]
 [chrome/test_running_on_compositor.html]

diff --git a/dom/animation/test/chrome/test_animation_properties_display.html b/dom/animation/test/chrome/test_animation_properties_display.html
@@ -0,0 +1,43 @@
+<!doctype html>
+<head>
+<meta charset=utf-8>
+<title>Bug 1536688 - Test that 'display' is not included in
+  KeyframeEffect.getProperties() when using shorthand 'all'</title>
+<script type="application/javascript" src="../testharness.js"></script>
+<script type="application/javascript" src="../testharnessreport.js"></script>
+<script type="application/javascript" src="../testcommon.js"></script>
+</head>
+<body>
+<a href="https://bugzilla.mozilla.org/show_bug.cgi?id=1536688"
+  target="_blank">Mozilla Bug 1536688</a>
+<div id="log"></div>
+<script>
+'use strict';
+
+SpecialPowers.pushPrefEnv(
+  {
+    set: [['dom.animations-api.core.enabled', true]],
+  },
+  function() {
+    test(t => {
+      const div = addDiv(t);
+      const animation = div.animate(
+        { all: ['unset', 'unset'] },
+        100 * MS_PER_SEC
+      );
+      // Flush styles since getProperties does not.
+      getComputedStyle(div).opacity;
+
+      const properties = animation.effect.getProperties();
+      assert_false(
+        properties.some(property => property.property === 'display'),
+        'Should not have a property for display'
+      );
+    });
+
+    done();
+  }
+);
+
+</script>
+</body>
diff --git a/servo/components/style/properties/data.py b/servo/components/style/properties/data.py
@@ -400,8 +400,6 @@ def __init__(self, name, sub_properties, spec=None, servo_pref=None, gecko_pref=
             and allowed_in_keyframe_block != "False"
 
     def get_animatable(self):
-        if self.ident == "all":
-            return False
         for sub in self.sub_properties:
             if sub.animatable:
                 return True

diff --git a/servo/ports/geckolib/glue.rs b/servo/ports/geckolib/glue.rs
@@ -5104,6 +5104,13 @@ pub extern "C" fn Servo_GetComputedKeyframeValues(
             let mut maybe_append_animation_value =
                 |property: LonghandId, value: Option<AnimationValue>| {
                     debug_assert!(!property.is_logical());
+                    debug_assert!(property.is_animatable());
+
+                    // 'display' is only animatable from SMIL
+                    if property == LonghandId::Display {
+                        return;
+                    }
+
                     if seen.contains(property) {
                         return;
                     }

diff --git a/.../tests/web-animations/animation-model/keyframe-effects/computed-keyframes-shorthands.html b/.../tests/web-animations/animation-model/keyframe-effects/computed-keyframes-shorthands.html
@@ -0,0 +1,30 @@
+<!doctype html>
+<meta charset=utf-8>
+<title>Calculating computed keyframes: Shorthand properties</title>
+<link rel="help" href="https://drafts.csswg.org/web-animations-1/#calculating-computed-keyframes">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="../../testcommon.js"></script>
+<body>
+<div id="log"></div>
+<div id="target"></div>
+<script>
+'use strict';
+
+test(t => {
+  const div = createDiv(t);
+  div.style.opacity = '0';
+
+  const animation = div.animate({ all: 'initial' }, 100 * MS_PER_SEC);
+  animation.currentTime = 50 * MS_PER_SEC;
+
+  assert_approx_equals(
+    parseFloat(getComputedStyle(div).opacity),
+    0.5,
+    0.0001,
+    'Should be half way through an opacity animation'
+  );
+}, 'It should be possible to animate the all shorthand');
+
+</script>
+</body>