commit 9b8f4e6e9e1f2aa1ea8104b693103caa0895fe34
parent 85d1d9d7cbac461fe8a4c9d4acab577b81319933
Author: Nicolas Chevobbe <nchevobbe@mozilla.com>
Date: Tue, 2 Dec 2025 10:21:47 +0000
Bug 2002399 - [devtools] Remove view transition animations from panel when they're over. r=devtools-reviewers,jdescottes.
There are a couple issues here.
First, DOMWindowUtils.getUnanimatedComputedStyle throws if the node it is called with
isn't connected anymore.
Second, there's some race condition in the animation panel between refreshAnimationsState
and mutations event we might called in the meantime. refreshAnimationsState knows
about this and filters out the animations, checking out if their type is falsy,
but it doesn't seem like we nullify the type of the animation player actor in
the server, so the client still handles (and display) animations it should not.
A test is added to make sure the animation panel is cleared when the view transition ends.
Differential Revision: https://phabricator.services.mozilla.com/D274270
Diffstat:
2 files changed, 58 insertions(+), 10 deletions(-)
diff --git a/devtools/client/inspector/animation/test/browser_animation_pseudo-element.js b/devtools/client/inspector/animation/test/browser_animation_pseudo-element.js
@@ -196,29 +196,60 @@ add_task(async function () {
"Expected view transition pseudo element node was selected"
);
+ info("Select <html> to reset the animation list");
+ await selectNode("html", inspector);
+ // wait for all the animations to be displayed again
+ await waitFor(
+ () =>
+ panel.querySelectorAll(".animation-list .animation-item").length ===
+ TEST_DATA.length
+ );
+
+ info(
+ "Stop the view transition and check that related animations are removed"
+ );
+ const TEST_DATA_WITHOUT_VIEW_TRANSITION = TEST_DATA.filter(
+ ({ expectedTargetLabel }) =>
+ !expectedTargetLabel.startsWith("::view-transition")
+ );
+
+ await SpecialPowers.spawn(gBrowser.selectedBrowser, [], async () => {
+ content.document.activeViewTransition.skipTransition();
+ });
+
+ await waitFor(
+ () =>
+ panel.querySelectorAll(".animation-list .animation-item").length ===
+ TEST_DATA_WITHOUT_VIEW_TRANSITION.length
+ );
+ ok(
+ true,
+ `Got expectedCount of animation item after stopping the view transition`
+ );
+
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
+ TEST_DATA_WITHOUT_VIEW_TRANSITION.length
);
ok(
true,
- `Got expectedCount of animation item should be ${TEST_DATA_AFTER_RELOAD.length} after reloading`
+ `Got expectedCount of animation item should be ${TEST_DATA_WITHOUT_VIEW_TRANSITION.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]);
+ for (let i = 0; i < TEST_DATA_WITHOUT_VIEW_TRANSITION.length; i++) {
+ await checkAnimationItemAtIndex(
+ panel,
+ i,
+ TEST_DATA_WITHOUT_VIEW_TRANSITION[i]
+ );
}
});
diff --git a/devtools/server/actors/animation.js b/devtools/server/actors/animation.js
@@ -88,6 +88,7 @@ class AnimationPlayerActor extends Actor {
this.onAnimationMutation = this.onAnimationMutation.bind(this);
+ this.animationsActor = animationsActor;
this.walker = animationsActor.walker;
this.player = player;
// getting the node might need to traverse the DOM, let's only do this once, when
@@ -120,7 +121,7 @@ class AnimationPlayerActor extends Actor {
if (this.observer && !Cu.isDeadWrapper(this.observer)) {
this.observer.disconnect();
}
- this.player = this.observer = this.walker = null;
+ this.player = this.observer = this.walker = this.animationsActor = null;
super.destroy();
}
@@ -351,7 +352,10 @@ class AnimationPlayerActor extends Actor {
// add the corresponding property in the AnimationPlayerFront' initialState
// getter.
return {
- type: this.getType(),
+ // Don't include the type if the animation was removed (e.g. it isn't handled by the
+ // AnimationsActor anymore). The client filters out animations without type as a
+ // result of its calls to AnimationPlayerFront#refreshState.
+ type: this.animationRemoved ? null : this.getType(),
// startTime is null whenever the animation is paused or waiting to start.
startTime: this.player.startTime,
currentTime: this.player.currentTime,
@@ -464,6 +468,10 @@ class AnimationPlayerActor extends Actor {
}
}
+ onAnimationRemoved() {
+ this.animationRemoved = true;
+ }
+
/**
* Get data about the animated properties of this animation player.
*
@@ -475,6 +483,13 @@ class AnimationPlayerActor extends Actor {
return { name: property.property, values: property.values };
});
+ // If the node isn't connected, the call to DOMWindowUtils.getUnanimatedComputedStyle
+ // below would throw. So early return from here, we'll miss the distance but that
+ // seems fine.
+ if (!this.node?.isConnected) {
+ return properties;
+ }
+
const DOMWindowUtils = this.window.windowUtils;
// Fill missing keyframe with computed value.
@@ -745,6 +760,7 @@ exports.AnimationsActor = class AnimationsActor extends Actor {
type: "removed",
player: this.actors[index],
});
+ this.actors[index].onAnimationRemoved();
this.actors.splice(index, 1);
}
}
@@ -777,6 +793,7 @@ exports.AnimationsActor = class AnimationsActor extends Actor {
type: "removed",
player: this.actors[index],
});
+ this.actors[index].onAnimationRemoved();
this.actors.splice(index, 1);
}
