fix(core): prevents event replay from being called on comment nodes (#60130)

thePunderWoman · mmalerba · commit 7ab0a8d1e719 · 2025-02-28T16:30:27.000Z
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
diff --git 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<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);
diff --git 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
@@ -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
@@ -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,
