Skip to content

Commit bd38cd4

Browse files
JoostKthePunderWoman
authored andcommitted
fix(animations): account for Element.animate exceptions (#64506)
In certain scenarios `Element.animate` fails, manifesting in an exception (Firefox) or a null return value (Chromium). The null value is not conform WebIDL but handled gracefully in this commit regardless. Closes #64486 PR Close #64506
1 parent 50e2c54 commit bd38cd4

File tree

2 files changed

+97
-20
lines changed

2 files changed

+97
-20
lines changed

packages/animations/browser/src/render/web_animations/web_animations_player.ts

Lines changed: 43 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,7 @@ export class WebAnimationsPlayer implements AnimationPlayer {
2828
private _originalOnDoneFns: Function[] = [];
2929
private _originalOnStartFns: Function[] = [];
3030

31-
// using non-null assertion because it's re(set) by init();
32-
public readonly domPlayer!: Animation;
31+
public domPlayer: Animation | null = null;
3332
public time = 0;
3433

3534
public parentPlayer: AnimationPlayer | null = null;
@@ -55,34 +54,43 @@ export class WebAnimationsPlayer implements AnimationPlayer {
5554
}
5655

5756
init(): void {
58-
this._buildPlayer();
57+
if (!this._buildPlayer()) {
58+
return;
59+
}
5960
this._preparePlayerBeforeStart();
6061
}
6162

62-
private _buildPlayer(): void {
63-
if (this._initialized) return;
63+
private _buildPlayer(): Animation | null {
64+
if (this._initialized) return this.domPlayer;
6465
this._initialized = true;
6566

6667
const keyframes = this.keyframes;
67-
// @ts-expect-error overwriting a readonly property
68-
this.domPlayer = this._triggerWebAnimation(this.element, keyframes, this.options);
68+
69+
const animation = this._triggerWebAnimation(this.element, keyframes, this.options);
70+
if (!animation) {
71+
this._onFinish();
72+
return null;
73+
}
74+
75+
this.domPlayer = animation;
6976
this._finalKeyframe = keyframes.length ? keyframes[keyframes.length - 1] : new Map();
7077
const onFinish = () => this._onFinish();
71-
this.domPlayer.addEventListener('finish', onFinish);
78+
animation.addEventListener('finish', onFinish);
7279
this.onDestroy(() => {
7380
// We must remove the `finish` event listener once an animation has completed all its
7481
// iterations. This action is necessary to prevent a memory leak since the listener captures
7582
// `this`, creating a closure that prevents `this` from being garbage collected.
76-
this.domPlayer.removeEventListener('finish', onFinish);
83+
animation.removeEventListener('finish', onFinish);
7784
});
85+
return animation;
7886
}
7987

8088
private _preparePlayerBeforeStart() {
8189
// this is required so that the player doesn't start to animate right away
8290
if (this._delay) {
8391
this._resetDomPlayerState();
8492
} else {
85-
this.domPlayer.pause();
93+
this.domPlayer?.pause();
8694
}
8795
}
8896

@@ -99,8 +107,16 @@ export class WebAnimationsPlayer implements AnimationPlayer {
99107
element: HTMLElement,
100108
keyframes: Array<ɵStyleDataMap>,
101109
options: any,
102-
): Animation {
103-
return element.animate(this._convertKeyframesToObject(keyframes), options);
110+
): Animation | null {
111+
const keyframesObject = this._convertKeyframesToObject(keyframes);
112+
113+
// Account for `Element.animate` throwing an exception (Firefox) or returning null (Chromium) in certain
114+
// conditions. See https://github.com/angular/angular/issues/64486
115+
try {
116+
return element.animate(keyframesObject, options);
117+
} catch {
118+
return null;
119+
}
104120
}
105121

106122
onStart(fn: () => void): void {
@@ -118,7 +134,10 @@ export class WebAnimationsPlayer implements AnimationPlayer {
118134
}
119135

120136
play(): void {
121-
this._buildPlayer();
137+
const player = this._buildPlayer();
138+
if (!player) {
139+
return;
140+
}
122141
if (!this.hasStarted()) {
123142
this._onStartFns.forEach((fn) => fn());
124143
this._onStartFns = [];
@@ -127,16 +146,17 @@ export class WebAnimationsPlayer implements AnimationPlayer {
127146
this._specialStyles.start();
128147
}
129148
}
130-
this.domPlayer.play();
149+
player.play();
131150
}
132151

133152
pause(): void {
134153
this.init();
135-
this.domPlayer.pause();
154+
this.domPlayer?.pause();
136155
}
137156

138157
finish(): void {
139158
this.init();
159+
if (!this.domPlayer) return;
140160
if (this._specialStyles) {
141161
this._specialStyles.finish();
142162
}
@@ -154,9 +174,7 @@ export class WebAnimationsPlayer implements AnimationPlayer {
154174
}
155175

156176
private _resetDomPlayerState() {
157-
if (this.domPlayer) {
158-
this.domPlayer.cancel();
159-
}
177+
this.domPlayer?.cancel();
160178
}
161179

162180
restart(): void {
@@ -182,13 +200,18 @@ export class WebAnimationsPlayer implements AnimationPlayer {
182200
}
183201

184202
setPosition(p: number): void {
185-
if (this.domPlayer === undefined) {
203+
if (!this.domPlayer) {
186204
this.init();
187205
}
188-
this.domPlayer.currentTime = p * this.time;
206+
if (this.domPlayer) {
207+
this.domPlayer.currentTime = p * this.time;
208+
}
189209
}
190210

191211
getPosition(): number {
212+
if (!this.domPlayer) {
213+
return this._initialized ? 1 : 0;
214+
}
192215
// tsc is complaining with TS2362 without the conversion to number
193216
return +(this.domPlayer.currentTime ?? 0) / this.time;
194217
}

packages/animations/browser/test/render/web_animations/web_animations_player_spec.ts

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,60 @@ describe('WebAnimationsPlayer tests', () => {
9898
expect(p.currentTime).toEqual(500);
9999
expect(p.log).toEqual(['play']);
100100
});
101+
102+
// https://github.com/angular/angular/issues/64486
103+
it('should gracefully handle animation being null in Chromium-based browsers', () => {
104+
element['animate'] = () => {
105+
return null;
106+
};
107+
const player = new WebAnimationsPlayer(element, [], {duration: 1000});
108+
109+
let started = false;
110+
let done = false;
111+
player.onStart(() => (started = true));
112+
player.onDone(() => (done = true));
113+
114+
player.play();
115+
116+
expect(player.hasStarted()).toBe(false);
117+
expect(started).toBe(false);
118+
expect(done).toBe(true);
119+
120+
player.pause();
121+
player.play();
122+
player.restart();
123+
124+
expect(player.getPosition()).toBe(1);
125+
player.setPosition(0.5);
126+
expect(player.getPosition()).toBe(1);
127+
});
128+
129+
// https://github.com/angular/angular/issues/64486
130+
it('should gracefully handle animation exceptions in Firefox', () => {
131+
element['animate'] = () => {
132+
throw new Error('Simulated error');
133+
};
134+
const player = new WebAnimationsPlayer(element, [], {duration: 1000});
135+
136+
let started = false;
137+
let done = false;
138+
player.onStart(() => (started = true));
139+
player.onDone(() => (done = true));
140+
141+
player.play();
142+
143+
expect(player.hasStarted()).toBe(false);
144+
expect(started).toBe(false);
145+
expect(done).toBe(true);
146+
147+
player.pause();
148+
player.play();
149+
player.restart();
150+
151+
expect(player.getPosition()).toBe(1);
152+
player.setPosition(0.5);
153+
expect(player.getPosition()).toBe(1);
154+
});
101155
});
102156

103157
class MockDomAnimation implements Animation {

0 commit comments

Comments
 (0)