fix(Scope): allow removing a listener during event

2 files changed, 91 insertions, 5 deletions

diff --git a/src/ng/rootScope.js b/src/ng/rootScope.js
index 5754ad32..de4b7407 100644
--- a/src/ng/rootScope.js
+++ b/src/ng/rootScope.js
@@ -638,7 +638,7 @@ function $RootScopeProvider(){
         namedListeners.push(listener);
 
         return function() {
-          arrayRemove(namedListeners, listener);
+          namedListeners[indexOf(namedListeners, listener)] = null;
         };
       },
 
@@ -686,6 +686,14 @@ function $RootScopeProvider(){
           namedListeners = scope.$$listeners[name] || empty;
           event.currentScope = scope;
           for (i=0, length=namedListeners.length; i<length; i++) {
+
+            // if listeners were deregistered, defragment the array
+            if (!namedListeners[i]) {
+              namedListeners.splice(i, 1);
+              i--;
+              length--;
+              continue;
+            }
             try {
               namedListeners[i].apply(null, listenerArgs);
               if (stopPropagation) return event;
@@ -735,19 +743,29 @@ function $RootScopeProvider(){
               },
               defaultPrevented: false
             },
-            listenerArgs = concat([event], arguments, 1);
+            listenerArgs = concat([event], arguments, 1),
+            listeners, i, length;
 
         //down while you can, then up and next sibling or up and next sibling until back at root
         do {
           current = next;
           event.currentScope = current;
-          forEach(current.$$listeners[name], function(listener) {
+          listeners = current.$$listeners[name] || [];
+          for (i=0, length = listeners.length; i<length; i++) {
+            // if listeners were deregistered, defragment the array
+            if (!listeners[i]) {
+              listeners.splice(i, 1);
+              i--;
+              length--;
+              continue;
+            }
+
             try {
-              listener.apply(null, listenerArgs);
+              listeners[i].apply(null, listenerArgs);
             } catch(e) {
               $exceptionHandler(e);
             }
-          });
+          }
 
           // Insanity Warning: scope depth-first traversal
           // yes, this code is a bit crazy, but it works and we have tests to prove it!
diff --git a/test/ng/rootScopeSpec.js b/test/ng/rootScopeSpec.js
index 9830d981..ee7fb796 100644
--- a/test/ng/rootScopeSpec.js
+++ b/test/ng/rootScopeSpec.js
@@ -708,6 +708,74 @@ describe('Scope', function() {
       });
 
 
+      it('should allow removing event listener inside a listener on $emit', function() {
+        var spy1 = jasmine.createSpy('1st listener');
+        var spy2 = jasmine.createSpy('2nd listener');
+        var spy3 = jasmine.createSpy('3rd listener');
+
+        var remove1 = child.$on('evt', spy1);
+        var remove2 = child.$on('evt', spy2);
+        var remove3 = child.$on('evt', spy3);
+
+        spy1.andCallFake(remove1);
+
+        expect(child.$$listeners['evt'].length).toBe(3);
+
+        // should call all listeners and remove 1st
+        child.$emit('evt');
+        expect(spy1).toHaveBeenCalledOnce();
+        expect(spy2).toHaveBeenCalledOnce();
+        expect(spy3).toHaveBeenCalledOnce();
+        expect(child.$$listeners['evt'].length).toBe(3); // cleanup will happen on next $emit
+
+        spy1.reset();
+        spy2.reset();
+        spy3.reset();
+
+        // should call only 2nd because 1st was already removed and 2nd removes 3rd
+        spy2.andCallFake(remove3);
+        child.$emit('evt');
+        expect(spy1).not.toHaveBeenCalled();
+        expect(spy2).toHaveBeenCalledOnce();
+        expect(spy3).not.toHaveBeenCalled();
+        expect(child.$$listeners['evt'].length).toBe(1);
+      });
+
+
+      it('should allow removing event listener inside a listener on $broadcast', function() {
+        var spy1 = jasmine.createSpy('1st listener');
+        var spy2 = jasmine.createSpy('2nd listener');
+        var spy3 = jasmine.createSpy('3rd listener');
+
+        var remove1 = child.$on('evt', spy1);
+        var remove2 = child.$on('evt', spy2);
+        var remove3 = child.$on('evt', spy3);
+
+        spy1.andCallFake(remove1);
+
+        expect(child.$$listeners['evt'].length).toBe(3);
+
+        // should call all listeners and remove 1st
+        child.$broadcast('evt');
+        expect(spy1).toHaveBeenCalledOnce();
+        expect(spy2).toHaveBeenCalledOnce();
+        expect(spy3).toHaveBeenCalledOnce();
+        expect(child.$$listeners['evt'].length).toBe(3); //cleanup will happen on next $broadcast
+
+        spy1.reset();
+        spy2.reset();
+        spy3.reset();
+
+        // should call only 2nd because 1st was already removed and 2nd removes 3rd
+        spy2.andCallFake(remove3);
+        child.$broadcast('evt');
+        expect(spy1).not.toHaveBeenCalled();
+        expect(spy2).toHaveBeenCalledOnce();
+        expect(spy3).not.toHaveBeenCalled();
+        expect(child.$$listeners['evt'].length).toBe(1);
+      });
+
+
       describe('event object', function() {
         it('should have methods/properties', function() {
           var event;