fix(jqLite): remove native listener when all jqLite listeners were de…

…registered This fixes an iOS issue where some events buble only when native listeners are present (see angular#9509), but more importantly previously we would pass wrong argument into the `removeEventListenerFn` which caused native listeners to be never deregistered. Oops! Closes angular#9509
ibrahim87 · Oct 10, 2014 · d71fb6f · d71fb6f
1 parent addfff3
commit d71fb6f
Show file tree

Hide file tree

Showing 2 changed files with 98 additions and 6 deletions.
diff --git a/src/jqLite.js b/src/jqLite.js
@@ -274,18 +274,22 @@ function jqLiteOff(element, type, fn, unsupported) {
   if (!type) {
     for (type in events) {
       if (type !== '$destroy') {
-        removeEventListenerFn(element, type, events[type]);
+        removeEventListenerFn(element, type, handle);
       }
       delete events[type];
     }
   } else {
     forEach(type.split(' '), function(type) {
-      if (isUndefined(fn)) {
-        removeEventListenerFn(element, type, events[type]);
-        delete events[type];
-      } else {
-        arrayRemove(events[type] || [], fn);
+      if (isDefined(fn)) {
+        var listenerFns = events[type];
+        arrayRemove(listenerFns || [], fn);
+        if (listenerFns && listenerFns.length > 0) {
+          return;
+        }
       }
+
+      removeEventListenerFn(element, type, handle);
+      delete events[type];
     });
   }
 }

diff --git a/test/jqLiteSpec.js b/test/jqLiteSpec.js
@@ -1400,6 +1400,94 @@ describe('jqLite', function() {
       expect(extraSpy).toHaveBeenCalledOnce();
     });
 
+
+    describe('native listener deregistration', function() {
+
+      it('should deregister the native listener when all jqLite listeners for given type are gone ' +
+         'after off("eventName", listener) call',  function() {
+        var aElem = jqLite(a);
+        var addEventListenerSpy = spyOn(aElem[0], 'addEventListener').andCallThrough();
+        var removeEventListenerSpy = spyOn(aElem[0], 'removeEventListener').andCallThrough();
+        var nativeListenerFn;
+
+        var jqLiteListener = function() {};
+        aElem.on('click', jqLiteListener);
+
+        expect(addEventListenerSpy).toHaveBeenCalledOnceWith('click', jasmine.any(Function), false);
+        nativeListenerFn = addEventListenerSpy.mostRecentCall.args[1];
+        expect(removeEventListenerSpy).not.toHaveBeenCalled();
+
+        aElem.off('click', jqLiteListener);
+        expect(removeEventListenerSpy).toHaveBeenCalledOnceWith('click', nativeListenerFn, false);
+      });
+
+
+      it('should deregister the native listener when all jqLite listeners for given type are gone ' +
+         'after off("eventName") call',  function() {
+        var aElem = jqLite(a);
+        var addEventListenerSpy = spyOn(aElem[0], 'addEventListener').andCallThrough();
+        var removeEventListenerSpy = spyOn(aElem[0], 'removeEventListener').andCallThrough();
+        var nativeListenerFn;
+
+        aElem.on('click', function() {});
+        expect(addEventListenerSpy).toHaveBeenCalledOnceWith('click', jasmine.any(Function), false);
+        nativeListenerFn = addEventListenerSpy.mostRecentCall.args[1];
+        expect(removeEventListenerSpy).not.toHaveBeenCalled();
+
+        aElem.off('click');
+        expect(removeEventListenerSpy).toHaveBeenCalledOnceWith('click', nativeListenerFn, false);
+      });
+
+
+      it('should deregister the native listener when all jqLite listeners for given type are gone ' +
+         'after off("eventName1 eventName2") call',  function() {
+        var aElem = jqLite(a);
+        var addEventListenerSpy = spyOn(aElem[0], 'addEventListener').andCallThrough();
+        var removeEventListenerSpy = spyOn(aElem[0], 'removeEventListener').andCallThrough();
+        var nativeListenerFn;
+
+        aElem.on('click', function() {});
+        expect(addEventListenerSpy).toHaveBeenCalledOnceWith('click', jasmine.any(Function), false);
+        nativeListenerFn = addEventListenerSpy.mostRecentCall.args[1];
+        addEventListenerSpy.reset();
+
+        aElem.on('dblclick', function() {});
+        expect(addEventListenerSpy).toHaveBeenCalledOnceWith('dblclick', nativeListenerFn, false);
+
+        expect(removeEventListenerSpy).not.toHaveBeenCalled();
+
+        aElem.off('click dblclick');
+
+        expect(removeEventListenerSpy).toHaveBeenCalledWith('click', nativeListenerFn, false);
+        expect(removeEventListenerSpy).toHaveBeenCalledWith('dblclick', nativeListenerFn, false);
+        expect(removeEventListenerSpy.callCount).toBe(2);
+      });
+
+
+      it('should deregister the native listener when all jqLite listeners for given type are gone ' +
+         'after off() call',  function() {
+        var aElem = jqLite(a);
+        var addEventListenerSpy = spyOn(aElem[0], 'addEventListener').andCallThrough();
+        var removeEventListenerSpy = spyOn(aElem[0], 'removeEventListener').andCallThrough();
+        var nativeListenerFn;
+
+        aElem.on('click', function() {});
+        expect(addEventListenerSpy).toHaveBeenCalledOnceWith('click', jasmine.any(Function), false);
+        nativeListenerFn = addEventListenerSpy.mostRecentCall.args[1];
+        addEventListenerSpy.reset();
+
+        aElem.on('dblclick', function() {});
+        expect(addEventListenerSpy).toHaveBeenCalledOnceWith('dblclick', nativeListenerFn, false);
+
+        aElem.off();
+
+        expect(removeEventListenerSpy).toHaveBeenCalledWith('click', nativeListenerFn, false);
+        expect(removeEventListenerSpy).toHaveBeenCalledWith('dblclick', nativeListenerFn, false);
+        expect(removeEventListenerSpy.callCount).toBe(2);
+      });
+    });
+
+
     // Only run this test for jqLite and not normal jQuery
     if ( _jqLiteMode ) {
       it('should throw an error if a selector is passed', function () {