Problem with <input type="email" /> and form validation

[vytautas-pranskunas-]

After migration to 1.4 I discovered that form element does is not changing and does not get ng-dirty class is one of the form elements is input with type="email". Also with this input and forms are marked with attribute data-ng-animate="1"

[kentcdodds]

Verified:

I believe this is in fact an issue with 1.4. See this jsbin and enter a@b. Then do the same with this clone which is using 1.3.15. And you'll notice the className is not changing appropriately in the 1.4 version.

Note: the data-ng-animate I believe is by design.

[Narretz]

Looks like ngAnimate is trying to animate where there's no animation and gets stuck. @matsko has pushed a lot of fixes which will be in rc.1. So it's possible this is already fixed.

[gkalpak]

Indeed, it seems to have aready been fixed on master.

[matsko]

Looks like it's working. Sorry for the bug. We should have a RC1 release later today.

[matsko]

Closing for now, please re-open if the issue is still active.

[sravenhorst]

Unfortunately this is still broke. I made a new issue for it as this issue is closed: #11862

[gkalpak]

Ooops, really sorry for that 😞

The issue is indeed still reproducible on master: http://jsbin.com/tiwijowiza/3/edit (jsbin by @sravenhorst).

[gkalpak]

A little more context:

This does not only affect [type="email"], but potentially any input that has a validator with the same name as ctrl.$$parserName. E.g. it also affects [type="url"] and here is an example of it affecting a custom text input, where I explicitly set $$parserName to text and add a $validator.text validator.

Still not sure what is going wrong with ngAnimate, but here is what's going on with:
In NgModelController.$$runValidators():

this.$$runValidators = function(modelValue, viewValue, doneCallback) {
  ...
  if (!processParseErrors()) {...}      // (1)
  if (!processSyncValidators()) {...}   // (2)
  processAsyncValidators();             // (3)
  ...

The problem begins when (on an invalid element) for a given input there is no parse error, but there is a sync-validator error (for the validator which has the same name as the parser), e.g. email.

The series of events is as follows:

1. element.className === '...ng-invalid ng-invalid-email'
2. Executing processParseErrors() determines that there is no parser error (and since $$parserName === 'email') the following animations are queued:
   • addClass: 'ng-valid'
   • addClass: 'ng-valid-email'
   • removeClass: 'ng-invalid'
   • removeClass: 'ng-invalid-email'
3. Executing processSyncValidators() determines that there is an error (and since the validator's name is also 'email') the following animations are queued:
   • removeClass: 'ng-valid'
   • removeClass: 'ng-valid-email'
   • addClass: 'ng-invalid'
   • addClass: 'ng-invalid-email'
4. After the animations are over (which basically results in no changes), [data-ng-animate="1"] remains on the element. From that point on, the $animate.add/removeClass seems to have no effect, thus leaving the ng-invalid ng-invalid-email classes, even after ctrl.$valid gets true.

(Hopefully, @matsko can make better use of this info than me :))

[gkalpak]

A little more context:

The series of animation described in (2) and (3) above, result in joining all the animations in one and merging their options, which in the end will result in options.addClass: null and options.removeClass: null (basically no animation).

But the joned animation is not cancelled, so one we hit the registered $$postDigest callback, isValidAnimation will evaluate to false (here), leading into this branch and finally returning without cancelling/ending the animation.

Thus the element's animation will remain in PRE_DIGEST_STATE ([data-ng-animate="1"]) no matter how many more add/removeClass animation will be queued, because:

• There will always be an existing animation detected and the new add/removeClass animation will be joined with it.
• Upon joining the options will be merged and the new runner will be returned, but...
• There will be no more $$postDigest callback (since the one from the existing animation has already been executed).

All in all, this seems to be a more general bug in ngAnimate and the core issue seems to me that:

In the $$postdigest callback of a queued animation that has isValidAnimation === true but animationCancelled === false and isStructural === false will be never cancelled/ended.
This will cause all subsequent "joinable" animations to be joined with it, without ever been actually run.

I am sure @matsko has a much better insight into the necessary fixes, but I would suspect something like modifying the if block in animateQueue.js#L381-L398 like this:

if (animationCancelled || animationDetails.counter !== counter || !isValidAnimation) {
   if (animationCancelled) {...}

   if (animationCancelled || (isStructural && animationDetails.event !== event)) {
     options.domOperation();
     runner.end();
   }

  // <new-snippet>
  if (!isValidAnimation) clearElementAnimationState(element);
  // </new-snipet>

  return;
}

[gkalpak]

I incorrectly assumed that runner.end(); would clean-up the cancelled/ended/invalid animation (it doesn't). I had to update the above snippet, replacing it with:
if (!isValidAnimation) clearElementAnimationState(element); (which seems to do the trick)

/cc @matsko (if you think it's the right fix and want me to submit a PR, just lmk 😉)

[sravenhorst]

Thanks for all the effort so far! Great to see the quick investigation!

[gkalpak]

Trying to remedy for tricking people into believing it was fixed 😄