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
This commit is contained in:
Jessica Janiuk 2025-02-26 16:24:42 -05:00 committed by Miles Malerba
parent 8a73327ba0
commit 7ab0a8d1e7
4 changed files with 143 additions and 9 deletions

View file

@ -57,8 +57,8 @@ export const sharedStashFunction = (rEl: RElement, eventType: string, listenerFn
};
export const sharedMapFunction = (rEl: RElement, jsActionMap: Map<string, Set<Element>>) => {
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<Element>();
if (!blockSet.has(el)) {
blockSet.add(el);

View file

@ -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);
});
}
}

View file

@ -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++;

View file

@ -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: `
<ng-container add-listener>
<button id="click-me">Click me!</button>
</ng-container>`,
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: `
<ng-container add-listener>
<button id="click-me">Click me!</button>
</ng-container>`,
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: `
<ng-container add-listener>
<button id="click-me">Click me!</button>
</ng-container>`,
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,