diff --git a/docs/content/error/$rootScope/inevt.ngdoc b/docs/content/error/$rootScope/inevt.ngdoc deleted file mode 100644 index a06eeba18627..000000000000 --- a/docs/content/error/$rootScope/inevt.ngdoc +++ /dev/null @@ -1,22 +0,0 @@ -@ngdoc error -@name $rootScope:inevt -@fullName Recursive $emit/$broadcast event -@description - -This error occurs when the an event is `$emit`ed or `$broadcast`ed recursively on a scope. - -For example, when an event listener fires the same event being listened to. - -``` -$scope.$on('foo', function() { - $scope.$emit('foo'); -}); -``` - -Or when a parent element causes indirect recursion. - -``` -$scope.$on('foo', function() { - $rootScope.$broadcast('foo'); -}); -``` \ No newline at end of file diff --git a/src/ng/rootScope.js b/src/ng/rootScope.js index 964ba7bceaf5..7f37338e6636 100644 --- a/src/ng/rootScope.js +++ b/src/ng/rootScope.js @@ -1271,14 +1271,13 @@ function $RootScopeProvider() { var self = this; return function() { - var index = arrayRemove(namedListeners, listener); - if (index >= 0) { + var indexOfListener = namedListeners.indexOf(listener); + if (indexOfListener !== -1) { + // Use delete in the hope of the browser deallocating the memory for the array entry, + // while not shifting the array indexes of other listeners. + // See issue https://github.com/angular/angular.js/issues/16135 + delete namedListeners[indexOfListener]; decrementListenerCount(self, 1, name); - // We are removing a listener while iterating over the list of listeners. - // Update the current $$index if necessary to ensure no listener is skipped. - if (index <= namedListeners.$$index) { - namedListeners.$$index--; - } } }; }, @@ -1307,7 +1306,9 @@ function $RootScopeProvider() { * @return {Object} Event object (see {@link ng.$rootScope.Scope#$on}). */ $emit: function(name, args) { - var scope = this, + var empty = [], + namedListeners, + scope = this, stopPropagation = false, event = { name: name, @@ -1318,11 +1319,28 @@ function $RootScopeProvider() { }, defaultPrevented: false }, - listenerArgs = concat([event], arguments, 1); + listenerArgs = concat([event], arguments, 1), + i, length; do { - invokeListeners(scope, event, listenerArgs, name); - + 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 { + //allow all listeners attached to the current scope to run + namedListeners[i].apply(null, listenerArgs); + } catch (e) { + $exceptionHandler(e); + } + } //if any listener on the current scope stops propagation, prevent bubbling if (stopPropagation) { break; @@ -1373,11 +1391,28 @@ function $RootScopeProvider() { if (!target.$$listenerCount[name]) return event; - var listenerArgs = concat([event], arguments, 1); + var 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 while ((current = next)) { - invokeListeners(current, event, listenerArgs, name); + event.currentScope = current; + 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 { + 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! @@ -1408,27 +1443,6 @@ function $RootScopeProvider() { return $rootScope; - function invokeListeners(scope, event, listenerArgs, name) { - var listeners = scope.$$listeners[name]; - if (listeners) { - if (listeners.$$index !== undefined) { - throw $rootScopeMinErr('inevt', '{0} already $emit/$broadcast-ing on scope ({1})', name, scope.$id); - } - event.currentScope = scope; - try { - for (listeners.$$index = 0; listeners.$$index < listeners.length; listeners.$$index++) { - try { - //allow all listeners attached to the current scope to run - listeners[listeners.$$index].apply(null, listenerArgs); - } catch (e) { - $exceptionHandler(e); - } - } - } finally { - listeners.$$index = undefined; - } - } - } function beginPhase(phase) { if ($rootScope.$$phase) { diff --git a/test/ng/rootScopeSpec.js b/test/ng/rootScopeSpec.js index 90a79625e62c..eea9e824ae66 100644 --- a/test/ng/rootScopeSpec.js +++ b/test/ng/rootScopeSpec.js @@ -2444,10 +2444,12 @@ describe('Scope', function() { $rootScope.$on('abc', noop); expect($rootScope.$$listeners['abc'].length).toBe(2); + expect(0 in $rootScope.$$listeners['abc']).toBe(true); remove1(); - expect($rootScope.$$listeners['abc'].length).toBe(1); + expect($rootScope.$$listeners['abc'].length).toBe(2); + expect(0 in $rootScope.$$listeners['abc']).toBe(false); })); @@ -2583,107 +2585,6 @@ describe('Scope', function() { expect($rootScope.$$listenerCount).toEqual({abc: 1}); expect(child.$$listenerCount).toEqual({abc: 1}); })); - - - it('should throw on recursive $broadcast', inject(function($rootScope) { - $rootScope.$on('e', function() { $rootScope.$broadcast('e'); }); - - expect(function() { $rootScope.$emit('e', 5); }).toThrowMinErr('$rootScope', 'inevt', 'e already $emit/$broadcast-ing on scope (1)'); - expect(function() { $rootScope.$broadcast('e', 5); }).toThrowMinErr('$rootScope', 'inevt', 'e already $emit/$broadcast-ing on scope (1)'); - })); - - - it('should throw on nested recursive $broadcast', inject(function($rootScope) { - $rootScope.$on('e2', function() { $rootScope.$broadcast('e'); }); - $rootScope.$on('e', function() { $rootScope.$broadcast('e2'); }); - - expect(function() { $rootScope.$emit('e', 5); }).toThrowMinErr('$rootScope', 'inevt', 'e already $emit/$broadcast-ing on scope (1)'); - expect(function() { $rootScope.$broadcast('e', 5); }).toThrowMinErr('$rootScope', 'inevt', 'e already $emit/$broadcast-ing on scope (1)'); - })); - - - it('should throw on recursive $emit', inject(function($rootScope) { - $rootScope.$on('e', function() { $rootScope.$emit('e'); }); - - expect(function() { $rootScope.$emit('e', 5); }).toThrowMinErr('$rootScope', 'inevt', 'e already $emit/$broadcast-ing on scope (1)'); - expect(function() { $rootScope.$broadcast('e', 5); }).toThrowMinErr('$rootScope', 'inevt', 'e already $emit/$broadcast-ing on scope (1)'); - })); - - - it('should throw on nested recursive $emit', inject(function($rootScope) { - $rootScope.$on('e2', function() { $rootScope.$emit('e'); }); - $rootScope.$on('e', function() { $rootScope.$emit('e2'); }); - - expect(function() { $rootScope.$emit('e', 5); }).toThrowMinErr('$rootScope', 'inevt', 'e already $emit/$broadcast-ing on scope (1)'); - expect(function() { $rootScope.$broadcast('e', 5); }).toThrowMinErr('$rootScope', 'inevt', 'e already $emit/$broadcast-ing on scope (1)'); - })); - - - it('should throw on recursive $broadcast on child listener', inject(function($rootScope) { - var child = $rootScope.$new(); - child.$on('e', function() { $rootScope.$broadcast('e'); }); - - expect(function() { child.$emit('e', 5); }).toThrowMinErr('$rootScope', 'inevt', 'e already $emit/$broadcast-ing on scope (2)'); - expect(function() { child.$broadcast('e', 5); }).toThrowMinErr('$rootScope', 'inevt', 'e already $emit/$broadcast-ing on scope (2)'); - })); - - - it('should throw on nested recursive $broadcast on child listener', inject(function($rootScope) { - var child = $rootScope.$new(); - child.$on('e2', function() { $rootScope.$broadcast('e'); }); - child.$on('e', function() { $rootScope.$broadcast('e2'); }); - - expect(function() { child.$emit('e', 5); }).toThrowMinErr('$rootScope', 'inevt', 'e already $emit/$broadcast-ing on scope (2)'); - expect(function() { child.$broadcast('e', 5); }).toThrowMinErr('$rootScope', 'inevt', 'e already $emit/$broadcast-ing on scope (2)'); - })); - - - it('should throw on recursive $emit parent listener', inject(function($rootScope) { - var child = $rootScope.$new(); - $rootScope.$on('e', function() { child.$emit('e'); }); - - expect(function() { child.$emit('e', 5); }).toThrowMinErr('$rootScope', 'inevt', 'e already $emit/$broadcast-ing on scope (1)'); - expect(function() { $rootScope.$emit('e', 5); }).toThrowMinErr('$rootScope', 'inevt', 'e already $emit/$broadcast-ing on scope (1)'); - expect(function() { $rootScope.$broadcast('e', 5); }).toThrowMinErr('$rootScope', 'inevt', 'e already $emit/$broadcast-ing on scope (1)'); - })); - - - it('should throw on nested recursive $emit parent listener', inject(function($rootScope) { - var child = $rootScope.$new(); - $rootScope.$on('e2', function() { child.$emit('e'); }); - $rootScope.$on('e', function() { child.$emit('e2'); }); - - expect(function() { child.$emit('e', 5); }).toThrowMinErr('$rootScope', 'inevt', 'e already $emit/$broadcast-ing on scope (1)'); - expect(function() { $rootScope.$emit('e', 5); }).toThrowMinErr('$rootScope', 'inevt', 'e already $emit/$broadcast-ing on scope (1)'); - expect(function() { $rootScope.$broadcast('e', 5); }).toThrowMinErr('$rootScope', 'inevt', 'e already $emit/$broadcast-ing on scope (1)'); - })); - - - it('should clear recursive state of $broadcast if $exceptionHandler rethrows', function() { - module(function($exceptionHandlerProvider) { - $exceptionHandlerProvider.mode('rethrow'); - }); - inject(function($rootScope) { - var throwingListener = jasmine.createSpy('thrower').and.callFake(function() { - throw new Error('Listener Error!'); - }); - var secondListener = jasmine.createSpy('second'); - - $rootScope.$on('e', throwingListener); - $rootScope.$on('e', secondListener); - - expect(function() { $rootScope.$broadcast('e'); }).toThrowError('Listener Error!'); - expect(throwingListener).toHaveBeenCalled(); - expect(secondListener).not.toHaveBeenCalled(); - - throwingListener.calls.reset(); - secondListener.calls.reset(); - - expect(function() { $rootScope.$broadcast('e'); }).toThrowError('Listener Error!'); - expect(throwingListener).toHaveBeenCalled(); - expect(secondListener).not.toHaveBeenCalled(); - }); - }); }); }); @@ -2773,7 +2674,7 @@ describe('Scope', function() { expect(spy1).toHaveBeenCalledOnce(); expect(spy2).toHaveBeenCalledOnce(); expect(spy3).toHaveBeenCalledOnce(); - expect(child.$$listeners['evt'].length).toBe(2); + expect(child.$$listeners['evt'].length).toBe(3); // cleanup will happen on next $emit spy1.calls.reset(); spy2.calls.reset(); @@ -2807,7 +2708,7 @@ describe('Scope', function() { expect(spy1).toHaveBeenCalledOnce(); expect(spy2).toHaveBeenCalledOnce(); expect(spy3).toHaveBeenCalledOnce(); - expect(child.$$listeners['evt'].length).toBe(2); + expect(child.$$listeners['evt'].length).toBe(3); //cleanup will happen on next $broadcast spy1.calls.reset(); spy2.calls.reset(); @@ -3024,6 +2925,45 @@ describe('Scope', function() { })); }); }); + + + it('should allow recursive $emit/$broadcast', inject(function($rootScope) { + var callCount = 0; + $rootScope.$on('evt', function($event, arg0) { + callCount++; + if (arg0 !== 1234) { + $rootScope.$emit('evt', 1234); + $rootScope.$broadcast('evt', 1234); + } + }); + + $rootScope.$emit('evt'); + $rootScope.$broadcast('evt'); + expect(callCount).toBe(6); + })); + + + it('should allow recursive $emit/$broadcast between parent/child', inject(function($rootScope) { + var child = $rootScope.$new(); + var calls = ''; + + $rootScope.$on('evt', function($event, arg0) { + calls += 'r'; // For "root". + if (arg0 === 'fromChild') { + $rootScope.$broadcast('evt', 'fromRoot2'); + } + }); + + child.$on('evt', function($event, arg0) { + calls += 'c'; // For "child". + if (arg0 === 'fromRoot1') { + child.$emit('evt', 'fromChild'); + } + }); + + $rootScope.$broadcast('evt', 'fromRoot1'); + expect(calls).toBe('rccrrc'); + })); }); describe('doc examples', function() {