Skip to content

Commit

Permalink
fix(ngEventDirs): execute blur and focus expression using `scope.…
Browse files Browse the repository at this point in the history
…$evalAsync`

BREAKING CHANGE:
The `blur` and `focus` event fire synchronously, also during DOM operations
that remove elements. This lead to errors as the Angular model was not
in a consistent state. See this [fiddle](http://jsfiddle.net/fq1dq5yb/) for a demo.

This change executes the expression of those events using
`scope.$evalAsync` if an `$apply` is in progress, otherwise
keeps the old behavior.

Fixes angular#4979
Fixes angular#5945
Closes angular#8803
Closes angular#6910
Closes angular#5402
  • Loading branch information
tbosch committed Aug 28, 2014
1 parent 2efe1c2 commit df13570
Show file tree
Hide file tree
Showing 3 changed files with 99 additions and 21 deletions.
29 changes: 26 additions & 3 deletions src/ng/directive/ngEventDirs.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,14 @@
* Events that are handled via these handler are always configured not to propagate further.
*/
var ngEventDirectives = {};

// For events that might fire synchronously during DOM manipulation
// we need to execute their event handlers asynchronously using $evalAsync,
// so that they are not executed in an inconsistent state.
var forceAsyncEvents = {
'blur': true,
'focus': true
};
forEach(
'click dblclick mousedown mouseup mouseover mouseout mousemove mouseenter mouseleave keydown keyup keypress submit focus blur copy cut paste'.split(' '),
function(name) {
Expand All @@ -47,10 +55,16 @@ forEach(
compile: function($element, attr) {
var fn = $parse(attr[directiveName]);
return function ngEventHandler(scope, element) {
element.on(lowercase(name), function(event) {
scope.$apply(function() {
var eventName = lowercase(name);
element.on(eventName, function(event) {
var callback = function() {
fn(scope, {$event:event});
});
};
if (forceAsyncEvents[eventName] && scope.$$phase) {
scope.$evalAsync(callback);
} else {
scope.$apply(callback);
}
});
};
}
Expand Down Expand Up @@ -367,6 +381,10 @@ forEach(
* @description
* Specify custom behavior on focus event.
*
* Note: As the `focus` event is executed synchronously when calling `input.focus()`
* AngularJS executes the expression using `scope.$evalAsync` if the event is fired
* during an `$apply` to ensure a consistent state.
*
* @element window, input, select, textarea, a
* @priority 0
* @param {expression} ngFocus {@link guide/expression Expression} to evaluate upon
Expand All @@ -383,6 +401,11 @@ forEach(
* @description
* Specify custom behavior on blur event.
*
* Note: As the `blur` event is executed synchronously also during DOM manipulations
* (e.g. removing a focussed input),
* AngularJS executes the expression using `scope.$evalAsync` if the event is fired
* during an `$apply` to ensure a consistent state.
*
* @element window, input, select, textarea, a
* @priority 0
* @param {expression} ngBlur {@link guide/expression Expression} to evaluate upon
Expand Down
73 changes: 73 additions & 0 deletions test/ng/directive/ngEventDirsSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,77 @@ describe('event directives', function() {
expect($rootScope.formSubmitted).toEqual('foo');
}));
});

describe('focus', function() {

it('should call the listener asynchronously during $apply',
inject(function($rootScope, $compile) {
element = $compile('<input type="text" ng-focus="focus()">')($rootScope);
$rootScope.focus = jasmine.createSpy('focus');
// need to add to document so that the event fires
document.body.appendChild(element[0]);

$rootScope.$apply(function() {
element[0].focus();
expect($rootScope.focus).not.toHaveBeenCalled();
});

expect($rootScope.focus).toHaveBeenCalledOnce();
}));

it('should call the listener synchronously inside of $apply if outside of $apply',
inject(function($rootScope, $compile) {
element = $compile('<input type="text" ng-focus="focus()" ng-model="value">')($rootScope);
// need to add to document so that the event fires
document.body.appendChild(element[0]);

$rootScope.focus = jasmine.createSpy('focus').andCallFake(function() {
$rootScope.value = 'newValue';
});

element[0].focus();

expect($rootScope.focus).toHaveBeenCalledOnce();
expect(element.val()).toBe('newValue');
}));

});

describe('blur', function() {

it('should call the listener asynchronously during $apply',
inject(function($rootScope, $compile) {
element = $compile('<input type="text" ng-blur="blur()">')($rootScope);
// need to add to document so that the event fires
document.body.appendChild(element[0]);
element[0].focus();

$rootScope.blur = jasmine.createSpy('blur');

$rootScope.$apply(function() {
element[0].blur();
expect($rootScope.blur).not.toHaveBeenCalled();
});

expect($rootScope.blur).toHaveBeenCalledOnce();
}));

it('should call the listener synchronously inside of $apply if outside of $apply',
inject(function($rootScope, $compile) {
element = $compile('<input type="text" ng-blur="blur()" ng-model="value">')($rootScope);
// need to add to document so that the event fires
document.body.appendChild(element[0]);
element[0].focus();

$rootScope.blur = jasmine.createSpy('blur').andCallFake(function() {
$rootScope.value = 'newValue';
});

element[0].blur();

expect($rootScope.blur).toHaveBeenCalledOnce();
expect(element.val()).toBe('newValue');
}));

});
});
18 changes: 0 additions & 18 deletions test/ng/directive/ngKeySpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,23 +34,5 @@ describe('ngKeyup and ngKeydown directives', function() {
expect($rootScope.touched).toEqual(true);
}));

it('should get called on focus', inject(function($rootScope, $compile) {
element = $compile('<input ng-focus="touched = true">')($rootScope);
$rootScope.$digest();
expect($rootScope.touched).toBeFalsy();

browserTrigger(element, 'focus');
expect($rootScope.touched).toEqual(true);
}));

it('should get called on blur', inject(function($rootScope, $compile) {
element = $compile('<input ng-blur="touched = true">')($rootScope);
$rootScope.$digest();
expect($rootScope.touched).toBeFalsy();

browserTrigger(element, 'blur');
expect($rootScope.touched).toEqual(true);
}));

});

0 comments on commit df13570

Please sign in to comment.