From 3db5ff15bf8a96cc2775dd2ffafe16458c1f71c4 Mon Sep 17 00:00:00 2001 From: Tobias Bosch Date: Wed, 27 Aug 2014 15:29:39 -0700 Subject: [PATCH] fix(ngEventDirs): execute `blur` and `focus` expression using `scope.$evalAsync` BREAKING CHANGE: The `blur` and `focus` event fire synchronously, also during DOM operations that add/remove elements. This lead to errors as the Angular model was not in a consistent state. This change executes the expression of those events now asynchronously. Fixes #4979 Fixes #5945 Closes #8803 Closes #6910 Closes #5402 --- src/ng/directive/ngEventDirs.js | 30 +++++++++++++++++++++++++--- test/ng/directive/ngEventDirsSpec.js | 30 ++++++++++++++++++++++++++++ test/ng/directive/ngKeySpec.js | 18 ----------------- 3 files changed, 57 insertions(+), 21 deletions(-) diff --git a/src/ng/directive/ngEventDirs.js b/src/ng/directive/ngEventDirs.js index d228c76077c0..22f5f46b80fe 100644 --- a/src/ng/directive/ngEventDirs.js +++ b/src/ng/directive/ngEventDirs.js @@ -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) { @@ -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.$evalAsync(callback); + } else { + scope.$apply(callback); + } }); }; } @@ -367,6 +381,11 @@ forEach( * @description * Specify custom behavior on focus event. * + * Note: As the `blur` event is executed synchronously also during DOM manipulations + * (e.g. adding an input with the html `autofocus` attribute), + * AngularJS executes the expression asynchronously (using `scope.$evalAsync`) to ensure + * a consistent model. + * * @element window, input, select, textarea, a * @priority 0 * @param {expression} ngFocus {@link guide/expression Expression} to evaluate upon @@ -383,6 +402,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 asynchronously (using `scope.$evalAsync`) to ensure + * a consistent model. + * * @element window, input, select, textarea, a * @priority 0 * @param {expression} ngBlur {@link guide/expression Expression} to evaluate upon diff --git a/test/ng/directive/ngEventDirsSpec.js b/test/ng/directive/ngEventDirsSpec.js index 5b73c2dd6a8b..6de97b6b1357 100644 --- a/test/ng/directive/ngEventDirsSpec.js +++ b/test/ng/directive/ngEventDirsSpec.js @@ -39,4 +39,34 @@ describe('event directives', function() { expect($rootScope.formSubmitted).toEqual('foo'); })); }); + + describe('sync events that might be triggered by DOM changes', function() { + + it('should call the blur and focus listener asynchronously', inject(function($rootScope, $compile) { + element = $compile('')($rootScope); + $rootScope.blur = jasmine.createSpy('blur'); + $rootScope.focus = jasmine.createSpy('focus'); + // need to add to document so that blur/focus fires + document.body.appendChild(element[0]); + + element[0].focus(); + + expect($rootScope.blur).not.toHaveBeenCalled(); + expect($rootScope.focus).not.toHaveBeenCalled(); + + $rootScope.$apply(); + + expect($rootScope.blur).not.toHaveBeenCalled(); + expect($rootScope.focus).toHaveBeenCalledOnce(); + + element[0].blur(); + + $rootScope.$apply(); + + expect($rootScope.blur).toHaveBeenCalledOnce(); + expect($rootScope.focus).toHaveBeenCalledOnce(); + + })); + + }); }); diff --git a/test/ng/directive/ngKeySpec.js b/test/ng/directive/ngKeySpec.js index ef5addd507ed..c7b989a48b14 100644 --- a/test/ng/directive/ngKeySpec.js +++ b/test/ng/directive/ngKeySpec.js @@ -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('')($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('')($rootScope); - $rootScope.$digest(); - expect($rootScope.touched).toBeFalsy(); - - browserTrigger(element, 'blur'); - expect($rootScope.touched).toEqual(true); - })); - });