Permalink
Browse files

fix(Scope): $broadcast and $emit should set event.currentScope to null

When a event is finished propagating through Scope hierarchy the event's `currentScope` property
should be reset to `null` to avoid accidental use of this property in asynchronous event handlers.

In the previous code, the event's property would contain a reference to the last Scope instance that
was visited during the traversal, which is unlikely what the code trying to grab scope reference expects.

BREAKING CHANGE: $broadcast and $emit will now reset the `currentScope` property of the event to
null once the event finished propagating. If any code depends on asynchronously accessing thei
`currentScope` property, it should be migrated to use `targetScope` instead. All of these cases
should be considered programming bugs.

Closes #7445
Closes #7523
  • Loading branch information...
1 parent def5b57 commit 82f45aee5bd84d1cc53fb2e8f645d2263cdaacbc @IgorMinar IgorMinar committed May 20, 2014
Showing with 46 additions and 5 deletions.
  1. +9 −2 src/ng/rootScope.js
  2. +37 −3 test/ng/rootScopeSpec.js
View
@@ -971,7 +971,8 @@ function $RootScopeProvider(){
*
* - `targetScope` - `{Scope}`: the scope on which the event was `$emit`-ed or
* `$broadcast`-ed.
- * - `currentScope` - `{Scope}`: the current scope which is handling the event.
+ * - `currentScope` - `{Scope}`: the scope that is currently handling the event. Once the
+ * event propagates through the scope hierarchy, this property is set to null.
* - `name` - `{string}`: name of the event.
* - `stopPropagation` - `{function=}`: calling `stopPropagation` function will cancel
* further event propagation (available only for events that were `$emit`-ed).
@@ -1065,11 +1066,16 @@ function $RootScopeProvider(){
}
}
//if any listener on the current scope stops propagation, prevent bubbling
- if (stopPropagation) return event;
+ if (stopPropagation) {
+ event.currentScope = null;
+ return event;
+ }
//traverse upwards
scope = scope.$parent;
} while (scope);
+ event.currentScope = null;
+
return event;
},
@@ -1142,6 +1148,7 @@ function $RootScopeProvider(){
}
}
+ event.currentScope = null;
return event;
}
};
@@ -1563,15 +1563,30 @@ describe('Scope', function() {
describe('event object', function() {
it('should have methods/properties', function() {
- var event;
+ var eventFired = false;
+
child.$on('myEvent', function(e) {
expect(e.targetScope).toBe(grandChild);
expect(e.currentScope).toBe(child);
expect(e.name).toBe('myEvent');
+ eventFired = true;
+ });
+ grandChild.$emit('myEvent');
+ expect(eventFired).toBe(true);
+ });
+
+
+ it("should have it's `currentScope` property set to null after emit", function() {
+ var event;
+
+ child.$on('myEvent', function(e) {
event = e;
});
grandChild.$emit('myEvent');
- expect(event).toBeDefined();
+
+ expect(event.currentScope).toBe(null);
+ expect(event.targetScope).toBe(grandChild);
+ expect(event.name).toBe('myEvent');
});
@@ -1584,6 +1599,7 @@ describe('Scope', function() {
});
event = grandChild.$emit('myEvent');
expect(event.defaultPrevented).toBe(true);
+ expect(event.currentScope).toBe(null);
});
});
});
@@ -1700,6 +1716,24 @@ describe('Scope', function() {
it('should receive event object', inject(function($rootScope) {
var scope = $rootScope,
child = scope.$new(),
+ eventFired = false;
+
+ child.$on('fooEvent', function(event) {
+ eventFired = true;
+ expect(event.name).toBe('fooEvent');
+ expect(event.targetScope).toBe(scope);
+ expect(event.currentScope).toBe(child);
+ });
+ scope.$broadcast('fooEvent');
+
+ expect(eventFired).toBe(true);
+ }));
+
+
+ it("should have the event's `currentScope` property set to null after broadcast",
+ inject(function($rootScope) {
+ var scope = $rootScope,
+ child = scope.$new(),
event;
child.$on('fooEvent', function(e) {
@@ -1709,7 +1743,7 @@ describe('Scope', function() {
expect(event.name).toBe('fooEvent');
expect(event.targetScope).toBe(scope);
- expect(event.currentScope).toBe(child);
+ expect(event.currentScope).toBe(null);
}));

0 comments on commit 82f45ae

Please sign in to comment.