From 6b138db890e0f8a33127c7cfd10a56949ddf5510 Mon Sep 17 00:00:00 2001 From: ncuillery Date: Tue, 13 May 2014 10:41:22 +0200 Subject: [PATCH 1/3] fix($rootScope): avoid unstable reference in $broadcast event The event.currentScope was a reference to the iterator of the scope traversal (`current`). It caused problems when accessing the event.currentScope asynchronously : it was the last scope of the traversal instead of the scope which listen the event. --- src/ng/rootScope.js | 24 ++++++++++++++---------- test/ng/rootScopeSpec.js | 1 + 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/ng/rootScope.js b/src/ng/rootScope.js index 0936ef6a6a56..cc4aada2e95c 100644 --- a/src/ng/rootScope.js +++ b/src/ng/rootScope.js @@ -1096,23 +1096,25 @@ function $RootScopeProvider(){ * @return {Object} Event object, see {@link ng.$rootScope.Scope#$on} */ $broadcast: function(name, args) { + function Event(currentScope) { + this.name = name; + this.currentScope = currentScope; + this.targetScope = target; + this.defaultPrevented = false; + } + Event.prototype.preventDefault = function() { + this.defaultPrevented = true; + } + var target = this, current = target, next = target, - event = { - name: name, - targetScope: target, - preventDefault: function() { - event.defaultPrevented = true; - }, - defaultPrevented: false - }, - listenerArgs = concat([event], arguments, 1), + listenerArgs, + event = new Event(current), listeners, i, length; //down while you can, then up and next sibling or up and next sibling until back at root while ((current = next)) { - event.currentScope = current; listeners = current.$$listeners[name] || []; for (i=0, length = listeners.length; i Date: Tue, 13 May 2014 12:18:48 +0200 Subject: [PATCH 2/3] style: add missing semi-colon --- src/ng/rootScope.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ng/rootScope.js b/src/ng/rootScope.js index cc4aada2e95c..84dee5b57a08 100644 --- a/src/ng/rootScope.js +++ b/src/ng/rootScope.js @@ -1104,7 +1104,7 @@ function $RootScopeProvider(){ } Event.prototype.preventDefault = function() { this.defaultPrevented = true; - } + }; var target = this, current = target, From bd131bbbe004780880f942bc71a30fc1800738cb Mon Sep 17 00:00:00 2001 From: ncuillery Date: Thu, 15 May 2014 18:48:04 +0200 Subject: [PATCH 3/3] fix($rootScope): avoid unstable reference in $emit event The event.currentScope in `$emit` method was a reference to an iterator. When accessing asynchronously, the event.currentScope was the $rootScope (end of the do/while loop) instead of the scope which listen the event. The `$emit` and `$broadcast` methods use the same syntax now. Both methods are tested with : - sync/async access to `event.currentScope` and `event.targetScope`. - a test case for `preventDefault()` and `defaultPrevented`. --- src/ng/rootScope.js | 38 +++++++++++++++++++++++++------------- test/ng/rootScopeSpec.js | 22 ++++++++++++++++++++-- 2 files changed, 45 insertions(+), 15 deletions(-) diff --git a/src/ng/rootScope.js b/src/ng/rootScope.js index 84dee5b57a08..a0f8f45d3969 100644 --- a/src/ng/rootScope.js +++ b/src/ng/rootScope.js @@ -1029,25 +1029,33 @@ function $RootScopeProvider(){ * @return {Object} Event object (see {@link ng.$rootScope.Scope#$on}). */ $emit: function(name, args) { + function Event(currentScope) { + this.name = name; + this.currentScope = currentScope; + this.targetScope = target; + this.defaultPrevented = globalDefaultPrevented; + } + Event.prototype.preventDefault = function() { + // Deals with the current instance (this) and also the next ones with globalDefaultPrevented. + this.defaultPrevented = true; + globalDefaultPrevented = true; + }; + Event.prototype.stopPropagation = function() { + stopPropagation = true; + }; + var empty = [], namedListeners, + target = this, scope = this, stopPropagation = false, - event = { - name: name, - targetScope: scope, - stopPropagation: function() {stopPropagation = true;}, - preventDefault: function() { - event.defaultPrevented = true; - }, - defaultPrevented: false - }, - listenerArgs = concat([event], arguments, 1), + globalDefaultPrevented = false, + event = new Event(scope), + listenerArgs, i, length; do { namedListeners = scope.$$listeners[name] || empty; - event.currentScope = scope; for (i=0, length=namedListeners.length; i