fix(core): fix angular#20532, should be able to cancel listener from mixed zone (angular#20538)

JiaLiPassion · mhevery · commit a740e4f00ad4 · 2017-11-21T11:49:36.000-06:00
PR Close angular#20538
diff --git a/packages/platform-browser/src/dom/events/dom_events.ts b/packages/platform-browser/src/dom/events/dom_events.ts
@@ -203,16 +203,26 @@ export class DomEventsPlugin extends EventManagerPlugin {
       // just call native removeEventListener
       return target[NATIVE_REMOVE_LISTENER].apply(target, [eventName, callback, false]);
     }
+    // fix issue 20532, should be able to remove
+    // listener which was added inside of ngZone
+    let found = false;
     for (let i = 0; i < taskDatas.length; i++) {
       // remove listener from taskDatas if the callback equals
       if (taskDatas[i].handler === callback) {
+        found = true;
         taskDatas.splice(i, 1);
         break;
       }
     }
-    if (taskDatas.length === 0) {
-      // all listeners are removed, we can remove the globalListener from target
-      underlyingRemove.apply(target, [eventName, globalListener, false]);
+    if (found) {
+      if (taskDatas.length === 0) {
+        // all listeners are removed, we can remove the globalListener from target
+        underlyingRemove.apply(target, [eventName, globalListener, false]);
+      }
+    } else {
+      // not found in taskDatas, the callback may be added inside of ngZone
+      // use native remove listener to remove the calback
+      target[NATIVE_REMOVE_LISTENER].apply(target, [eventName, callback, false]);
     }
   }
 }
diff --git a/packages/platform-browser/test/dom/events/event_manager_spec.ts b/packages/platform-browser/test/dom/events/event_manager_spec.ts
@@ -255,6 +255,46 @@ export function main() {
       expect(receivedEvents).toEqual([]);
     });
 
+    it('should be able to remove event listener which was added inside of ngZone', () => {
+      const Zone = (window as any)['Zone'];
+
+      const element = el('<div><div></div></div>');
+      getDOM().appendChild(doc.body, element);
+      const dispatchedEvent = getDOM().createMouseEvent('click');
+      let receivedEvents: any[] /** TODO #9100 */ = [];
+      let receivedZones: any[] = [];
+      const handler1 = (e: any /** TODO #9100 */) => {
+        receivedEvents.push(e);
+        receivedZones.push(Zone.current.name);
+      };
+      const handler2 = (e: any /** TODO #9100 */) => {
+        receivedEvents.push(e);
+        receivedZones.push(Zone.current.name);
+      };
+      const manager = new EventManager([domEventPlugin], new FakeNgZone());
+
+      let remover1 = null;
+      let remover2 = null;
+      // handler1 is added in root zone
+      Zone.root.run(() => { remover1 = manager.addEventListener(element, 'click', handler1); });
+      // handler2 is added in 'angular' zone
+      Zone.root.fork({name: 'fakeAngularZone', properties: {isAngularZone: true}}).run(() => {
+        remover2 = manager.addEventListener(element, 'click', handler2);
+      });
+      getDOM().dispatchEvent(element, dispatchedEvent);
+      expect(receivedEvents).toEqual([dispatchedEvent, dispatchedEvent]);
+      expect(receivedZones).toEqual([Zone.root.name, 'fakeAngularZone']);
+
+      receivedEvents = [];
+      remover1 && remover1();
+      remover2 && remover2();
+      getDOM().dispatchEvent(element, dispatchedEvent);
+      // handler1 and handler2 are added in different zone
+      // one is angular zone, the other is not
+      // should still be able to remove them correctly
+      expect(receivedEvents).toEqual([]);
+    });
+
     it('should run blackListedEvents handler outside of ngZone', () => {
       const Zone = (window as any)['Zone'];
       const element = el('<div><div></div></div>');

Original file line number	Diff line number	Diff line change
`@@ -203,16 +203,26 @@ export class DomEventsPlugin extends EventManagerPlugin {`
`203`	`203`	`// just call native removeEventListener`
`204`	`204`	`return target[NATIVE_REMOVE_LISTENER].apply(target, [eventName, callback, false]);`
`205`	`205`	`}`
	`206`	`+ // fix issue 20532, should be able to remove`
	`207`	`+ // listener which was added inside of ngZone`
	`208`	`+ let found = false;`
`206`	`209`	`for (let i = 0; i < taskDatas.length; i++) {`
`207`	`210`	`// remove listener from taskDatas if the callback equals`
`208`	`211`	`if (taskDatas[i].handler === callback) {`
	`212`	`+ found = true;`
`209`	`213`	`taskDatas.splice(i, 1);`
`210`	`214`	`break;`
`211`	`215`	`}`
`212`	`216`	`}`
`213`		`- if (taskDatas.length === 0) {`
`214`		`- // all listeners are removed, we can remove the globalListener from target`
`215`		`- underlyingRemove.apply(target, [eventName, globalListener, false]);`
	`217`	`+ if (found) {`
	`218`	`+ if (taskDatas.length === 0) {`
	`219`	`+ // all listeners are removed, we can remove the globalListener from target`
	`220`	`+ underlyingRemove.apply(target, [eventName, globalListener, false]);`
	`221`	`+ }`
	`222`	`+ } else {`
	`223`	`+ // not found in taskDatas, the callback may be added inside of ngZone`
	`224`	`+ // use native remove listener to remove the calback`
	`225`	`+ target[NATIVE_REMOVE_LISTENER].apply(target, [eventName, callback, false]);`
`216`	`226`	`}`
`217`	`227`	`}`
`218`	`228`	`}`