Skip to content

Commit

Permalink
fix(Scope): $broadcast and $emit should set event.currentScope to null
Browse files Browse the repository at this point in the history
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 angular#7445"
  • Loading branch information
IgorMinar committed May 20, 2014
1 parent 6b52848 commit ecce65a
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 4 deletions.
9 changes: 8 additions & 1 deletion src/ng/rootScope.js
Original file line number Diff line number Diff line change
Expand Up @@ -1065,11 +1065,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;
},

Expand Down Expand Up @@ -1142,6 +1147,8 @@ function $RootScopeProvider(){
}
}

event.currentScope = null;

return event;
}
};
Expand Down
40 changes: 37 additions & 3 deletions test/ng/rootScopeSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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).toBeDefined();
});


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');
});


Expand All @@ -1584,6 +1599,7 @@ describe('Scope', function() {
});
event = grandChild.$emit('myEvent');
expect(event.defaultPrevented).toBe(true);
expect(event.currentScope).toBe(null);
});
});
});
Expand Down Expand Up @@ -1698,6 +1714,24 @@ describe('Scope', function() {

describe('listener', 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;
Expand All @@ -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);
}));


Expand Down

0 comments on commit ecce65a

Please sign in to comment.