commit 6b3a46d5aaa4f54ce49d970ac25fc290e9a741e3
parent ac33ad716c68ded2818d7886a37a22204feb86aa
Author: Nicolas Chevobbe <nchevobbe@mozilla.com>
Date: Sun, 23 Nov 2025 22:45:23 +0000
Bug 2001126 - [devtools] Fix watching animations for selected node in the Animations panel. r=devtools-reviewers,bomsy.
We used to update the animation front reference in the panel on (top) target available,
and pass the inspector walker to the animation actor.
The issue is that when navigating, the inspector walker is only initialized once
the target is selected, which occurs after the target available callbacks are called.
This also prevented us to be able to retrieve animations for node in iframes.
This patch changes the approach by modifying the existing `update` method, that
is renamed `watchAnimationsForSelectedNode`. It is called when the panel is
initialized, when a new node front is selected, and when the panel is made
visible after being hidden.
At this point, we stop listening for mutations on our previous store animation front,
and if a suitable node is selected, fetches the animation front from the selected
node target, pass the selected node target walker to the animation actor and
retrieves the current animations for the selected node subree.
This makes it much easier to reason about and should always provide what the
panel needs at this point.
A test case is added in browser_animation_pseudo-element.js to make sure the
pseudo elements are displayed after a reload (as the walker is only used for
animations on pseudo elements).
Differential Revision: https://phabricator.services.mozilla.com/D273217
Diffstat:
3 files changed, 164 insertions(+), 57 deletions(-)
diff --git a/devtools/client/fronts/animation.js b/devtools/client/fronts/animation.js
@@ -208,6 +208,16 @@ class AnimationsFront extends FrontClassWithSpec(animationsSpec) {
// Attribute name from which to retrieve the actorID out of the target actor's form
this.formAttributeName = "animationsActor";
}
+
+ setWalkerActor(walkerFront) {
+ this.walker = walkerFront;
+ return super.setWalkerActor(walkerFront);
+ }
+
+ destroy() {
+ super.destroy();
+ this.walker = null;
+ }
}
exports.AnimationsFront = AnimationsFront;
diff --git a/devtools/client/inspector/animation/animation.js b/devtools/client/inspector/animation/animation.js
@@ -62,7 +62,10 @@ class AnimationInspector {
this.simulateAnimationForKeyframesProgressBar =
this.simulateAnimationForKeyframesProgressBar.bind(this);
this.toggleElementPicker = this.toggleElementPicker.bind(this);
- this.update = this.update.bind(this);
+ this.watchAnimationsForSelectedNode =
+ this.watchAnimationsForSelectedNode.bind(this);
+ this.unwatchAnimationsForSelectedNode =
+ this.unwatchAnimationsForSelectedNode.bind(this);
this.onAnimationStateChanged = this.onAnimationStateChanged.bind(this);
this.onAnimationsCurrentTimeUpdated =
this.onAnimationsCurrentTimeUpdated.bind(this);
@@ -71,9 +74,9 @@ class AnimationInspector {
this.onElementPickerStarted = this.onElementPickerStarted.bind(this);
this.onElementPickerStopped = this.onElementPickerStopped.bind(this);
this.onNavigate = this.onNavigate.bind(this);
+ this.onNewNodeFront = this.onNewNodeFront.bind(this);
this.onSidebarResized = this.onSidebarResized.bind(this);
this.onSidebarSelectionChanged = this.onSidebarSelectionChanged.bind(this);
- this.onTargetAvailable = this.onTargetAvailable.bind(this);
EventEmitter.decorate(this);
this.emitForTests = this.emitForTests.bind(this);
@@ -144,13 +147,16 @@ class AnimationInspector {
}
async initListeners() {
- await this.inspector.commands.targetCommand.watchTargets({
- types: [this.inspector.commands.targetCommand.TYPES.FRAME],
- onAvailable: this.onTargetAvailable,
+ await this.watchAnimationsForSelectedNode({
+ // During the initialization of the panel, this.isPanelVisible returns false,
+ // since it's not ready yet.
+ // We need to bypass the check in order to retrieve the animationsFront and fetch
+ // the animations for the selected node.
+ force: true,
});
this.inspector.on("new-root", this.onNavigate);
- this.inspector.selection.on("new-node-front", this.update);
+ this.inspector.selection.on("new-node-front", this.onNewNodeFront);
this.inspector.sidebar.on("select", this.onSidebarSelectionChanged);
this.inspector.toolbox.on("select", this.onSidebarSelectionChanged);
this.inspector.toolbox.on(
@@ -169,8 +175,11 @@ class AnimationInspector {
destroy() {
this.setAnimationStateChangedListenerEnabled(false);
- this.inspector.off("new-root", this.onNavigate);
- this.inspector.selection.off("new-node-front", this.update);
+ this.inspector.off("new-root", this.onNewNodeFront);
+ this.inspector.selection.off(
+ "new-node-front",
+ this.watchAnimationsForSelectedNode
+ );
this.inspector.sidebar.off("select", this.onSidebarSelectionChanged);
this.inspector.toolbox.off(
"inspector-sidebar-resized",
@@ -225,6 +234,12 @@ class AnimationInspector {
* @param {number} currentTime
*/
async doSetCurrentTimes(currentTime) {
+ // If we don't have an animationsFront, it means that we don't have visible animations
+ // so we can safely bail here.
+ if (!this.animationsFront) {
+ return;
+ }
+
const { animations, timeScale } = this.state;
currentTime = currentTime + timeScale.minStartTime;
await this.animationsFront.setCurrentTimes(animations, currentTime, true, {
@@ -300,7 +315,11 @@ class AnimationInspector {
return Promise.reject("Animation inspector already destroyed");
}
- return this.inspector.walker.getNodeFromActor(actorID, ["node"]);
+ if (!this.animationsFront?.walker) {
+ return Promise.reject("No animations front walker");
+ }
+
+ return this.animationsFront.walker.getNodeFromActor(actorID, ["node"]);
}
isPanelVisible() {
@@ -415,11 +434,11 @@ class AnimationInspector {
this.wasPanelVisibled = isPanelVisibled;
if (this.isPanelVisible()) {
- await this.update();
+ await this.watchAnimationsForSelectedNode();
this.onSidebarResized(null, this.inspector.getSidebarSize());
} else {
+ await this.unwatchAnimationsForSelectedNode();
this.stopAnimationsCurrentTimeTimer();
- this.setAnimationStateChangedListenerEnabled(false);
}
}
@@ -431,16 +450,6 @@ class AnimationInspector {
this.inspector.store.dispatch(updateSidebarSize(size));
}
- async onTargetAvailable({ targetFront }) {
- if (targetFront.isTopLevel) {
- this.animationsFront = await targetFront.getFront("animations");
- this.animationsFront.setWalkerActor(this.inspector.walker);
- this.animationsFront.on("mutations", this.onAnimationsMutation);
-
- await this.update();
- }
- }
-
removeAnimationsCurrentTimeListener(listener) {
this.animationsCurrentTimeListeners =
this.animationsCurrentTimeListeners.filter(l => l !== listener);
@@ -498,6 +507,12 @@ class AnimationInspector {
return; // Already destroyed or another node selected.
}
+ // If we don't have an animationsFront, it means that we don't have visible animations
+ // so we can safely bail here.
+ if (!this.animationsFront) {
+ return;
+ }
+
let animations = this.state.animations;
// "changed" event on each animation will fire respectively when the playback
// rate changed. Since for each occurrence of event, change of UI is urged.
@@ -525,6 +540,12 @@ class AnimationInspector {
return; // Already destroyed or another node selected.
}
+ // If we don't have an animationsFront, it means that we don't have visible animations
+ // so we can safely bail here.
+ if (!this.animationsFront) {
+ return;
+ }
+
let { animations, timeScale } = this.state;
try {
@@ -720,26 +741,74 @@ class AnimationInspector {
this.inspector.toolbox.nodePicker.togglePicker();
}
- async update() {
- if (!this.isPanelVisible()) {
+ onNewNodeFront() {
+ this.watchAnimationsForSelectedNode();
+ }
+
+ /**
+ * Retrieve animations for the inspector selected node (and its subtree), add an event
+ * listener for animations on the node (and its subtree) and update the panel.
+ * If the panel is not visible (and `force` is not `true`), the panel won't be updated,
+ * and this will remove the previous listener.
+ *
+ * @param {object} options
+ * @param {boolean} options.force: Set to true to force updating the panel, even if
+ * it is not visible.
+ */
+ async watchAnimationsForSelectedNode({ force = false } = {}) {
+ this.unwatchAnimationsForSelectedNode();
+
+ if (!this.isPanelVisible() && !force) {
return;
}
const done = this.inspector.updating("animationinspector");
-
const selection = this.inspector.selection;
- const animations =
- selection.isConnected() && selection.isElementNode()
- ? await this.animationsFront.getAnimationPlayersForNode(
- selection.nodeFront
- )
- : [];
- this.fireUpdateAction(animations);
+
+ let animations;
+ const shouldWatchAnimationForSelectedNode =
+ selection && selection.isConnected() && selection.isElementNode();
+ if (shouldWatchAnimationForSelectedNode) {
+ // Since the panel only displays the animations for the selected node and its subtree,
+ // we can get the animation front from the selected node target, so we can handle
+ // animations in iframe for example
+ this.animationsFront =
+ await selection.nodeFront.targetFront.getFront("animations");
+ // At this point, we have a selected node, so the target should have an inspector
+ // and its walker, that we can pass to the animation front
+ this.animationsFront.setWalkerActor(
+ selection.nodeFront.inspectorFront.walker
+ );
+ // Then we can listen for future animations on the subtree
+ this.animationsFront.on("mutations", this.onAnimationsMutation);
+ // and directly retrieve the existing one, if there are some
+ animations = await this.animationsFront.getAnimationPlayersForNode(
+ selection.nodeFront
+ );
+ }
+
+ this.fireUpdateAction(animations || []);
this.setAnimationStateChangedListenerEnabled(true);
done();
}
+ /**
+ * Nullify animationFront, remove the listener that might have been set on it, as well
+ * as listeners on AnimationPlayer fronts.
+ *
+ * @param {object} options
+ * @param {boolean} options.force: Set to true to force updating the panel, even if
+ * it is not visible.
+ */
+ unwatchAnimationsForSelectedNode() {
+ if (this.animationsFront) {
+ this.animationsFront.off("mutations", this.onAnimationsMutation);
+ this.animationsFront = null;
+ }
+ this.setAnimationStateChangedListenerEnabled(false);
+ }
+
async refreshAnimationsState(animations) {
let error = null;
diff --git a/devtools/client/inspector/animation/test/browser_animation_pseudo-element.js b/devtools/client/inspector/animation/test/browser_animation_pseudo-element.js
@@ -110,32 +110,7 @@ add_task(async function () {
info("Checking content of each animation item");
for (let i = 0; i < TEST_DATA.length; i++) {
- const testData = TEST_DATA[i];
- info(`Checking pseudo element for animation at index #${i}`);
- const animationItemEl = await findAnimationItemByIndex(panel, i);
-
- if (!animationItemEl) {
- ok(false, `Didn't find an animation at index #${i}`);
- continue;
- }
-
- info("Checking text content of animation target");
- const animationTargetEl = animationItemEl.querySelector(
- ".animation-list .animation-item .animation-target"
- );
- is(
- animationTargetEl.textContent,
- testData.expectedTargetLabel,
- `Got expected target for animation at index #${i}`
- );
-
- info("Checking text content of animation name");
- const animationNameEl = animationItemEl.querySelector(".animation-name");
- is(
- animationNameEl.textContent,
- testData.expectedAnimationNameLabel,
- `Got expected animation name for animation at index #${i}`
- );
+ await checkAnimationItemAtIndex(panel, i, TEST_DATA[i]);
}
info(
@@ -220,8 +195,61 @@ add_task(async function () {
"::view-transition-group(my-vt)",
"Expected view transition pseudo element node was selected"
);
+
+ info("Reload the page");
+ await reloadBrowser();
+
+ info("Waiting for expected animations to be displayed");
+ // No need to check for view transition this time, we want to assert that we do get
+ // animations for pseudo element after reloading (assert fix for Bug 2001126)
+ const TEST_DATA_AFTER_RELOAD = TEST_DATA.filter(
+ ({ expectedTargetLabel }) =>
+ !expectedTargetLabel.startsWith("::view-transition")
+ );
+ await waitFor(
+ () =>
+ panel.querySelectorAll(".animation-list .animation-item").length ===
+ TEST_DATA_AFTER_RELOAD.length
+ );
+ ok(
+ true,
+ `Got expectedCount of animation item should be ${TEST_DATA_AFTER_RELOAD.length} after reloading`
+ );
+
+ info("Checking content of each animation item after reload");
+ for (let i = 0; i < TEST_DATA_AFTER_RELOAD.length; i++) {
+ await checkAnimationItemAtIndex(panel, i, TEST_DATA_AFTER_RELOAD[i]);
+ }
});
+async function checkAnimationItemAtIndex(panel, index, testData) {
+ info(`Checking pseudo element for animation at index #${index}`);
+ const animationItemEl = await findAnimationItemByIndex(panel, index);
+
+ if (!animationItemEl) {
+ ok(false, `Didn't find an animation at index #${index}`);
+ return;
+ }
+
+ info("Checking text content of animation target");
+ const animationTargetEl = animationItemEl.querySelector(
+ ".animation-list .animation-item .animation-target"
+ );
+ is(
+ animationTargetEl.textContent,
+ testData.expectedTargetLabel,
+ `Got expected target for animation at index #${index}`
+ );
+
+ info("Checking text content of animation name");
+ const animationNameEl = animationItemEl.querySelector(".animation-name");
+ is(
+ animationNameEl.textContent,
+ testData.expectedAnimationNameLabel,
+ `Got expected animation name for animation at index #${index}`
+ );
+}
+
function assertAnimationCount(panel, expectedCount) {
info("Checking count of animation item");
is(
