Implemented a "safe apply" pattern to the event directives. #5402
Conversation
|
I'm sorry, but I wasn't able to verify your CLA signature. CLA signature is required for any code contributions to AngularJS. Please sign our CLA and ensure that the CLA signature email address and the email address in this PR's commits match. If you signed the CLA as a corporation, please let me know the company's name. Thanks a bunch! PS: If you signed the CLA in the past then most likely the email addresses don't match. Please sign the CLA again or update the email address in the commit of this PR. |
|
Make sure your git author email address ( |
When the DOM is modified during a digest cycle it can cause the blur event to be raised. Change the code to call $apply only if necessary. Should resolve #5401
|
O.K. I had the CLA signed. I'm still getting familiar with GIT and GitHub. Hopefully the latest commit looks right. |
|
In particular it needs to be signed with |
|
I would really rather not use "safe apply" to fix this. @caitp - you were going to take a look? |
|
It's interesting because (the error thrown in the jsfiddle) doesn't appear to affect FF or IE. I'll poke through it some time this week to see what the underlying cause is, probably tomorrow or Wednesday. |
|
The AngularJS docs app uses |
|
$timeout is an option, but if we use that in ngEventDirs that's a pretty breaking change which would potentially require changes to a lot of tests ($timeout.flush() everywhere). I haven't looked into the reason why we're hitting that outside of FF yet, though, so it's possible that it's A) not implemented very well in the repro case, or B) some other problem. |
|
I think that it is only an issue for focus/blur events. So these On 17 December 2013 22:39, Caitlin Potter [email protected] wrote:
|
|
well, "$safeApply" would have the advantage of not breaking the API at all, but maybe it's not such a big deal to use $timeout for just those couple events. Still, it seems like it shouldn't be necessary at all, I'm sure there's something wrong happening which could be avoided. @mike-schenk do you want some help bundling focus/blur into a separate directive "group" similar to the other one, but using $timeout? you'll need to add another line like this to make that work --- Actually I'm wrong, I guess you wouldn't really need that, heh. http://plnkr.co/edit/H05z8Y5nXSFYg0qT3CIP?p=preview working example, see the copy of angular.js around L#17731 It's just a bit unfortunate that it adds a whole bunch of code :( |
ghost
assigned
|
there might be a better way to fix this, please see: #5401 (comment) |
|
I thought Angular was going to make all applies be safe applies... that's not happening? In some cases where you have multiple levels of callback which may or may not be async, it becomes impossible to tell if you'd be in a $digest or not, and really what's the harm of gracefully handling redundant $apply calls? |
|
CLA signature verified! Thank you! Someone from the team will now triage your PR and it will be processed based on the determined priority (doc updates and fixes with tests are prioritized over other changes). |
…$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 angular#4979 Fixes angular#5945 Closes angular#8803 Closes angular#6910 Closes angular#5402
…$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
…$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
…$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 #4979 Fixes #5945 Closes #8803 Closes #6910 Closes #5402
Should resolve #5401