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

fix(input): determine input[type=number] validity with ValidityState #4293

Closed
wants to merge 1 commit into from

Conversation

caitp
Copy link
Contributor

@caitp caitp commented Oct 6, 2013

Fixes #2144

What happens:

<form name="form">
  <input type="number" ng-model="value" name="value" />
  Number is valid: {{form.value.$valid}}
</form>
  • Chromium / Safari / Opera
    1. Non-numeric evaluates to empty string
    2. Empty-string is considered valid unless require flag is set
    3. False positive: invalid number reported as valid
  • Firefox / Internet Explorer
    1. Non-numeric string entered, string is not transformed by native code
    2. Regex tests string for validity
    3. Regex test fails, invalid number reported as invalid

This patch should:

  1. Listen to input's ValidityState on browsers which implement ValidityState
  2. Allow the differentiation between "invalid" empty strings and "valid" empty strings
  3. Not need require flag to validate number inputs correctly.
  4. Provide (kind of gross) framework for adding ValidityState support to other InputTypes

@mary-poppins
Copy link

Thanks for the PR!

  • Contributor signed CLA now or in the past
    • If you just signed, leave a comment here with your real name
  • PR's commit messages follow the commit message format

If you need to make changes to your pull request, you can update the commit with git commit --amend.
Then, update the pull request with git push -f.

Thanks again for your help!

@caitp caitp closed this Oct 6, 2013
@caitp caitp reopened this Oct 6, 2013
@caitp
Copy link
Contributor Author

caitp commented Oct 6, 2013

Okay, I've hacked around with this for a bit... Basically when setting up the textInputType stuff, I pass in a boolean asking it to compare the ValidityState to a known/cached value, and if so, run through the steps -- even if the value appears to not have changed (due to being reported as the empty string by Blink/WebKit).

This doesn't break any of the other validators due to being an optional thing. So I think this works okay, and it's even sort of testable.

Here's a working demo

@caitp
Copy link
Contributor Author

caitp commented Oct 7, 2013

The problem with this is, it doesn't set the model to NULL when it's determine to be invalid for whatever reason (and this can be seen in the test --- where I'm not checking that the model value is falsy).

I'm not totally sure why this is, if someone could clear that up then I think this would work pretty much perfectly.

On Chromium / Safari / Opera, currently:

When a non-numeric string is entered into an input[type=number], the browser
reports the value and innerText as the empty string.

Without the ngRequire directive, the empty string is considered a valid value.

This leads to false-positives.

The aim of this patch is to suppress these false positives on browsers which
implement the `ValidityState` object. Input directives may subscribe to check
for `ValidityState` changes, and use the ValidityState during their processing.

This allows the differentiation between "valid" and "invalid" empty strings.

Closes angular#2144
@IgorMinar
Copy link
Contributor

I'm sorry, but I wasn't able to verify your CLA signature. CLA signature is required for any code contributions to AngularJS.

Please sign our CLA and ensure that the CLA signature email address and the email address in this PR's commits match.

If you signed the CLA as a corporation, please let me know the company's name.

Thanks a bunch!

PS: If you signed the CLA in the past then most likely the email addresses don't match. Please sign the CLA again or update the email address in the commit of this PR.
PS2: If you are a Googler, please sign the CLA as well to simplify the CLA verification process.

@IgorMinar
Copy link
Contributor

CLA signature verified! Thank you!

Someone from the team will now triage your PR and it will be processed based on the determined priority (doc updates and fixes with tests are prioritized over other changes).

@caitp
Copy link
Contributor Author

caitp commented Dec 20, 2013

It would be good to get this into 1.2.7, but unfortunately I can't get (more meaningful) tests passing on Chrome. I can't seem to get the ValidityState to change on input/change events, so that kind of sucks.

But, it does work in a real browser, outside of jasmine/ngMocks so hey ._. I dunno.

@georgantasp
Copy link

+1

caitp added a commit to caitp/angular.js that referenced this pull request 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
Copy link
Contributor Author

caitp commented Jan 23, 2014

@matsko I've written a new version of this pull request since it was difficult to deal with the merge conflicts for this one. I think the new implementation is cleaner and less expensive, too, so it's probably a good thing. I think this issue should be closed in favour of #5944

caitp added a commit to caitp/angular.js that referenced this pull request 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 pull request 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
@matsko
Copy link
Contributor

matsko commented Jan 23, 2014

@caitp Thank you!

caitp added a commit to caitp/angular.js that referenced this pull request 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
@matsko
Copy link
Contributor

matsko commented Jan 23, 2014

Closed in favour of #5944

@matsko matsko closed this Jan 23, 2014
caitp added a commit to caitp/angular.js that referenced this pull request 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 pull request 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 pull request 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 that referenced this pull request 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 #4293
Closes #2144
Closes #4857
Closes #5120
Closes #4945
Closes #5500
Closes #5944
@pedubreuil
Copy link

Hi !
It seems that the problem is back with the version v1.2.15-build.2364+sha.7c34e1f : myform.numericfield.$error.number is still not updated

Thanks in advance !

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

Successfully merging this pull request may close these issues.

input[type="number"] is valid when not a number is entered in Chrome
7 participants