From 4340a63c52fca4d08ef258a3c053f50932eab27e Mon Sep 17 00:00:00 2001 From: Jaime Burgos <73321943+SkyZeroZx@users.noreply.github.com> Date: Wed, 2 Sep 2026 18:04:41 -0500 Subject: [PATCH] fix(router): avoid view transitions when the user agent provides one Preserve hasUAVisualTransition through Location and the Router navigation pipeline. This prevents withViewTransitions from starting an author transition after the browser has already performed one, including across redirects and when using experimental platform navigation. (cherry picked from commit af26a8c52178a0e972c2841051db44f9d04ac371) --- goldens/public-api/common/index.api.md | 2 + integration/legacy-animations/size.json | 2 +- packages/common/src/location/location.ts | 10 ++++- .../common/src/location/platform_location.ts | 2 + .../common/test/location/location_spec.ts | 28 ++++++++++++++ .../dom-navigation/src/navigation_types.ts | 2 + .../dom-navigation/testing/fake_navigation.ts | 17 ++++++++- .../test/fake_platform_navigation.spec.ts | 14 +++++++ packages/router/src/navigation_transition.ts | 3 ++ packages/router/src/router.ts | 38 ++++++++++++------- .../statemanager/navigation_state_manager.ts | 11 +++++- .../router/src/statemanager/state_manager.ts | 14 +++++-- packages/router/src/utils/view_transition.ts | 16 ++++++-- 13 files changed, 134 insertions(+), 25 deletions(-) diff --git a/goldens/public-api/common/index.api.md b/goldens/public-api/common/index.api.md index f589acbe28b..a92ffcc7c2f 100644 --- a/goldens/public-api/common/index.api.md +++ b/goldens/public-api/common/index.api.md @@ -429,6 +429,7 @@ export const LOCATION_INITIALIZED: InjectionToken>; // @public export interface LocationChangeEvent { + hasUAVisualTransition?: boolean; // (undocumented) state: any; // (undocumented) @@ -929,6 +930,7 @@ export enum Plural { // @public (undocumented) interface PopStateEvent_2 { + hasUAVisualTransition?: boolean; // (undocumented) pop?: boolean; // (undocumented) diff --git a/integration/legacy-animations/size.json b/integration/legacy-animations/size.json index f7b794aa90e..031faa5e286 100644 --- a/integration/legacy-animations/size.json +++ b/integration/legacy-animations/size.json @@ -1,5 +1,5 @@ { - "dist/main.js": 153915, + "dist/main.js": 158918, "dist/polyfills.js": 35726, "dist/open-close.component-[hash].js": 1190 } diff --git a/packages/common/src/location/location.ts b/packages/common/src/location/location.ts index b4636ce1208..e049ba62e54 100644 --- a/packages/common/src/location/location.ts +++ b/packages/common/src/location/location.ts @@ -18,6 +18,8 @@ export interface PopStateEvent { state?: any; type?: string; url?: string; + /** Whether the user agent performed a visual transition for this navigation. */ + hasUAVisualTransition?: boolean; } /** @@ -75,12 +77,16 @@ export class Location implements OnDestroy { // https://developer.mozilla.org/en-US/docs/Web/API/URL/URL#parameters this._basePath = _stripOrigin(stripTrailingSlash(_stripIndexHtml(baseHref))); this._locationStrategy.onPopState((ev) => { - this._subject.next({ + const popStateEvent: PopStateEvent = { 'url': this.path(true), 'pop': true, 'state': ev.state, 'type': ev.type, - }); + }; + if (ev.hasUAVisualTransition) { + popStateEvent.hasUAVisualTransition = true; + } + this._subject.next(popStateEvent); }); } diff --git a/packages/common/src/location/platform_location.ts b/packages/common/src/location/platform_location.ts index 649b403b66c..60dea5a3550 100644 --- a/packages/common/src/location/platform_location.ts +++ b/packages/common/src/location/platform_location.ts @@ -85,6 +85,8 @@ export const LOCATION_INITIALIZED = new InjectionToken>( export interface LocationChangeEvent { type: string; state: any; + /** Whether the user agent performed a visual transition for this navigation. */ + hasUAVisualTransition?: boolean; } /** diff --git a/packages/common/test/location/location_spec.ts b/packages/common/test/location/location_spec.ts index b3a4e312824..08b7d7144f3 100644 --- a/packages/common/test/location/location_spec.ts +++ b/packages/common/test/location/location_spec.ts @@ -219,6 +219,34 @@ describe('Location Class', () => { expect(notificationCount).toBe(1); }); + + it('should preserve whether the user agent performed a visual transition', () => { + let hasUAVisualTransition: boolean | undefined; + location.subscribe((event) => { + hasUAVisualTransition = event.hasUAVisualTransition; + }); + + locationStrategy.internalPath = '/test'; + (locationStrategy as any)._subject.next({ + newUrl: '/test', + pop: true, + type: 'popstate', + hasUAVisualTransition: true, + } as any); + + expect(hasUAVisualTransition).toBeTrue(); + }); + + it('should not add a UA visual transition when the platform event does not provide one', () => { + let hasOwnUAVisualTransition: boolean | undefined; + location.subscribe((event) => { + hasOwnUAVisualTransition = Object.hasOwn(event, 'hasUAVisualTransition'); + }); + + locationStrategy.simulatePopState('/test'); + + expect(hasOwnUAVisualTransition).toBeFalse(); + }); }); describe('location.normalize(url) should return only route', () => { diff --git a/packages/core/primitives/dom-navigation/src/navigation_types.ts b/packages/core/primitives/dom-navigation/src/navigation_types.ts index 8ff38ff84cc..817000e5de7 100644 --- a/packages/core/primitives/dom-navigation/src/navigation_types.ts +++ b/packages/core/primitives/dom-navigation/src/navigation_types.ts @@ -146,6 +146,7 @@ export declare class NavigateEvent extends Event { readonly canIntercept: boolean; readonly userInitiated: boolean; readonly hashChange: boolean; + readonly hasUAVisualTransition: boolean; readonly destination: NavigationDestination; readonly signal: AbortSignal; readonly formData: FormData | null; @@ -161,6 +162,7 @@ export interface NavigateEventInit extends EventInit { canIntercept?: boolean; userInitiated?: boolean; hashChange?: boolean; + hasUAVisualTransition?: boolean; destination: NavigationDestination; signal: AbortSignal; formData?: FormData | null; diff --git a/packages/core/primitives/dom-navigation/testing/fake_navigation.ts b/packages/core/primitives/dom-navigation/testing/fake_navigation.ts index d7d7276ed9d..31fe23c12d0 100644 --- a/packages/core/primitives/dom-navigation/testing/fake_navigation.ts +++ b/packages/core/primitives/dom-navigation/testing/fake_navigation.ts @@ -540,6 +540,7 @@ export class FakeNavigation implements Navigation { // Happens as part of "updating the document" steps https://whatpr.org/html/10919/browsing-the-web.html#updating-the-document const popStateEvent = createPopStateEvent({ state: navigateEvent.destination.getHistoryState(), + hasUAVisualTransition: navigateEvent.hasUAVisualTransition, }); this._window.dispatchEvent(popStateEvent); if (navigateEvent.hashChange) { @@ -810,6 +811,7 @@ function dispatchNavigateEvent({ canIntercept, userInitiated, hashChange, + hasUAVisualTransition, navigationType, destination, info, @@ -820,6 +822,7 @@ function dispatchNavigateEvent({ canIntercept: boolean; userInitiated: boolean; hashChange: boolean; + hasUAVisualTransition?: boolean; navigationType: NavigationType; destination: FakeNavigationDestination; info: unknown; @@ -838,6 +841,9 @@ function dispatchNavigateEvent({ event.canIntercept = canIntercept; event.userInitiated = userInitiated; event.hashChange = hashChange; + if (hasUAVisualTransition) { + event.hasUAVisualTransition = true; + } event.signal = eventAbortController.signal; event.abortController = eventAbortController; event.info = info; @@ -1162,12 +1168,21 @@ function createFakeNavigationCurrentEntryChangeEvent({ * Create a fake equivalent of `PopStateEvent`. This does not use a class * because ES5 transpiled JavaScript cannot extend native Event. */ -function createPopStateEvent({state}: {state: unknown}) { +function createPopStateEvent({ + state, + hasUAVisualTransition, +}: { + state: unknown; + hasUAVisualTransition?: boolean; +}) { const event = new Event('popstate', { bubbles: false, cancelable: false, }) as {-readonly [P in keyof PopStateEvent]: PopStateEvent[P]}; event.state = state; + if (hasUAVisualTransition) { + event.hasUAVisualTransition = true; + } return event as PopStateEvent; } diff --git a/packages/core/primitives/dom-navigation/testing/test/fake_platform_navigation.spec.ts b/packages/core/primitives/dom-navigation/testing/test/fake_platform_navigation.spec.ts index b41b8557e29..bbe76b0d4f6 100644 --- a/packages/core/primitives/dom-navigation/testing/test/fake_platform_navigation.spec.ts +++ b/packages/core/primitives/dom-navigation/testing/test/fake_platform_navigation.spec.ts @@ -169,6 +169,7 @@ describe('navigation', () => { }), }), ); + expect(Object.hasOwn(navigateEvent, 'hasUAVisualTransition')).toBeFalse(); expect(navigateEvent.destination.getState()).toBeUndefined(); const committedEntry = await committed; expect(committedEntry).toEqual( @@ -958,6 +959,7 @@ describe('navigation', () => { expect(locals.popStateEvents.length).toBe(1); const popStateEvent = locals.popStateEvents[0]; expect(popStateEvent.state).toBeNull(); + expect(Object.hasOwn(popStateEvent, 'hasUAVisualTransition')).toBeFalse(); expect(locals.navigation.canGoBack).toBeTrue(); expect(locals.navigation.canGoForward).toBeTrue(); const finishedEntry = await finished; @@ -968,6 +970,18 @@ describe('navigation', () => { expect(locals.popStateEvents.length).toBe(1); }); + it('propagates a UA visual transition to the popstate event', async () => { + await setUpEntries(); + locals.setExtraNavigateCallback((event) => { + Object.defineProperty(event, 'hasUAVisualTransition', {value: true}); + }); + + await locals.navigation.back().finished; + + expect(locals.navigateEvents[0].hasUAVisualTransition).toBeTrue(); + expect(locals.popStateEvents[0].hasUAVisualTransition).toBeTrue(); + }); + it('traverses forward', async () => { expect(locals.navigation.canGoBack).toBeFalse(); expect(locals.navigation.canGoForward).toBeFalse(); diff --git a/packages/router/src/navigation_transition.ts b/packages/router/src/navigation_transition.ts index e6bd2fc5ad3..12beb119c93 100644 --- a/packages/router/src/navigation_transition.ts +++ b/packages/router/src/navigation_transition.ts @@ -318,6 +318,7 @@ export interface NavigationTransition { urlAfterRedirects?: UrlTree; rawUrl: UrlTree; extras: NavigationExtras; + hasUAVisualTransition?: boolean; resolve: (value: boolean | PromiseLike) => void; reject: (reason?: any) => void; promise: Promise; @@ -414,6 +415,7 @@ export class NavigationTransitions { | 'currentRawUrl' | 'rawUrl' | 'extras' + | 'hasUAVisualTransition' | 'resolve' | 'reject' | 'promise' @@ -775,6 +777,7 @@ export class NavigationTransitions { this.environmentInjector, currentSnapshot.root, targetSnapshot!.root, + overallTransitionState.hasUAVisualTransition, ); // If view transitions are enabled, block the navigation until the view diff --git a/packages/router/src/router.ts b/packages/router/src/router.ts index 29556665890..760f41a0e8f 100644 --- a/packages/router/src/router.ts +++ b/packages/router/src/router.ts @@ -233,11 +233,18 @@ export class Router { ...opts, }; - this.scheduleNavigation(mergedTree, IMPERATIVE_NAVIGATION, null, extras, { - resolve: currentTransition.resolve, - reject: currentTransition.reject, - promise: currentTransition.promise, - }); + this.scheduleNavigation( + mergedTree, + IMPERATIVE_NAVIGATION, + null, + extras, + currentTransition.hasUAVisualTransition, + { + resolve: currentTransition.resolve, + reject: currentTransition.reject, + promise: currentTransition.promise, + }, + ); } } @@ -288,8 +295,8 @@ export class Router { // run into ngZone this.nonRouterCurrentEntryChangeSubscription ??= this.stateManager.registerNonRouterCurrentEntryChangeListener( - (url, state, source, extras) => { - this.navigateToSyncWithBrowser(url, source, state, extras); + (url, state, source, extras, hasUAVisualTransition) => { + this.navigateToSyncWithBrowser(url, source, state, extras, hasUAVisualTransition); }, ); } @@ -306,6 +313,7 @@ export class Router { source: NavigationTrigger, state: RestoredState | null | undefined, extras: NavigationExtras, + hasUAVisualTransition?: boolean, ) { // TODO: restoredState should always include the entire state, regardless // of navigationId. This requires a breaking change to update the type on @@ -338,12 +346,14 @@ export class Router { } const urlTree = this.parseUrl(routerUrl); - this.scheduleNavigation(urlTree, source, restoredState, extras).catch((e) => { - if (this.disposed) { - return; - } - this.injector.get(ɵINTERNAL_APPLICATION_ERROR_HANDLER)(e); - }); + this.scheduleNavigation(urlTree, source, restoredState, extras, hasUAVisualTransition).catch( + (e) => { + if (this.disposed) { + return; + } + this.injector.get(ɵINTERNAL_APPLICATION_ERROR_HANDLER)(e); + }, + ); } /** The current URL. */ @@ -657,6 +667,7 @@ export class Router { source: NavigationTrigger, restoredState: RestoredState | null, extras: NavigationExtras, + hasUAVisualTransition?: boolean, priorPromise?: { resolve: (result: boolean | PromiseLike) => void; reject: (reason?: any) => void; @@ -696,6 +707,7 @@ export class Router { currentRawUrl: this.currentUrlTree, rawUrl, extras, + hasUAVisualTransition, resolve: resolve!, reject: reject!, promise, diff --git a/packages/router/src/statemanager/navigation_state_manager.ts b/packages/router/src/statemanager/navigation_state_manager.ts index 990e9d96ff5..0139727048c 100644 --- a/packages/router/src/statemanager/navigation_state_manager.ts +++ b/packages/router/src/statemanager/navigation_state_manager.ts @@ -101,6 +101,7 @@ export class NavigationStateManager extends StateManager { private nonRouterCurrentEntryChangeSubject = new Subject<{ path: string; state: RestoredState | null | undefined; + hasUAVisualTransition?: boolean; }>(); nonRouterEntryChangeListener?: SubscriptionLike; @@ -130,16 +131,18 @@ export class NavigationStateManager extends StateManager { state: RestoredState | null | undefined, trigger: NavigationTrigger, extras: NavigationExtras, + hasUAVisualTransition?: boolean, ) => void, ): SubscriptionLike { this.activeHistoryEntry = this.navigation.currentEntry!; this.nonRouterEntryChangeListener = this.nonRouterCurrentEntryChangeSubject.subscribe( - ({path, state}) => { + ({path, state, hasUAVisualTransition}) => { listener( path, state, 'popstate', !this.precommitHandlerSupported ? {replaceUrl: true} : {}, + hasUAVisualTransition, ); }, ); @@ -538,7 +541,11 @@ export class NavigationStateManager extends StateManager { // The url will always start with the appRootUrl because of the boundary check in handleNavigate. const path = event.destination.url.substring(this.appRootUrl.href.length - 1); const state = event.destination.getState() as RestoredState | null | undefined; - this.nonRouterCurrentEntryChangeSubject.next({path, state}); + this.nonRouterCurrentEntryChangeSubject.next({ + path, + state, + hasUAVisualTransition: event.hasUAVisualTransition, + }); } private eventAndRouterDestinationsMatch( diff --git a/packages/router/src/statemanager/state_manager.ts b/packages/router/src/statemanager/state_manager.ts index afecaedae0e..89b83c7cc82 100644 --- a/packages/router/src/statemanager/state_manager.ts +++ b/packages/router/src/statemanager/state_manager.ts @@ -153,6 +153,7 @@ export abstract class StateManager { state: RestoredState | null | undefined, trigger: NavigationTrigger, extras: NavigationExtras, + hasUAVisualTransition?: boolean, ) => void, ): SubscriptionLike; @@ -194,6 +195,7 @@ export class HistoryStateManager extends StateManager { state: RestoredState | null | undefined, trigger: NavigationTrigger, extras: NavigationExtras, + hasUAVisualTransition?: boolean, ) => void, ): SubscriptionLike { return this.location.subscribe((event) => { @@ -201,9 +203,15 @@ export class HistoryStateManager extends StateManager { // The `setTimeout` was added in #12160 and is likely to support Angular/AngularJS // hybrid apps. setTimeout(() => { - listener(event['url']!, event.state as RestoredState | null | undefined, 'popstate', { - replaceUrl: true, - }); + listener( + event['url']!, + event.state as RestoredState | null | undefined, + 'popstate', + { + replaceUrl: true, + }, + event.hasUAVisualTransition, + ); }); } }); diff --git a/packages/router/src/utils/view_transition.ts b/packages/router/src/utils/view_transition.ts index 10377800903..259fc19aed5 100644 --- a/packages/router/src/utils/view_transition.ts +++ b/packages/router/src/utils/view_transition.ts @@ -63,17 +63,27 @@ export interface ViewTransitionInfo { /** * A helper function for using browser view transitions. This function skips the call to - * `startViewTransition` if the browser does not support it. + * `startViewTransition` if the browser does not support it or has already provided a transition. * - * @returns A Promise that resolves when the view transition callback begins. + * @returns A Promise that resolves when the view transition callback begins, or `undefined` when + * the user agent has already provided a transition and the navigation should continue immediately. */ export function createViewTransition( injector: Injector, from: ActivatedRouteSnapshot, to: ActivatedRouteSnapshot, -): Promise { + hasUAVisualTransition?: boolean, +): Promise | undefined { const transitionOptions = injector.get(VIEW_TRANSITION_OPTIONS); const document = injector.get(DOCUMENT); + + if (hasUAVisualTransition) { + transitionOptions.skipNextTransition = false; + // The browser has already started presenting the navigation. Continuing synchronously gives it + // the earliest opportunity to display the post-navigation DOM. + return; + } + if (!document.startViewTransition || transitionOptions.skipNextTransition) { transitionOptions.skipNextTransition = false; // The timing of `startViewTransition` is closer to a macrotask. It won't be called