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

Input type="number" invalid on load if ng-required is used #9106

Closed
greglockwood opened this issue Sep 16, 2014 · 7 comments
Closed

Input type="number" invalid on load if ng-required is used #9106

greglockwood opened this issue Sep 16, 2014 · 7 comments

Comments

@greglockwood
Copy link

A regression has occurred in Angular version 1.3.0-rc.1 where using ng-required on an input with type="number" results in incorrect behaviour on initialisation.

The issue is that regardless of what the ng-required expression returns, the input is marked as having an invalid number when it is empty due to being bound to a non-existent property.

I have reproduced this with a Plunkr, which shows output like so:
image

I was able to have a hacky workaround locally by creating a directive that would shift() the badInputChecker parser function from the start of the ngModelController.$parsers array, and push it onto the end instead. Thus it is likely related to the fact that the badInputChecker is added as the first parser function.

@Narretz
Copy link
Contributor

Narretz commented Sep 16, 2014

I think this is the same problem as #9063 and #9044. When ngRequired calls $validate in the observer, the viewValue is first parsed, and since it is empty it sets the error.

@leefernandes
Copy link
Contributor

beta.19 vs rc.2 demos:

1.3.0-beta.19: http://codepen.io/ItsLeeOwen/pen/Fjxnh
1.3.0-rc.2: http://codepen.io/ItsLeeOwen/pen/aAzrL

@gkalpak
Copy link
Member

gkalpak commented Sep 19, 2014

Actually, as far as I can tell, it is not that anything is actually found to be invalid.

I believe the problem is that badInputChecker() returns undefined if native validation reports an error and returns the value (which is modelValue) if everything is valid.

The problem arises when the modelValue itself is undefined, so even if badInputChecker() finds everything to be valid, it returns the undefined modelValue, which tricks the NgModelController into believing it is actually invalid.

Since badInputChecker() does not really care about the modelValue it is passed (it only returns if the input is valid or ignores it), it shouldn't make any difference if it is passed a value of undefined or null. But it makes all the difference to the controller, because it can't differenciate if a returned undefined is due to invalidity or just because that is the value that was passed in.

Thus, moving the parser added by badInputChecker() after Angular's numberInputType parser (which turns an empty value to null), will solve this problem whithout changing anything in case of invalid input. It will just fix this false alarm.

(BTW, the dateInputType might suffer from the same issue, because it too has `badInputChecker()` at the top.) **UPDATE:** It turns out that the dataInputTypes are not affected by this, as their $formatter makes sure the $viewValue is never undefined (but an empty string).

What do you think, you inputDirectives-savvy people ?
(/CCing @tbosch and @matsko who seem to be the last ones to have touched the relevant parts.)

@gkalpak
Copy link
Member

gkalpak commented Sep 19, 2014

This is a POC fiddle showing the problem going away (without breaking proper validation).
I use version 1.3.0-rc.2 with the only difference that I put the numberInputType parser after badInputChecker().

gkalpak added a commit to gkalpak/angular.js that referenced this issue Sep 20, 2014
…er] with observed attributes

When no value has been set for an input[number] it's viewValue is undefined. This lead to false
parse validation errors, when `ctrl.$validate()` was fired before interacting with the element. A
typical situation of this happening is when other directives on the same element `$observe()`
attribute values. (Such directives are ngRequired, min, max etc.)
More specifically, the parser for native HTML5 validation returns undefined when the input is
invalid and returns the value unchanged when the input is valid. Thus, when the value itself is
undefined, the NgModelController is tricked into believing there is a native validation error.

Closes angular#9106
gkalpak added a commit to gkalpak/angular.js that referenced this issue Sep 20, 2014
…er] with observed attributes

When no value has been set for an input[number] it's viewValue is undefined. This lead to false
'number' validation errors, when `ctrl.$validate()` was fired before interacting with the element.
A typical situation of this happening is when other directives on the same element `$observe()`
attribute values. (Such directives are ngRequired, min, max etc.)
More specifically, the parser for native HTML5 validation returns either undefined (when the input
is invalid) or the parsed value (when the input is valid). Thus, when the value itself is
undefined, the NgModelController is tricked into believing that there is a native validation error.

Closes angular#9106
@matsko
Copy link
Contributor

matsko commented Sep 23, 2014

@tbosch I'm assigning this to you. If you can't get around to it this week then I can have a look at the start of next week.

@caitp
Copy link
Contributor

caitp commented Sep 24, 2014

So this was broken by db044c4 as far as I can tell.

It doesn't really make any sense for us to be parsing model values, that's like the opposite of what model values are for. I don't think I can bug matsko about this right now, unfortunately :( @tbosch am I crazy? Is there a good reason why we are parsing values which should have already been parsed if they needed to be parsed?

edit I guess it really is parsing view values, just... the view value might not actually exist yet. :c

@tbosch
Copy link
Contributor

tbosch commented Oct 2, 2014

@caitp, yes, we are reparsing the view value here so that we can react to changes in $parsers and $formatters (e.g. used by ng-required when the expression for ng-required changes).

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