fix(input): use ValidityState to determine validity #5944
Conversation
|
Well I've been trying to get a protractor test to work for this, but it is the same issue as with the jasmine tests, the browser won't update validity as far as I can tell in response to WebElement.sendKeys(). It's my first attempt at a protractor test, so I might be doing something wrong, but they really make this difficult to work with. |
| @@ -431,7 +452,7 @@ function textInputType(scope, element, attr, ctrl, $sniffer, $browser) { | |||
| value = trim(value); | |||
| } | |||
|
|
|||
| if (ctrl.$viewValue !== value) { | |||
| if (ctrl.$viewValue !== value || (validity && !value && !validity.valueMissing)) { | |||
There was a problem hiding this comment.
If there's no value and we don't have a valueMissing error, we need to run the validators again to make sure it's not broken.
We only want to do this when there's no value and we don't have a valueMissing error, because if we went from empty string to empty string, an ngRequired handler would have already dealt with the issue --- so it is probably a different ValidityState error.
There was a problem hiding this comment.
we should condense this comment and add it to the code
|
@caitp I put together a plunk which (I think) demonstrates that the Caveat: I only tested the plunk in Chrome. |
|
@atdrago that's essentially what this patch does. The problem is, without changes like the ones in this patch, we require an ngRequired or required attribute in order to get an error at all, and the error that gets reported does not include the number error, and so this is not actually acceptable. After working on some blink forms tests last week though, I think I might have an idea of how to make this change testable, so I'll see if I can make this mergeable. ... nope, unfortunately badInput needs to rely on browser events, and browserTrigger isn't quite enough. So this can't really be tested outside of the browser, currently :( |
|
At the moment, writing an automated test for this is impossible unfortunately (using both webdriver and mocking). But we do need to make this work, otherwise continuing to have input[type=number] support is pretty pointless. Someone should look at this for 1.2.12 |
|
Hello, |
|
@flicus I believe this has been postponed until 1.2.13. If you would like to use a 1.2.11 fork that has @caitp's changes merged in, you can use https://github.com/quicksnap/angular.js/tree/1.2.11-QUICKSNAP -- it also has a build checked in for bower purposes. I'm just using it as a workaround until this PR is in core. |
|
1.2.13 has the same behavior. |
|
Yes, we haven't merged this yet :p |
| @@ -404,7 +404,28 @@ function validate(ctrl, validatorName, validity, value){ | |||
| return validity ? value : undefined; | |||
| } | |||
|
|
|||
|
|
|||
| function validateHtml5(ctrl, validatorName, element) { | |||
| var v = element.prop('validity'); | |||
There was a problem hiding this comment.
please don't use single char variables. it makes the code hard to follow. validity or validityState would be more appropriate
|
some minor naming changes requested, but otherwise lgtm |
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
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.
It is not entirely possible to test this outside of an E2E scenario, because browsers (or at least Chrome) do not seem to update validity in response to artificial events.
If I can get some help writing E2E tests for this, then I'll be more confident that this is an effective solution.
In the mean time, I'll put together another demo. DEMO
Closes #4293
Closes #2144
Closes #4857
Closes #5120
Closes #5500