fix(ivy): ensure eventListeners added outside angular context are not called... (angular#34514)

atscott · AndrewKushnir · commit 32b72f39f061 · 2020-01-22T15:38:13.000-08:00
by DebugElement.triggerEventHandler. ZoneJS tracks the eventListeners on a node but we need to be able to differentiate between those added by Angular and those that were added outside the Angular context. This fix aligns with the behavior that was present in View Engine (not calling those listeners). If we decide later that we want to call those listeners, we still need a way to differentiate between those that we have wrapped in dom_renderer and those that were not (because they were added outside the Angular context). PR Close angular#34514
diff --git a/packages/core/src/debug/debug_node.ts b/packages/core/src/debug/debug_node.ts
@@ -410,7 +410,7 @@ class DebugElement__POST_R3__ extends DebugNode__POST_R3__ implements DebugEleme
     this.listeners.forEach(listener => {
       if (listener.name === eventName) {
         const callback = listener.callback;
-        callback(eventObj);
+        callback.call(node, eventObj);
         invokedListeners.push(callback);
       }
     });
@@ -419,11 +419,20 @@ class DebugElement__POST_R3__ extends DebugNode__POST_R3__ implements DebugEleme
     // that Zone.js only adds to `EventTarget` in browser environments.
     if (typeof node.eventListeners === 'function') {
       // Note that in Ivy we wrap event listeners with a call to `event.preventDefault` in some
-      // cases. We use `Function` as a special token that gives us access to the actual event
+      // cases. We use '__ngUnwrap__' as a special token that gives us access to the actual event
       // listener.
       node.eventListeners(eventName).forEach((listener: Function) => {
-        const unwrappedListener = listener(Function);
-        return invokedListeners.indexOf(unwrappedListener) === -1 && unwrappedListener(eventObj);
+        // In order to ensure that we can detect the special __ngUnwrap__ token described above, we
+        // use `toString` on the listener and see if it contains the token. We use this approach to
+        // ensure that it still worked with compiled code since it cannot remove or rename string
+        // literals. We also considered using a special function name (i.e. if(listener.name ===
+        // special)) but that was more cumbersome and we were also concerned the compiled code could
+        // strip the name, turning the condition in to ("" === "") and always returning true.
+        if (listener.toString().indexOf('__ngUnwrap__') !== -1) {
+          const unwrappedListener = listener('__ngUnwrap__');
+          return invokedListeners.indexOf(unwrappedListener) === -1 &&
+              unwrappedListener.call(node, eventObj);
+        }
       });
     }
   }
diff --git a/packages/core/test/debug/debug_node_spec.ts b/packages/core/test/debug/debug_node_spec.ts
@@ -12,7 +12,7 @@ import {Component, DebugElement, DebugNode, Directive, ElementRef, EmbeddedViewR
 import {NgZone} from '@angular/core/src/zone';
 import {ComponentFixture, TestBed, async} from '@angular/core/testing';
 import {By} from '@angular/platform-browser/src/dom/debug/by';
-import {hasClass} from '@angular/platform-browser/testing/src/browser_util';
+import {createMouseEvent, hasClass} from '@angular/platform-browser/testing/src/browser_util';
 import {expect} from '@angular/platform-browser/testing/src/matchers';
 import {ivyEnabled, onlyInIvy} from '@angular/private/testing';
 
@@ -1253,4 +1253,23 @@ class TestCmptWithPropInterpolation {
     expect(fixture.debugElement.query(e => e.name === 'myComponent')).toBeTruthy();
     expect(fixture.debugElement.query(e => e.name === 'div')).toBeTruthy();
   });
+
+  it('does not call event listeners added outside angular context', () => {
+    let listenerCalled = false;
+    const eventToTrigger = createMouseEvent('mouseenter');
+    function listener() { listenerCalled = true; }
+    @Component({template: ''})
+    class MyComp {
+      constructor(private readonly zone: NgZone, private readonly element: ElementRef) {}
+      ngOnInit() {
+        this.zone.runOutsideAngular(
+            () => { this.element.nativeElement.addEventListener('mouseenter', listener); });
+      }
+    }
+    const fixture =
+        TestBed.configureTestingModule({declarations: [MyComp]}).createComponent(MyComp);
+    fixture.detectChanges();
+    fixture.debugElement.triggerEventHandler('mouseenter', eventToTrigger);
+    expect(listenerCalled).toBe(false);
+  });
 }
diff --git a/packages/platform-browser/src/dom/dom_renderer.ts b/packages/platform-browser/src/dom/dom_renderer.ts
@@ -50,10 +50,16 @@ export function flattenStyles(
 }
 
 function decoratePreventDefault(eventHandler: Function): Function {
+  // `DebugNode.triggerEventHandler` needs to know if the listener was created with
+  // decoratePreventDefault or is a listener added outside the Angular context so it can handle the
+  // two differently. In the first case, the special '__ngUnwrap__' token is passed to the unwrap
+  // the listener (see below).
   return (event: any) => {
-    // Ivy uses `Function` as a special token that allows us to unwrap the function
-    // so that it can be invoked programmatically by `DebugNode.triggerEventHandler`.
-    if (event === Function) {
+    // Ivy uses '__ngUnwrap__' as a special token that allows us to unwrap the function
+    // so that it can be invoked programmatically by `DebugNode.triggerEventHandler`. The debug_node
+    // can inspect the listener toString contents for the existence of this special token. Because
+    // the token is a string literal, it is ensured to not be modified by compiled code.
+    if (event === '__ngUnwrap__') {
       return eventHandler;
     }
 
