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

$error.number not working as expected. #4857

Closed
vdclouis opened this issue Nov 9, 2013 · 24 comments
Closed

$error.number not working as expected. #4857

vdclouis opened this issue Nov 9, 2013 · 24 comments

Comments

@vdclouis
Copy link

vdclouis commented Nov 9, 2013

Demo see official docs:
http://docs.angularjs.org/api/ng.directive:input.number

<form name="myForm" ng-controller="Ctrl">
  Number: <input type="number" name="input" ng-model="value" min="0" max="99" required>
  <span class="error" ng-show="myForm.input.$error.required">Required!</span>
  <span class="error" ng-show="myForm.input.$error.number">Not valid number!</span>
</form>

"Required!" is shown when there's nothing in the input field.
But "Not valid number!" is not when entering text, negative numbers or number greater than 99.

@petebacondarwin
Copy link
Contributor

Hmm. This is because the browser is displaying a "number" input box and it is returning undefined for its value. So the number validator never gets a chance to check its validity.

@petebacondarwin
Copy link
Contributor

There doesn't seem to be any way of accessing the actual value of the input box, that I can see.
We should be able to hook into the element.validity property, if available, which will have badinput set to true in this case.

@caitp
Copy link
Contributor

caitp commented Nov 9, 2013

one of my patches tries to address this (for the number input type), #4293 --- It's not totally perfect, but it might be a starting point to work around the issues which the ValidityState introduces

@ghost ghost assigned btford Dec 2, 2013
@btford
Copy link
Contributor

btford commented Dec 10, 2013

I'm reassigning this to @matsko since it's related to #4293.

@ghost ghost assigned matsko Dec 10, 2013
caitp added a commit to caitp/angular.js that referenced this issue Jan 23, 2014
In browsers where HTML5 constraint validation is (partially) implemented, an invalid number
entered into an input[type=number] (for example) input element would be visible to the
script context as the empty string. When the required or ngRequired attributes are not used,
this results in the invalid state of the input being ignored and considered valid.

To address this, a validator which considers the state of the HTML5 ValidityState object is
used when available.

Closes angular#4293
Closes angular#2144
Closes angular#4857
Closes angular#5120
Closes angular#4945
Closes angular#5500
caitp added a commit to caitp/angular.js that referenced this issue Jan 23, 2014
In browsers where HTML5 constraint validation is (partially) implemented, an invalid number
entered into an input[type=number] (for example) input element would be visible to the
script context as the empty string. When the required or ngRequired attributes are not used,
this results in the invalid state of the input being ignored and considered valid.

To address this, a validator which considers the state of the HTML5 ValidityState object is
used when available.

Closes angular#4293
Closes angular#2144
Closes angular#4857
Closes angular#5120
Closes angular#4945
Closes angular#5500
caitp added a commit to caitp/angular.js that referenced this issue Jan 23, 2014
In browsers where HTML5 constraint validation is (partially) implemented, an invalid number
entered into an input[type=number] (for example) input element would be visible to the
script context as the empty string. When the required or ngRequired attributes are not used,
this results in the invalid state of the input being ignored and considered valid.

To address this, a validator which considers the state of the HTML5 ValidityState object is
used when available.

Closes angular#4293
Closes angular#2144
Closes angular#4857
Closes angular#5120
Closes angular#4945
Closes angular#5500
caitp added a commit to caitp/angular.js that referenced this issue Jan 23, 2014
In browsers where HTML5 constraint validation is (partially) implemented, an invalid number
entered into an input[type=number] (for example) input element would be visible to the
script context as the empty string. When the required or ngRequired attributes are not used,
this results in the invalid state of the input being ignored and considered valid.

To address this, a validator which considers the state of the HTML5 ValidityState object is
used when available.

Closes angular#4293
Closes angular#2144
Closes angular#4857
Closes angular#5120
Closes angular#4945
Closes angular#5500
@tbosch tbosch modified the milestones: 1.2.12, 1.2.11, 1.2.13 Feb 3, 2014
@btford btford modified the milestones: 1.2.14, 1.2.13 Feb 15, 2014
caitp added a commit to caitp/angular.js that referenced this issue Feb 21, 2014
In browsers where HTML5 constraint validation is (partially) implemented, an invalid number
entered into an input[type=number] (for example) input element would be visible to the
script context as the empty string. When the required or ngRequired attributes are not used,
this results in the invalid state of the input being ignored and considered valid.

