Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

Commit

Permalink
fix($rootScope): avoid unstable reference in $emit event
Browse files Browse the repository at this point in the history
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`.
  • Loading branch information
ncuillery committed May 15, 2014
1 parent 814f0e0 commit bd131bb
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 15 deletions.
38 changes: 25 additions & 13 deletions src/ng/rootScope.js
Original file line number Diff line number Diff line change
Expand Up @@ -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<length; i++) {

// if listeners were deregistered, defragment the array
Expand All @@ -1059,6 +1067,8 @@ function $RootScopeProvider(){
}
try {
//allow all listeners attached to the current scope to run
event = new Event(scope);
listenerArgs = concat([event], arguments, 1);
namedListeners[i].apply(null, listenerArgs);
} catch (e) {
$exceptionHandler(e);
Expand All @@ -1069,7 +1079,6 @@ function $RootScopeProvider(){
//traverse upwards
scope = scope.$parent;
} while (scope);

return event;
},

Expand Down Expand Up @@ -1100,16 +1109,19 @@ function $RootScopeProvider(){
this.name = name;
this.currentScope = currentScope;
this.targetScope = target;
this.defaultPrevented = false;
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;
};

var target = this,
current = target,
next = target,
listenerArgs,
globalDefaultPrevented = false,
event = new Event(current),
listeners, i, length;

Expand Down
22 changes: 20 additions & 2 deletions test/ng/rootScopeSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1572,6 +1572,10 @@ describe('Scope', function() {
});
grandChild.$emit('myEvent');
expect(event).toBeDefined();

// Asynchronous expectation : current & target should be the same as above
expect(event.targetScope).toBe(grandChild);
expect(event.currentScope).toBe(child);
});


Expand Down Expand Up @@ -1693,6 +1697,18 @@ describe('Scope', function() {
expect(result.name).toBe('some');
expect(result.targetScope).toBe(child1);
});

it('should have preventDefault method and defaultPrevented property', function() {
var event = child2.$broadcast('myEvent');
expect(event.defaultPrevented).toBe(false);

grandChild21.$on('myEvent', function(event) {
event.preventDefault();
});
event = child2.$broadcast('myEvent');
expect(event.defaultPrevented).toBe(true);
});

});


Expand All @@ -1704,16 +1720,18 @@ describe('Scope', function() {
event;

child.$on('fooEvent', function(e) {
expect(e.targetScope).toBe(scope);
expect(e.currentScope).toBe(child);
expect(e.name).toBe('fooEvent');
event = e;
});
scope.$broadcast('fooEvent');

expect(event.name).toBe('fooEvent');
// Asynchronous expectation : current & target should be the same as above
expect(event.targetScope).toBe(scope);
expect(event.currentScope).toBe(child);
}));


it('should support passing messages as varargs', inject(function($rootScope) {
var scope = $rootScope,
child = scope.$new(),
Expand Down

0 comments on commit bd131bb

Please sign in to comment.