From d22eb386a5651c43c23746b1b3fe28f27e2f05ea Mon Sep 17 00:00:00 2001 From: SkyZeroZx <73321943+SkyZeroZx@users.noreply.github.com> Date: Tue, 11 Aug 2026 11:54:09 -0500 Subject: [PATCH] refactor(core): distinguish animations that share a name animate.leave waits for the longest-running animation before removing an element. When multiple animations use the same keyframe name, their animationend events have the same animationName, so a shorter animation can be mistaken for the longest one and remove the element too early. Track the longest animation duration returned by getAnimations() and compare it with the duration from event.animation when available. Keep the existing name/property checks as a fallback for older browsers and computed-style detection. Allow a 1ms tolerance for rounding differences between CSSOM and Web Animations and add a regression test using values equivalent to fractional calc() durations. (cherry picked from commit a26fbfa641b965ad581a3919c06d2c51d8b16125) --- .../core/src/animation/longest_animation.ts | 28 +++++--- packages/core/src/animation/utils.ts | 46 ++++++++++--- .../core/test/acceptance/animation_spec.ts | 65 ++++++++++++++++++- .../test/animation/longest_animation_spec.ts | 28 ++++++++ 4 files changed, 148 insertions(+), 19 deletions(-) diff --git a/packages/core/src/animation/longest_animation.ts b/packages/core/src/animation/longest_animation.ts index d2f4c727a13..5776dd29420 100644 --- a/packages/core/src/animation/longest_animation.ts +++ b/packages/core/src/animation/longest_animation.ts @@ -69,6 +69,24 @@ function longestExists(longest: LongestAnimation): boolean { ); } +/** Gets an animation's total duration, including its delay and playback rate. */ +export function getAnimationDuration(animation: Animation): number | undefined { + const timing = animation.effect?.getTiming(); + if (timing === undefined) return undefined; + + // duration can be a string 'auto' or a number. + const animationDuration = typeof timing.duration === 'number' ? timing.duration : 0; + let duration = (timing.delay ?? 0) + animationDuration; + + // Account for playback rate if it is set. + const playbackRate = animation.playbackRate; + if (playbackRate !== undefined && playbackRate !== 0 && playbackRate !== 1) { + duration /= Math.abs(playbackRate); + } + + return duration; +} + /** * Determines the longest animation, but with `getComputedStyles` instead of `getAnimations`. This * is ultimately safer than getAnimations because it can be used when recalculations are in @@ -127,15 +145,7 @@ function determineLongestAnimationFromElementAnimations( if (timing?.iterations === Infinity) { continue; } - // duration can be a string 'auto' or a number. - const animDuration = typeof timing?.duration === 'number' ? timing.duration : 0; - let duration = (timing?.delay ?? 0) + animDuration; - - // Account for playback rate if it is set - const playbackRate = animation.playbackRate; - if (playbackRate !== undefined && playbackRate !== 0 && playbackRate !== 1) { - duration /= Math.abs(playbackRate); - } + const duration = getAnimationDuration(animation) ?? 0; let propertyName: string | undefined; let animationName: string | undefined; diff --git a/packages/core/src/animation/utils.ts b/packages/core/src/animation/utils.ts index 050e3a7791e..90a39b06cf2 100644 --- a/packages/core/src/animation/utils.ts +++ b/packages/core/src/animation/utils.ts @@ -21,8 +21,10 @@ import {RuntimeError, RuntimeErrorCode} from '../errors'; import {Renderer} from '../render3/interfaces/renderer'; import {RElement} from '../render3/interfaces/renderer_dom'; import {TNode} from '../render3/interfaces/node'; +import {getAnimationDuration} from './longest_animation'; const DEFAULT_ANIMATIONS_DISABLED = false; +const ANIMATION_DURATION_TOLERANCE_MS = 1; export const areAnimationSupported = (typeof ngServerMode === 'undefined' || !ngServerMode) && @@ -332,14 +334,42 @@ export function isLongestAnimation( // If we don't have any record of a longest animation, then we shouldn't // block the animationend/transitionend event from doing its work. if (longestAnimation === undefined) return true; - return ( - nativeElement === getEventTarget(event) && - ((longestAnimation.animationName !== undefined && - (event as AnimationEvent).animationName === longestAnimation.animationName) || - (longestAnimation.propertyName !== undefined && - (longestAnimation.propertyName === 'all' || - (event as TransitionEvent).propertyName === longestAnimation.propertyName))) - ); + + if (nativeElement !== getEventTarget(event)) return false; + + // Distinct CSS animations can share a name. Chrome 151 stable exposes their instance: + // https://developer.chrome.com/release-notes/151#animation_accessor_on_animation_and_transition_events + const eventAnimation = ( + event as (AnimationEvent | TransitionEvent) & {readonly animation?: Animation | null} + ).animation; + + // Compare the event animation's duration instead of retaining the Animation object. This also + // disambiguates records obtained from computed styles when getAnimations() was empty. + if (eventAnimation) { + const eventAnimationDuration = getAnimationDuration(eventAnimation); + // CSSOM can round serialized times while Web Animations retains more precision. Only reject an + // event when it is shorter by more than the tolerance so the longest event is not ignored. + if ( + eventAnimationDuration !== undefined && + eventAnimationDuration + ANIMATION_DURATION_TOLERANCE_MS < longestAnimation.duration + ) { + return false; + } + } + + // Fall back to strings for older browsers. + if (longestAnimation.animationName !== undefined) { + return (event as AnimationEvent).animationName === longestAnimation.animationName; + } + + if (longestAnimation.propertyName !== undefined) { + return ( + longestAnimation.propertyName === 'all' || + (event as TransitionEvent).propertyName === longestAnimation.propertyName + ); + } + + return false; } /** diff --git a/packages/core/test/acceptance/animation_spec.ts b/packages/core/test/acceptance/animation_spec.ts index 7751d32c429..f9f3316b660 100644 --- a/packages/core/test/acceptance/animation_spec.ts +++ b/packages/core/test/acceptance/animation_spec.ts @@ -864,6 +864,68 @@ describe('Animation', () => { expect(fixture.debugElement.query(By.css('div'))).toBeNull(); })); + it('should wait for the longest duplicate-named animation when getAnimations is empty', fakeAsync(() => { + const multiple = ` + .duplicate-animation-name { + animation: + duplicate-name 10s linear, + duplicate-name 20s linear; + } + @keyframes duplicate-name { + from { + opacity: 1; + } + to { + opacity: 0; + } + } + `; + @Component({ + changeDetection: ChangeDetectionStrategy.Eager, + selector: 'test-cmp', + styles: multiple, + template: + '@if (show()) {

Element with text

}', + encapsulation: ViewEncapsulation.None, + }) + class TestComponent { + show = signal(true); + } + + TestBed.configureTestingModule({animationsEnabled: true}); + + const fixture = TestBed.createComponent(TestComponent); + const cmp = fixture.componentInstance; + fixture.detectChanges(); + const paragraph = fixture.debugElement.query(By.css('p')); + const getAnimations = paragraph.nativeElement.getAnimations.bind(paragraph.nativeElement); + spyOn(paragraph.nativeElement, 'getAnimations').and.returnValue([]); + + expect(paragraph.nativeElement.className).not.toContain('duplicate-animation-name'); + cmp.show.set(false); + fixture.detectChanges(); + tickAnimationFrames(1); + expect(cmp.show()).toBeFalse(); + fixture.detectChanges(); + expect(paragraph.nativeElement.className).toContain('duplicate-animation-name'); + + const [shortAnimation, longAnimation] = getAnimations(); + const dispatchAnimationEnd = (animation: Animation) => { + const event = new AnimationEvent('animationend', {animationName: 'duplicate-name'}); + Object.defineProperty(event, 'animation', {value: animation}); + paragraph.nativeElement.dispatchEvent(event); + }; + + dispatchAnimationEnd(shortAnimation); + tick(); + expect(fixture.nativeElement.outerHTML).toContain('duplicate-animation-name'); + + dispatchAnimationEnd(longAnimation); + tick(); + expect(fixture.nativeElement.outerHTML).not.toContain('duplicate-animation-name'); + expect(fixture.debugElement.query(By.css('p'))).toBeNull(); + })); + describe('legacy animations compatibility', () => { beforeAll(() => { TestBed.resetTestEnvironment(); @@ -2678,8 +2740,7 @@ describe('Animation', () => { const panels = () => Array.from(fixture.nativeElement.querySelectorAll('.panel')); const panelByText = (text: string) => panels().find((el) => (el as HTMLElement).textContent?.includes(text)) as - | HTMLElement - | undefined; + HTMLElement | undefined; expect(panels().length).toBe(1); expect(panelByText('Panel A')).toBeTruthy(); diff --git a/packages/core/test/animation/longest_animation_spec.ts b/packages/core/test/animation/longest_animation_spec.ts index 76fae451e14..5668a96fb4a 100644 --- a/packages/core/test/animation/longest_animation_spec.ts +++ b/packages/core/test/animation/longest_animation_spec.ts @@ -8,6 +8,7 @@ import {determineLongestAnimation} from '../../src/animation/longest_animation'; import {LongestAnimation} from '../../src/animation/interfaces'; +import {isLongestAnimation, longestAnimations} from '../../src/animation/utils'; import {isNode} from '@angular/private/testing'; describe('determineLongestAnimation', () => { @@ -27,6 +28,33 @@ describe('determineLongestAnimation', () => { expect(el.getAnimations).not.toHaveBeenCalled(); }); + it('should tolerate duration rounding when checking the longest animation event', () => { + const el = document.createElement('div'); + longestAnimations.set(el, { + animationName: 'duplicate-name', + propertyName: undefined, + // Equivalent to calc(20s / 3) + calc(1s / 7) after CSSOM serialization. + duration: 6809.527, + }); + + const createAnimationEndEvent = (duration: number) => { + const event = new AnimationEvent('animationend', {animationName: 'duplicate-name'}); + const animation = { + playbackRate: 1, + effect: { + getTiming: () => ({duration, delay: 1000 / 7}), + }, + } as unknown as Animation; + Object.defineProperty(event, 'animation', {value: animation}); + spyOn(event, 'composedPath').and.returnValue([el]); + return event; + }; + + expect(isLongestAnimation(createAnimationEndEvent(20_000 / 3), el)).toBeTrue(); + expect(isLongestAnimation(createAnimationEndEvent(10_000 / 3), el)).toBeFalse(); + longestAnimations.delete(el); + }); + describe('with getAnimations() support', () => { it('should find the longest animation among multiple animations', () => { const el = document.createElement('div');