To address this, a validator which considers the state of the HTML5 ValidityState object is
used when available.

Closes angular#4293
Closes angular#2144
Closes angular#4857
Closes angular#5120
Closes angular#4945
Closes angular#5500
caitp added a commit to caitp/angular.js that referenced this issue Feb 21, 2014
In browsers where HTML5 constraint validation is (partially) implemented, an invalid number
entered into an input[type=number] (for example) input element would be visible to the
script context as the empty string. When the required or ngRequired attributes are not used,
this results in the invalid state of the input being ignored and considered valid.

To address this, a validator which considers the state of the HTML5 ValidityState object is
used when available.

Closes angular#4293
Closes angular#2144
Closes angular#4857
Closes angular#5120
Closes angular#4945
Closes angular#5500
caitp added a commit to caitp/angular.js that referenced this issue Feb 21, 2014
In browsers where HTML5 constraint validation is (partially) implemented, an invalid number
entered into an input[type=number] (for example) input element would be visible to the
script context as the empty string. When the required or ngRequired attributes are not used,
this results in the invalid state of the input being ignored and considered valid.

To address this, a validator which considers the state of the HTML5 ValidityState object is
used when available.

Closes angular#4293
Closes angular#2144
Closes angular#4857
Closes angular#5120
Closes angular#4945
Closes angular#5500
@caitp caitp closed this as completed in c2d447e Feb 21, 2014
@CocaColaGuy
Copy link

The validity function doesn't work either. What does work is adding the attr 'required'.

@caitp
Copy link
Contributor

caitp commented Feb 23, 2014

@CocaColaGuy, the validity stuff should work if you're on a modern browser --- if not, I'd be interested to hear which browser it's not working for

@CocaColaGuy
Copy link

i'm working on the latest chrome, windows 8...

@caitp
Copy link
Contributor

caitp commented Feb 23, 2014

Give an example of what you're doing? Post a plnkr... http://plnkr.co/edit/gmfs23yGqgGUxlqKzzEj?p=preview this is working just fine here (Chrome 33, OSX 10.9)

@caitp
Copy link
Contributor

caitp commented Feb 23, 2014

Interestingly, this is not working in FF29 right now, for some reason they are not exposing the ValidityState.badInput at all.. I think I'm going to file a bug re: that, since it's still in the draft -_--

It seems to be implemented in the browser http://mxr.mozilla.org/mozilla-central/source/content/html/content/src/ValidityState.cpp#80 but I can't read it in FF29, trying with FF30.

..yep, works in FF30. Okay then :c

@CocaColaGuy
Copy link

basically, this is it:
http://jsfiddle.net/U3pVM/3125/
here the validation of the a number doesn't even work... on my computer (the code is a bit longer) it actually does. note that in anywhere on the original code i'm not manipulating the code beyond what you see here. so it a bit wired, but adding 'required' fixed the problem for me.

@caitp
Copy link
Contributor

caitp commented Feb 23, 2014

@CocaColaGuy, you're using an ancient version of AngularJS in that fiddle... This fix hasn't been shipped in a release yet. Use http://code.angularjs.org/snapshot/angular.js

http://jsfiddle.net/9su9t/ the required validation runs prior to the html5 native validation, and seems to prevent the native validation from working, so we might want to fix that. But it may not be totally possible, because badInput + required still sets valueMissing.

@CocaColaGuy
Copy link

lol, unfortunately I'm only allowed to use production versions... but thx for the tip :-)

@starr0stealer
Copy link

I am running into this issue, can confirm it is fixed in the recent releases. But if the input is required and the user inputs a alpha value, only the required status is set. I would expect to be able to inform the user that they did not input a number valid. The form would still be in invalid state so we could still prevent them from submitting.
Would this be possible for it to provide $error.number during this use case, instead of $error.required ?

-- Edit
To speak further about this request. Currently both Firefox and Internet Explorer don't support input[number], and handle this validation correctly because it is treated as a Text input. This shows browser inconsistency which is something I believe large libraries such as Angular and jQuery aim to prevent.

@caitp
Copy link
Contributor

caitp commented Apr 15, 2014

@starr0stealer can you post an example of this with browser information you're testing with?

