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

Suggestion to use safe $apply in ngModelDirective #8762

Closed
smashercosmo opened this issue Aug 25, 2014 · 3 comments
Closed

Suggestion to use safe $apply in ngModelDirective #8762

smashercosmo opened this issue Aug 25, 2014 · 3 comments

Comments

@smashercosmo
Copy link

In ngModelDirective $apply is used to change model's value on event different from default and to change model's $touched state. But I suggest to use safe $apply instead, because we can't guarantee that some event (blur for example) won't be triggered from the outside during digest cycle. Real life example. Try to change value in the first input - you'll get "$digest already in progress" error.

@jeffbcross jeffbcross self-assigned this Sep 3, 2014
@jeffbcross jeffbcross added this to the Backlog milestone Sep 3, 2014
@jeffbcross jeffbcross removed their assignment Sep 3, 2014
@jeffbcross
Copy link
Contributor

@tbosch any idea why this wasn't fixed by 719c747?

@tbosch
Copy link
Contributor

tbosch commented Oct 2, 2014

The problem is we have another blur listener built into ngModel that does not use the semantics of ng-blur (see https://github.com/angular/angular.js/blob/master/src/ng/directive/input.js#L2430). Need to fix this as well...

@Narretz Narretz self-assigned this Oct 18, 2014
alexanderchan added a commit to alexanderchan/angular.js that referenced this issue Oct 28, 2014
If the model is blurred during an apply it should trigger the $touched
asynchronously.

Fixes angular#8762
@phaas
Copy link

phaas commented Nov 13, 2014

I wonder if this change fixes this issue:

http://jsfiddle.net/phaas/01ggb4du/6/

  1. On $destroy the directive manipulates the focused element in a way that makes it lose focus (real world example: boostrap typeahead.destroy removes wrapper objects).
  2. jQuery fires the blur event
  3. ngModel picks up blur event, attempts to $digest
  4. $digest is already in progress.

Above example does actually work in angular 1.2.9

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants