From 19409bcb7cd54b25a1d5b300bf36f480ec00252c Mon Sep 17 00:00:00 2001 From: Nicolas Chevobbe Date: Wed, 3 Feb 2016 23:21:44 +0100 Subject: [PATCH] Bug 1232681 - Display script-generated animations correctly. r=pbro MozReview-Commit-ID: 2pk7sxVTHTk --- .../components/animation-time-block.js | 23 ++++++-- .../animationinspector/test/browser.ini | 1 + ...tion_playerWidgets_appear_on_panel_init.js | 38 ++++++++++-- .../test/doc_multiple_animation_types.html | 58 +++++++++++++++++++ .../en-US/animationinspector.properties | 11 ++++ devtools/client/themes/animationinspector.css | 9 +++ devtools/server/actors/animation.js | 26 ++++++--- .../server/tests/unit/test_animation_name.js | 8 +++ .../server/tests/unit/test_animation_type.js | 7 +++ 9 files changed, 163 insertions(+), 18 deletions(-) create mode 100644 devtools/client/animationinspector/test/doc_multiple_animation_types.html diff --git a/devtools/client/animationinspector/components/animation-time-block.js b/devtools/client/animationinspector/components/animation-time-block.js index 8c12b6389e66..e05f6cd21ba0 100644 --- a/devtools/client/animationinspector/components/animation-time-block.js +++ b/devtools/client/animationinspector/components/animation-time-block.js @@ -148,13 +148,24 @@ AnimationTimeBlock.prototype = { /** * Get a formatted title for this animation. This will be either: - * "some-name", "some-name : CSS Transition", or "some-name : CSS Animation", - * depending if the server provides the type, and what type it is. + * "some-name", "some-name : CSS Transition", "some-name : CSS Animation", + * "some-name : Script Animation", or "Script Animation", depending + * if the server provides the type, what type it is and if the animation + * has a name * @param {AnimationPlayerFront} animation */ function getFormattedAnimationTitle({state}) { - // Older servers don't send the type. - return state.type - ? L10N.getFormatStr("timeline." + state.type + ".nameLabel", state.name) - : state.name; + // Older servers don't send a type, and only know about + // CSSAnimations and CSSTransitions, so it's safe to use + // just the name. + if (!state.type) { + return state.name; + } + + // Script-generated animations may not have a name. + if (state.type === "scriptanimation" && !state.name) { + return L10N.getStr("timeline.scriptanimation.unnamedLabel"); + } + + return L10N.getFormatStr(`timeline.${state.type}.nameLabel`, state.name); } diff --git a/devtools/client/animationinspector/test/browser.ini b/devtools/client/animationinspector/test/browser.ini index 418d9fa0e201..d4f02fc3a1f4 100644 --- a/devtools/client/animationinspector/test/browser.ini +++ b/devtools/client/animationinspector/test/browser.ini @@ -8,6 +8,7 @@ support-files = doc_modify_playbackRate.html doc_negative_animation.html doc_simple_animation.html + doc_multiple_animation_types.html head.js [browser_animation_animated_properties_displayed.js] diff --git a/devtools/client/animationinspector/test/browser_animation_playerWidgets_appear_on_panel_init.js b/devtools/client/animationinspector/test/browser_animation_playerWidgets_appear_on_panel_init.js index 01c7251d828f..2c66eb636ead 100644 --- a/devtools/client/animationinspector/test/browser_animation_playerWidgets_appear_on_panel_init.js +++ b/devtools/client/animationinspector/test/browser_animation_playerWidgets_appear_on_panel_init.js @@ -7,11 +7,41 @@ // Test that player widgets are displayed right when the animation panel is // initialized, if the selected node ( by default) is animated. +const { ANIMATION_TYPES } = require("devtools/server/actors/animation"); + add_task(function*() { - yield addTab(TEST_URL_ROOT + "doc_body_animation.html"); + yield new Promise(resolve => { + SpecialPowers.pushPrefEnv({"set": [ + ["dom.animations-api.core.enabled", true] + ]}, resolve); + }); + + yield addTab(TEST_URL_ROOT + "doc_multiple_animation_types.html"); let {panel} = yield openAnimationInspector(); - is(panel.animationsTimelineComponent.animations.length, 1, - "One animation is handled by the timeline after init"); - assertAnimationsDisplayed(panel, 1, "One animation is displayed after init"); + is(panel.animationsTimelineComponent.animations.length, 3, + "Three animations are handled by the timeline after init"); + assertAnimationsDisplayed(panel, 3, + "Three animations are displayed after init"); + is( + panel.animationsTimelineComponent + .animationsEl + .querySelectorAll(`.animation.${ANIMATION_TYPES.SCRIPT_ANIMATION}`) + .length, + 1, + "One script-generated animation is displayed"); + is( + panel.animationsTimelineComponent + .animationsEl + .querySelectorAll(`.animation.${ANIMATION_TYPES.CSS_ANIMATION}`) + .length, + 1, + "One CSS animation is displayed"); + is( + panel.animationsTimelineComponent + .animationsEl + .querySelectorAll(`.animation.${ANIMATION_TYPES.CSS_TRANSITION}`) + .length, + 1, + "One CSS transition is displayed"); }); diff --git a/devtools/client/animationinspector/test/doc_multiple_animation_types.html b/devtools/client/animationinspector/test/doc_multiple_animation_types.html new file mode 100644 index 000000000000..f3606a63bd1c --- /dev/null +++ b/devtools/client/animationinspector/test/doc_multiple_animation_types.html @@ -0,0 +1,58 @@ + + + + + + + +
+
+
+ + + + diff --git a/devtools/client/locales/en-US/animationinspector.properties b/devtools/client/locales/en-US/animationinspector.properties index bf44b7c4df6a..c31b632c5ad1 100644 --- a/devtools/client/locales/en-US/animationinspector.properties +++ b/devtools/client/locales/en-US/animationinspector.properties @@ -108,6 +108,17 @@ timeline.cssanimation.nameLabel=%S - CSS Animation # %S will be replaced by the name of the transition at run-time. timeline.csstransition.nameLabel=%S - CSS Transition +# LOCALIZATION NOTE (timeline.scriptanimation.nameLabel): +# This string is displayed in a tooltip of the animation panel that is shown +# when hovering over the name of a script-generated animation in the timeline UI. +# %S will be replaced by the name of the animation at run-time. +timeline.scriptanimation.nameLabel=%S - Script Animation + +# LOCALIZATION NOTE (timeline.scriptanimation.unnamedLabel): +# This string is displayed in a tooltip of the animation panel that is shown +# when hovering over an unnamed script-generated animation in the timeline UI. +timeline.scriptanimation.unnamedLabel=Script Animation + # LOCALIZATION NOTE (timeline.unknown.nameLabel): # This string is displayed in a tooltip of the animation panel that is shown # when hovering over the name of an unknown animation type in the timeline UI. diff --git a/devtools/client/themes/animationinspector.css b/devtools/client/themes/animationinspector.css index 0ae8f4b12d25..0872dd5e0ead 100644 --- a/devtools/client/themes/animationinspector.css +++ b/devtools/client/themes/animationinspector.css @@ -42,6 +42,11 @@ --timeline-background-color: var(--theme-highlight-blue); } +.animation.scriptanimation { + --timeline-border-color: var(--theme-highlight-green); + --timeline-background-color: var(--theme-graphs-green); +} + html { height: 100%; } @@ -529,6 +534,10 @@ body { background-color: var(--theme-highlight-blue); } +.keyframes.scriptanimation { + background-color: var(--theme-graphs-green); +} + .keyframes .frame { position: absolute; top: 0; diff --git a/devtools/server/actors/animation.js b/devtools/server/actors/animation.js index 47a191033fe8..c95427d1c728 100644 --- a/devtools/server/actors/animation.js +++ b/devtools/server/actors/animation.js @@ -39,6 +39,7 @@ const events = require("sdk/event/core"); const ANIMATION_TYPES = { CSS_ANIMATION: "cssanimation", CSS_TRANSITION: "csstransition", + SCRIPT_ANIMATION: "scriptanimation", UNKNOWN: "unknown" }; exports.ANIMATION_TYPES = ANIMATION_TYPES; @@ -119,19 +120,28 @@ var AnimationPlayerActor = ActorClass({ return data; }, - isAnimation: function(player = this.player) { + isCssAnimation: function(player = this.player) { return player instanceof this.window.CSSAnimation; }, - isTransition: function(player = this.player) { + isCssTransition: function(player = this.player) { return player instanceof this.window.CSSTransition; }, + isScriptAnimation: function(player = this.player) { + return player instanceof this.window.Animation && !( + player instanceof this.window.CSSAnimation || + player instanceof this.window.CSSTransition + ); + }, + getType: function() { - if (this.isAnimation()) { + if (this.isCssAnimation()) { return ANIMATION_TYPES.CSS_ANIMATION; - } else if (this.isTransition()) { + } else if (this.isCssTransition()) { return ANIMATION_TYPES.CSS_TRANSITION; + } else if (this.isScriptAnimation()) { + return ANIMATION_TYPES.SCRIPT_ANIMATION; } return ANIMATION_TYPES.UNKNOWN; @@ -146,9 +156,9 @@ var AnimationPlayerActor = ActorClass({ getName: function() { if (this.player.id) { return this.player.id; - } else if (this.isAnimation()) { + } else if (this.isCssAnimation()) { return this.player.animationName; - } else if (this.isTransition()) { + } else if (this.isCssTransition()) { return this.player.transitionProperty; } @@ -626,9 +636,9 @@ var AnimationsActor = exports.AnimationsActor = ActorClass({ // a "removed" event for the one we already have. let index = this.actors.findIndex(a => { let isSameType = a.player.constructor === player.constructor; - let isSameName = (a.isAnimation() && + let isSameName = (a.isCssAnimation() && a.player.animationName === player.animationName) || - (a.isTransition() && + (a.isCssTransition() && a.player.transitionProperty === player.transitionProperty); let isSameNode = a.player.effect.target === player.effect.target; diff --git a/devtools/server/tests/unit/test_animation_name.js b/devtools/server/tests/unit/test_animation_name.js index 82cf438ee5f8..2f266b6aeff2 100644 --- a/devtools/server/tests/unit/test_animation_name.js +++ b/devtools/server/tests/unit/test_animation_name.js @@ -24,6 +24,9 @@ function run_test() { } }; + window.CSSAnimation.prototype = Object.create(window.Animation.prototype); + window.CSSTransition.prototype = Object.create(window.Animation.prototype); + // Helper to get a mock DOM node. function getMockNode() { return { @@ -46,6 +49,11 @@ function run_test() { animation: new window.Animation(), props: { id: "animation-id" }, expectedName: "animation-id" + }, { + desc: "Animation without an id", + animation: new window.Animation(), + props: {}, + expectedName: "" }, { desc: "CSSTransition with an id", animation: new window.CSSTransition(), diff --git a/devtools/server/tests/unit/test_animation_type.js b/devtools/server/tests/unit/test_animation_type.js index 0d31266c5ffd..e8859a748317 100644 --- a/devtools/server/tests/unit/test_animation_type.js +++ b/devtools/server/tests/unit/test_animation_type.js @@ -24,6 +24,9 @@ function run_test() { } }; + window.CSSAnimation.prototype = Object.create(window.Animation.prototype); + window.CSSTransition.prototype = Object.create(window.Animation.prototype); + // Helper to get a mock DOM node. function getMockNode() { return { @@ -47,6 +50,10 @@ function run_test() { desc: "Test CSSTransition type", animation: new window.CSSTransition(), expectedType: ANIMATION_TYPES.CSS_TRANSITION + }, { + desc: "Test ScriptAnimation type", + animation: new window.Animation(), + expectedType: ANIMATION_TYPES.SCRIPT_ANIMATION }, { desc: "Test unknown type", animation: {effect: {target: getMockNode()}},