From 7ab0a8d1e719c9aee862bff0495fcd0138c00a10 Mon Sep 17 00:00:00 2001 From: Jessica Janiuk Date: Wed, 26 Feb 2025 16:24:42 -0500 Subject: [PATCH] fix(core): prevents event replay from being called on comment nodes (#60130) In some rare cases with directives, it is possible that the stash function might be called on a comment node. This actually verifies that the node is an element and exits otherwise. fixes: #60070 PR Close #60130 --- packages/core/src/event_delegation_utils.ts | 2 +- packages/core/src/hydration/event_replay.ts | 12 +- .../core/src/render3/instructions/listener.ts | 6 +- .../platform-server/test/event_replay_spec.ts | 132 +++++++++++++++++- 4 files changed, 143 insertions(+), 9 deletions(-) diff --git a/packages/core/src/event_delegation_utils.ts b/packages/core/src/event_delegation_utils.ts index 9e141081033..ee849db7ddf 100644 --- a/packages/core/src/event_delegation_utils.ts +++ b/packages/core/src/event_delegation_utils.ts @@ -57,8 +57,8 @@ export const sharedStashFunction = (rEl: RElement, eventType: string, listenerFn }; export const sharedMapFunction = (rEl: RElement, jsActionMap: Map>) => { - let blockName = rEl.getAttribute(DEFER_BLOCK_SSR_ID_ATTRIBUTE) ?? ''; const el = rEl as unknown as Element; + let blockName = el.getAttribute(DEFER_BLOCK_SSR_ID_ATTRIBUTE) ?? ''; const blockSet = jsActionMap.get(blockName) ?? new Set(); if (!blockSet.has(el)) { blockSet.add(el); diff --git a/packages/core/src/hydration/event_replay.ts b/packages/core/src/hydration/event_replay.ts index 93dca86a3cf..75d20460777 100644 --- a/packages/core/src/hydration/event_replay.ts +++ b/packages/core/src/hydration/event_replay.ts @@ -23,7 +23,7 @@ import {ENVIRONMENT_INITIALIZER, Injector} from '../di'; import {inject} from '../di/injector_compatibility'; import {Provider} from '../di/interface/provider'; import {setStashFn} from '../render3/instructions/listener'; -import {RElement} from '../render3/interfaces/renderer_dom'; +import {RElement, RNode} from '../render3/interfaces/renderer_dom'; import {CLEANUP, LView, TView} from '../render3/interfaces/view'; import {unwrapRNode} from '../render3/util/view_utils'; @@ -106,9 +106,13 @@ export function withEventReplay(): Provider[] { if (!appsWithEventReplay.has(appRef)) { const jsActionMap = inject(JSACTION_BLOCK_ELEMENT_MAP); if (shouldEnableEventReplay(injector)) { - setStashFn((rEl: RElement, eventName: string, listenerFn: VoidFunction) => { - sharedStashFunction(rEl, eventName, listenerFn); - sharedMapFunction(rEl, jsActionMap); + setStashFn((rEl: RNode, eventName: string, listenerFn: VoidFunction) => { + // If a user binds to a ng-container and uses a directive that binds using a host listener, + // this element could be a comment node. So we need to ensure we have an actual element + // node before stashing anything. + if ((rEl as Node).nodeType !== Node.ELEMENT_NODE) return; + sharedStashFunction(rEl as RElement, eventName, listenerFn); + sharedMapFunction(rEl as RElement, jsActionMap); }); } } diff --git a/packages/core/src/render3/instructions/listener.ts b/packages/core/src/render3/instructions/listener.ts index 7ad1869b548..e057dbc1639 100644 --- a/packages/core/src/render3/instructions/listener.ts +++ b/packages/core/src/render3/instructions/listener.ts @@ -12,7 +12,7 @@ import {NotificationSource} from '../../change_detection/scheduling/zoneless_sch import {assertIndexInRange} from '../../util/assert'; import {TNode, TNodeType} from '../interfaces/node'; import {GlobalTargetResolver, Renderer} from '../interfaces/renderer'; -import {RElement} from '../interfaces/renderer_dom'; +import {RElement, RNode} from '../interfaces/renderer_dom'; import {isComponentHost, isDirectiveHost} from '../interfaces/type_checks'; import {CLEANUP, CONTEXT, LView, RENDERER, TView} from '../interfaces/view'; import {assertTNodeType} from '../node_assert'; @@ -37,7 +37,7 @@ import {DirectiveDef} from '../interfaces/definition'; * an actual implementation when the event replay feature is enabled via * `withEventReplay()` call. */ -let stashEventListener = (el: RElement, eventName: string, listenerFn: (e?: any) => any) => {}; +let stashEventListener = (el: RNode, eventName: string, listenerFn: (e?: any) => any) => {}; export function setStashFn(fn: typeof stashEventListener) { stashEventListener = fn; @@ -218,7 +218,7 @@ export function listenerInternal( processOutputs = false; } else { listenerFn = wrapListener(tNode, lView, context, listenerFn); - stashEventListener(native, eventName, listenerFn); + stashEventListener(target as RElement, eventName, listenerFn); const cleanupFn = renderer.listen(target as RElement, eventName, listenerFn); ngDevMode && ngDevMode.rendererAddEventListener++; diff --git a/packages/platform-server/test/event_replay_spec.ts b/packages/platform-server/test/event_replay_spec.ts index 28ddf2ea09c..aec888abb82 100644 --- a/packages/platform-server/test/event_replay_spec.ts +++ b/packages/platform-server/test/event_replay_spec.ts @@ -6,7 +6,15 @@ * found in the LICENSE file at https://angular.dev/license */ -import {APP_ID, Component, destroyPlatform, ErrorHandler, PLATFORM_ID} from '@angular/core'; +import { + APP_ID, + Component, + destroyPlatform, + Directive, + ErrorHandler, + HostListener, + PLATFORM_ID, +} from '@angular/core'; import {withEventReplay} from '@angular/platform-browser'; import {EventPhase} from '@angular/core/primitives/event-dispatch'; @@ -183,6 +191,128 @@ describe('event replay', () => { expect(outerOnClickSpy).toHaveBeenCalledBefore(innerOnClickSpy); }); + describe('host bindings', () => { + it('should not error when when binding to document:click on a container', async () => { + const clickSpy = jasmine.createSpy(); + @Directive({ + selector: '[add-listener]', + }) + class AddGlobalListener { + @HostListener('document:click') + handleClick = clickSpy; + } + + @Component({ + selector: 'app', + template: ` + + + `, + imports: [AddGlobalListener], + }) + class AppComponent {} + + const appId = 'custom-app-id'; + const providers = [{provide: APP_ID, useValue: appId}]; + const hydrationFeatures = () => [withEventReplay()]; + + const html = await ssr(AppComponent, {envProviders: providers, hydrationFeatures}); + const ssrContents = getAppContents(html); + const doc = getDocument(); + + prepareEnvironment(doc, ssrContents); + resetTViewsFor(AppComponent); + const clickMe = doc.getElementById('click-me')!; + clickMe.click(); + await hydrate(doc, AppComponent, { + envProviders: [{provide: PLATFORM_ID, useValue: 'browser'}, ...providers], + hydrationFeatures, + }); + + expect(clickSpy).not.toHaveBeenCalled(); + }); + + it('should not error when when binding to window:click on a container', async () => { + const clickSpy = jasmine.createSpy(); + @Directive({ + selector: '[add-listener]', + }) + class AddGlobalListener { + @HostListener('window:click') + handleClick = clickSpy; + } + + @Component({ + selector: 'app', + template: ` + + + `, + imports: [AddGlobalListener], + }) + class AppComponent {} + + const appId = 'custom-app-id'; + const providers = [{provide: APP_ID, useValue: appId}]; + const hydrationFeatures = () => [withEventReplay()]; + + const html = await ssr(AppComponent, {envProviders: providers, hydrationFeatures}); + const ssrContents = getAppContents(html); + const doc = getDocument(); + + prepareEnvironment(doc, ssrContents); + resetTViewsFor(AppComponent); + const clickMe = doc.getElementById('click-me')!; + clickMe.click(); + await hydrate(doc, AppComponent, { + envProviders: [{provide: PLATFORM_ID, useValue: 'browser'}, ...providers], + hydrationFeatures, + }); + + expect(clickSpy).not.toHaveBeenCalled(); + }); + + it('should not error when when binding to body:click on a container', async () => { + const clickSpy = jasmine.createSpy(); + @Directive({ + selector: '[add-listener]', + }) + class AddGlobalListener { + @HostListener('body:click') + handleClick = clickSpy; + } + + @Component({ + selector: 'app', + template: ` + + + `, + imports: [AddGlobalListener], + }) + class AppComponent {} + + const appId = 'custom-app-id'; + const providers = [{provide: APP_ID, useValue: appId}]; + const hydrationFeatures = () => [withEventReplay()]; + + const html = await ssr(AppComponent, {envProviders: providers, hydrationFeatures}); + const ssrContents = getAppContents(html); + const doc = getDocument(); + + prepareEnvironment(doc, ssrContents); + resetTViewsFor(AppComponent); + const clickMe = doc.getElementById('click-me')!; + clickMe.click(); + await hydrate(doc, AppComponent, { + envProviders: [{provide: PLATFORM_ID, useValue: 'browser'}, ...providers], + hydrationFeatures, + }); + + expect(clickSpy).not.toHaveBeenCalled(); + }); + }); + it('should remove jsaction attributes, but continue listening to events.', async () => { @Component({ standalone: true,