From 6bcb31d58b6c8bb064a7dff0c1d2936f942b774b Mon Sep 17 00:00:00 2001 From: arturovt Date: Fri, 10 Apr 2026 22:16:12 +0300 Subject: [PATCH] fix(core): prevent double invocation of load listener on img/iframe during event replay MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When SSR with `withEventReplay()` is used in zoneless mode, Angular registers a native DOM listener on elements like `` via `renderer.listen`, and also stashes the same listener in `__jsaction_fns` for replay. After hydration, `invokeListeners` calls the handler once (the replay), but the browser independently fires a native `load` event on the element as it finishes loading — triggering the listener a second time. The fix introduces a counter map on the element (keyed by event type) that `invokeListeners` increments before calling handlers. The native DOM listener then checks this map via `shouldSkipNativeListener` and skips itself if a replay already handled the event. The check is limited to elements that load external resources on their own (`img`, `iframe`, `script`, `link`, `object`, `embed`, `input`) — bubbling events like `click` on other elements are unaffected. `shouldSkipNativeListener` follows the `enableImpl` pattern: it defaults to `() => false` so terser/esbuild eliminate the branch entirely in apps that don't use event replay. `enableSkipNativeListenerImpl()` is called from the `withEventReplay()` environment initializer to swap in the real implementation. Fixes #59260 --- packages/core/src/event_delegation_utils.ts | 68 ++++++++++++++++++- packages/core/src/hydration/event_replay.ts | 2 + packages/core/src/render3/view/listeners.ts | 8 +++ .../hydration/bundle.golden_symbols.json | 5 ++ .../platform-server/test/event_replay_spec.ts | 34 ++++++++++ 5 files changed, 116 insertions(+), 1 deletion(-) diff --git a/packages/core/src/event_delegation_utils.ts b/packages/core/src/event_delegation_utils.ts index 10cb9b1e943d..62910f2cc9f9 100644 --- a/packages/core/src/event_delegation_utils.ts +++ b/packages/core/src/event_delegation_utils.ts @@ -8,7 +8,7 @@ // tslint:disable:no-duplicate-imports import type {EventContract} from '../primitives/event-dispatch'; -import {Attribute} from '../primitives/event-dispatch'; +import {Attribute, EventPhase} from '../primitives/event-dispatch'; import {APP_ID} from './application/application_tokens'; import {InjectionToken} from './di'; import type {RElement, RNode} from './render3/interfaces/renderer_dom'; @@ -16,9 +16,18 @@ import {INJECTOR, type LView} from './render3/interfaces/view'; export const DEFER_BLOCK_SSR_ID_ATTRIBUTE = 'ngb'; +/** + * A symbol used to track which event types have been replayed on a given element via + * `invokeListeners`. This prevents the native DOM listener (registered by the `listener` + * instruction in templates) from also firing for an event that was already handled by + * the event-replay mechanism, which would cause the handler to be called twice. + */ +const REPLAYED_EVENTS_KEY: unique symbol = /* @__PURE__ */ Symbol('ngReplayedEvents'); + declare global { interface Element { __jsaction_fns: Map | undefined; + [REPLAYED_EVENTS_KEY]?: Map; } } @@ -106,11 +115,68 @@ export function invokeListeners(event: Event, currentTarget: Element | null) { if (!handlerFns || !currentTarget?.isConnected) { return; } + // Track that we're about to replay this event so the native DOM listener can detect + // it was already handled and skip itself. + const replayedEvents = currentTarget[REPLAYED_EVENTS_KEY] ?? new Map(); + replayedEvents.set(event.type, (replayedEvents.get(event.type) ?? 0) + 1); + currentTarget[REPLAYED_EVENTS_KEY] = replayedEvents; for (const handler of handlerFns) { handler(event); } } +// Elements whose resources load independently and can fire `load`/`error` on their own +// after hydration. Other elements (div, span, etc.) only get events from user interaction, +// so they don't need this treatment. +const AUTO_LOAD_ELEMENTS = /^(IMG|IFRAME|SCRIPT|LINK|OBJECT|EMBED|INPUT)$/; + +// Guards against calling enableSkipNativeListenerImpl more than once. +let isSkipNativeListenerImplEnabled = false; + +let _shouldSkipNativeListenerImpl: (event: Event) => boolean = () => false; + +/** + * Returns whether the native DOM listener should be skipped for this event because + * `invokeListeners` already called it during replay. + * + * Defaults to `() => false` so that terser/esbuild can inline the constant and drop + * the branch in apps that don't use event replay. The real implementation is swapped + * in by `enableSkipNativeListenerImpl` when `withEventReplay()` is configured. + */ +export function shouldSkipNativeListener(event: Event): boolean { + return _shouldSkipNativeListenerImpl(event); +} + +/** + * Installs the real `shouldSkipNativeListener` implementation. Called by `withEventReplay()` + * so the logic is only included in bundles that actually need event replay. + */ +export function enableSkipNativeListenerImpl(): void { + if (!isSkipNativeListenerImplEnabled) { + _shouldSkipNativeListenerImpl = (event: Event): boolean => { + // The replay invocation arrives with eventPhase === EventPhase.REPLAY — let it through. + // We only want to suppress the native DOM listener that fires afterwards. + if (event.eventPhase === EventPhase.REPLAY) return false; + + const target = (event.currentTarget ?? event.target) as Element | null; + // Only relevant for elements that load resources on their own. For everything else + // (div, button, etc.) the native listener should always fire normally. + if (!target || !AUTO_LOAD_ELEMENTS.test(target.tagName)) { + return false; + } + + const map = target[REPLAYED_EVENTS_KEY]; + const count = map?.get(event.type); + if (!count) return false; + + count <= 1 ? map!.delete(event.type) : map!.set(event.type, count - 1); + return true; + }; + + isSkipNativeListenerImplEnabled = true; + } +} + /** Shorthand for an event listener callback function to reduce duplication. */ export type EventCallback = (event?: any) => any; diff --git a/packages/core/src/hydration/event_replay.ts b/packages/core/src/hydration/event_replay.ts index 3009ebda0e7a..b13d274d5237 100644 --- a/packages/core/src/hydration/event_replay.ts +++ b/packages/core/src/hydration/event_replay.ts @@ -42,6 +42,7 @@ import { invokeListeners, removeListeners, enableStashEventListenerImpl, + enableSkipNativeListenerImpl, setStashFn, } from '../event_delegation_utils'; import {APP_ID} from '../application/application_tokens'; @@ -108,6 +109,7 @@ export function withEventReplay(): Provider[] { const jsActionMap = inject(JSACTION_BLOCK_ELEMENT_MAP); if (shouldEnableEventReplay(injector)) { enableStashEventListenerImpl(); + enableSkipNativeListenerImpl(); const appId = injector.get(APP_ID); const clearStashFn = setStashFn( appId, diff --git a/packages/core/src/render3/view/listeners.ts b/packages/core/src/render3/view/listeners.ts index c673ec589618..6ad3b0454f13 100644 --- a/packages/core/src/render3/view/listeners.ts +++ b/packages/core/src/render3/view/listeners.ts @@ -28,6 +28,7 @@ import {assertNotSame} from '../../util/assert'; import {handleUncaughtError} from '../instructions/shared'; import { type EventCallback, + shouldSkipNativeListener, stashEventListenerImpl, type WrappedEventCallback, } from '../../event_delegation_utils'; @@ -50,6 +51,13 @@ export function wrapListener( // Note: we are performing most of the work in the listener function itself // to optimize listener registration. return function wrapListenerIn_markDirtyAndPreventDefault(event: any) { + // During event replay, `invokeListeners` calls this function directly. Elements like + // `` may then fire a native `load` event on their own — skip it so the handler + // doesn't run twice. Without event replay, `shouldSkipNativeListener` is a `() => false` + // stub that terser/esbuild inline away, so there's no overhead. + if (shouldSkipNativeListener(event)) { + return false; + } // In order to be backwards compatible with View Engine, events on component host nodes // must also mark the component view itself dirty (i.e. the view that it owns). const startView = isComponentHost(tNode) ? getComponentLViewByIndex(tNode.index, lView) : lView; diff --git a/packages/core/test/bundling/hydration/bundle.golden_symbols.json b/packages/core/test/bundling/hydration/bundle.golden_symbols.json index f72dfa68985e..a905e55f6847 100644 --- a/packages/core/test/bundling/hydration/bundle.golden_symbols.json +++ b/packages/core/test/bundling/hydration/bundle.golden_symbols.json @@ -12,6 +12,7 @@ "APP_ID", "APP_ID_ATTRIBUTE_NAME", "APP_INITIALIZER", + "AUTO_LOAD_ELEMENTS", "ActionResolver", "AfterRenderImpl", "AfterRenderManager", @@ -245,6 +246,7 @@ "REMOVE_STYLES_ON_COMPONENT_DESTROY", "REMOVE_STYLES_ON_COMPONENT_DESTROY_DEFAULT", "RENDERER", + "REPLAYED_EVENTS_KEY", "REQ_URL", "RESPONSE_TYPE", "RendererFactory2", @@ -350,6 +352,7 @@ "_requestIdleCallback", "_retrieveDeferBlockDataImpl", "_retrieveHydrationInfoImpl", + "_shouldSkipNativeListenerImpl", "_stashEventListenerImpl", "_wasLastNodeCreated", "acceptNode", @@ -510,6 +513,7 @@ "enableLocateOrCreateTextNodeImpl", "enableRetrieveDeferBlockDataImpl", "enableRetrieveHydrationInfoImpl", + "enableSkipNativeListenerImpl", "enableStashEventListenerImpl", "enterDI", "enterSkipHydrationBlock", @@ -756,6 +760,7 @@ "isRootView", "isScheduler", "isSchedulerTick", + "isSkipNativeListenerImplEnabled", "isSsrContentsIntegrity", "isStashEventListenerImplEnabled", "isSubscribable", diff --git a/packages/platform-server/test/event_replay_spec.ts b/packages/platform-server/test/event_replay_spec.ts index fcf1ad460947..8c537042d6cf 100644 --- a/packages/platform-server/test/event_replay_spec.ts +++ b/packages/platform-server/test/event_replay_spec.ts @@ -114,6 +114,40 @@ describe('event replay', () => { expect(onClickSpy).toHaveBeenCalled(); }); + it('should replay (load) event exactly once', async () => { + // Regression test: https://github.com/angular/angular/issues/59260 + // withEventReplay() was calling the (load) handler twice — once via + // invokeRegisteredReplayListeners and again via the listener instruction. + const onLoadSpy = jasmine.createSpy('onLoad'); + + @Component({ + selector: 'app', + template: ``, + }) + class AppComponent { + onLoad = onLoadSpy; + } + + const hydrationFeatures = () => [withEventReplay()]; + const html = await ssr(AppComponent, {hydrationFeatures}); + const ssrContents = getAppContents(html); + const doc = getDocument(); + + prepareEnvironment(doc, ssrContents); + resetTViewsFor(AppComponent); + + const img = doc.getElementById('img')!; + // Simulate the image load event firing before Angular hydrates (e.g. cached image). + // `load` does not bubble, but jsaction registers a capture listener on the document + // so it will still be stashed for replay. + img.dispatchEvent(new Event('load')); + + const appRef = await hydrate(doc, AppComponent, {hydrationFeatures}); + appRef.tick(); + + expect(onLoadSpy).toHaveBeenCalledTimes(1); + }); + it('stash event listeners should not conflict when multiple apps are bootstrapped', async () => { const onClickSpy = jasmine.createSpy();