Unfortunately, since we rely on testing the bad-input state of the form control, it's very difficult to actually verify this with unit tests, so it's possible for any number of things to break with browser changes or angular changes, and we'd have no idea about it without testing manually. In Chrome for example, the unit tests checking the badInput handling has to ask for special privileges to do anything meaningful. It's really sad and I wish it were better :( But from what I recall, this was working more or less as expected a few months ago, so I'd be interested in seeing how it's broken.

@starr0stealer
Copy link

@caitp Unsure what your requesting from me. To see the issue you can check this Plunker below.

http://plnkr.co/edit/8fqFRuofIKcbF7KhwrMz?p=preview

In Chrome it only throws $error.required, but in FIrefox it throws $error.number. Because its treated as a Text input, and a value has been provided so the required phase does result correctly. To me even in Chrome any value provided should count toward the required state, regardless of its input validity

@caitp
Copy link
Contributor

caitp commented Apr 15, 2014

@starr0stealer The reason for that is because ng-required doesn't care about the native validity state (although it certainly could). Want to send a PR to get it looking at the validity state if it exists? I thought it was already doing that, but I guess not. (Note that some browsers may also consider valueMissing to be true when the value is invalid)

As a side point, the latest FF Nightly (and interestingly the Nightly from whenever the last time I updated was) does handle input[type=number] --- But it's funny, their implementation of ValidityState seems to be broken in some ways (nothing major, just properties are not enumerable, so JSON.stringify produces nonsense output), so yay, bug filed on that. Not very important to this issue, just sort of interesting if you enjoy web standards-y stuff.

@starr0stealer
Copy link

@caitp I'll see what I can pull together. I think Angular is indeed looking at the validity object of the input (going by what I see in the code). Inspecting with Chrome on a normal Input[number] without any AngularJS, inputting an Alpha value results in the values below. Within normal browser behavior it looks like the badInput takes precedence over valueMissing.

validity: ValidityState
badInput: true
customError: false
patternMismatch: false
rangeOverflow: false
rangeUnderflow: false
stepMismatch: false
tooLong: false
typeMismatch: false
valid: false
valueMissing: true

@caitp
Copy link
Contributor

caitp commented Apr 15, 2014

@starr0stealer it is not taking the validity state into consideration when dealing with the required directive:

https://github.com/angular/angular.js/blob/master/src/ng/directive/input.js#L2062-L2070

var validator = function(value) {
  if (attr.required && ctrl.$isEmpty(value)) {
    ctrl.$setValidity('required', false);
    return;
  } else {
    ctrl.$setValidity('required', true);
    return value;
  }
};

@starr0stealer
Copy link

@caitp Ah, I hadn't finished reviewing the code. I just seen the sections making calls to element.prop('validity'), as well as the addNativeHtml5Validators method.
May get tricky here, because even if it looks at the validity object solely for required state, it will do the same thing. Logic will need to be added to make sure badInput doesn't exist for checking the required state.

Maybe it can be as simple as checking for the input type in the requiredDirective method, if number check that badInput is false.

@caitp
Copy link
Contributor

caitp commented Apr 15, 2014

I think that makes things a bit complicated. There should be a standard order of operations for determining if valueMissing "really" means valueMissing, I think. If there isn't, someone should probably file a bug on the spec. (although if valueMissing isn't supposed to be truthy when badInput occurs, then someone ought to file a bug on the browsers implementing it wrong instead). Realistically, ValidityState should make all this stuff easy for JS, we shouldn't have to care about distinguishing between valueMissing or badInput.

@starr0stealer
Copy link

I see your point. It would indeed be best that the requiredDirective method would only need to check validity.valueMissing value. Looks like I'll need to try and dig up why Chrome decided to set both badInput and valueMissing but only raises the 'Please enter a number' message.

@starr0stealer
Copy link

I was not able to find any existing issue opened about this subject (both badInput and valueMissing set to true), so I opened a new issue on the Chromium project (link below). With that said I believe the requiredDirective method still needs to be changed to use the validity object otherwise once the outlined issue is fixed within Chrome we will still be stuck at square one.

@caitp Would you still wont a PR for this? Regarding changes for the required state check to use validity object rather than angular.isEmpty

https://code.google.com/p/chromium/issues/detail?id=363816

@caitp
Copy link
Contributor

caitp commented Apr 15, 2014

Yeah, we should fix this =)

@starr0stealer
Copy link

Alright I will get onto making the change and then make sure the test for required still work. Thank you for your time and prompt responses. Great community for a great library :)

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.

8 participants