From 4bc5b4d93b12f369d9c393f1ef0a2e6e6dc588b0 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Mon, 10 May 2021 06:05:04 +0200 Subject: [PATCH] fix(core): global listeners not being bound on non-node host elements (#42014) We skip event listeners on non-element host nodes (e.g. `ng-container` or `ng-element`), because they don't map to a DOM node so there's nothing to bind the event to. The problem is that this also prevents listeners bound to global targets from being bound. These changes add an extra condition to allow for the event to be bound if it has a custom event target resolver. Fixes #14191. PR Close #42014 --- .../core/src/render3/instructions/listener.ts | 7 +- .../core/test/acceptance/listener_spec.ts | 105 +++++++++++++++++- 2 files changed, 109 insertions(+), 3 deletions(-) diff --git a/packages/core/src/render3/instructions/listener.ts b/packages/core/src/render3/instructions/listener.ts index fb32c2b455..ae31a5ffe8 100644 --- a/packages/core/src/render3/instructions/listener.ts +++ b/packages/core/src/render3/instructions/listener.ts @@ -131,8 +131,11 @@ function listenerInternal( let processOutputs = true; - // add native event listener - applicable to elements only - if (tNode.type & TNodeType.AnyRNode) { + // Adding a native event listener is applicable when: + // - The corresponding TNode represents a DOM element. + // - The event target has a resolver (usually resulting in a global object, + // such as `window` or `document`). + if ((tNode.type & TNodeType.AnyRNode) || eventTargetResolver) { const native = getNativeByTNode(tNode, lView) as RElement; const target = eventTargetResolver ? eventTargetResolver(native) : native; const lCleanupIndex = lCleanup.length; diff --git a/packages/core/test/acceptance/listener_spec.ts b/packages/core/test/acceptance/listener_spec.ts index 671a8baf9b..51bf82f4ff 100644 --- a/packages/core/test/acceptance/listener_spec.ts +++ b/packages/core/test/acceptance/listener_spec.ts @@ -6,7 +6,8 @@ * found in the LICENSE file at https://angular.io/license */ -import {Component, Directive, ErrorHandler, EventEmitter, HostListener, Input, Output, QueryList, ViewChild, ViewChildren} from '@angular/core'; +import {CommonModule} from '@angular/common'; +import {Component, Directive, ErrorHandler, EventEmitter, HostListener, Input, OnInit, Output, QueryList, TemplateRef, ViewChild, ViewChildren, ViewContainerRef} from '@angular/core'; import {TestBed} from '@angular/core/testing'; import {By} from '@angular/platform-browser'; import {onlyInIvy} from '@angular/private/testing'; @@ -397,6 +398,108 @@ describe('event listeners', () => { expect(comp.counter).toBe(1); }); + onlyInIvy('global event listeners on non-node host elements are supported only in Ivy') + .it('should bind global event listeners on an ng-container directive host', () => { + let clicks = 0; + + @Directive({selector: '[add-global-listener]'}) + class AddGlobalListener { + @HostListener('document:click') + handleClick() { + clicks++; + } + } + + @Component({ + template: ` + + + + ` + }) + class MyComp { + } + + TestBed.configureTestingModule({declarations: [MyComp, AddGlobalListener]}); + const fixture = TestBed.createComponent(MyComp); + fixture.detectChanges(); + const button = fixture.nativeElement.querySelector('button'); + button.click(); + fixture.detectChanges(); + expect(clicks).toBe(1); + }); + + onlyInIvy('global event listeners on non-node host elements are supported only in Ivy') + .it('should bind global event listeners on an ng-template directive host', () => { + let clicks = 0; + + @Directive({selector: '[add-global-listener]'}) + class AddGlobalListener { + @HostListener('document:click') + handleClick() { + clicks++; + } + } + + @Component({ + template: ` + + + + + + ` + }) + class MyComp { + } + + TestBed.configureTestingModule( + {declarations: [MyComp, AddGlobalListener], imports: [CommonModule]}); + const fixture = TestBed.createComponent(MyComp); + fixture.detectChanges(); + const button = fixture.nativeElement.querySelector('button'); + button.click(); + fixture.detectChanges(); + expect(clicks).toBe(1); + }); + + onlyInIvy('global event listeners on non-node host elements are supported only in Ivy') + .it('should bind global event listeners on a structural directive host', () => { + let clicks = 0; + + @Directive({selector: '[add-global-listener]'}) + class AddGlobalListener implements OnInit { + @HostListener('document:click') + handleClick() { + clicks++; + } + + constructor(private _vcr: ViewContainerRef, private _templateRef: TemplateRef) {} + + ngOnInit() { + this._vcr.createEmbeddedView(this._templateRef); + } + } + + @Component({ + template: ` +
+ +
+ ` + }) + class MyComp { + } + + TestBed.configureTestingModule({declarations: [MyComp, AddGlobalListener]}); + const fixture = TestBed.createComponent(MyComp); + fixture.detectChanges(); + const button = fixture.nativeElement.querySelector('button'); + button.click(); + fixture.detectChanges(); + expect(clicks).toBe(1); + }); + onlyInIvy('issue has only been resolved for Ivy') .it('should be able to access a property called $event using `this`', () => { let eventVariable: number|undefined;