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

Input validation with ng-required function not compatible across different browsers. #4945

Closed
jonricaurte opened this issue Nov 13, 2013 · 11 comments

Comments

@jonricaurte
Copy link

I have a form with input validation that also uses an ng-required function to determine if the input is required.

http://plnkr.co/edit/sBSkkaR6UCPodKxlQBf7?p=preview

Steps to reproduce:

  1. Type "a" into input
  2. delete "a" from input

In angular 1.1.5, after you delete "a" from input you can see that in chrome validateCharacters shows undefined and then in the required function shows an empty string. However, in Firefox and IE there is an empty string in validateCharacters and then in the required function it is undefined. If you update to angular 1.2, then Firefox and Chrome are the same but IE is still the old way:

http://plnkr.co/edit/trECTGaXRo5QVWeYTTdV

Is there any workaround for this?

@jimmy-collazos
Copy link

I think the problem is in validators methods (email, required, minlength, number, ...) because these methods return undefined if is invalid.

Check flow of validations methods (email by example) :
https://github.com/angular/angular.js/blob/g3_v1_2/src/ng/directive/input.js#L622-L634

var emailValidator = function(value) {
    if (ctrl.$isEmpty(value) || EMAIL_REGEXP.test(value)) {
      ctrl.$setValidity('email', true);
      return value;
    } else {
      ctrl.$setValidity('email', false);
      return undefined;
    }
  };
ctrl.$formatters.push(emailValidator);
ctrl.$parsers.push(emailValidator);

https://github.com/angular/angular.js/blob/g3_v1_2/src/ng/directive/input.js#L1151-L1153

forEach(this.$parsers, function(fn) {
      value = fn(value);
    });

I think it is a mistake to return an undefined values in $parses for input (element) values, because input value always is a string

@caitp
Copy link
Contributor

caitp commented Dec 4, 2013

If it is a mistake, are you proposing a solution which would enable your use cases without breaking other ones?

(Also it should be noted that the ngRequired directive does not work the way it's being used in @jonricaurte's plnkr: https://github.com/angular/angular.js/blob/master/src/ng/directive/input.js#L1314, perhaps some documentation needs to be updated)

@jonricaurte
Copy link
Author

Thanks for the responses. @caitp Could you clarify how it is not being used correctly? It seems like you should be able to have a function for ng-required to dynamically change whether or not an input is required. An example of a use case for this would be if you were filling out an address form and some countries require a state field and some don't. Here is a similar example:

http://plnkr.co/edit/MXROFfHABF0kWjuvGPJ1?p=preview

So the function bound to ng-required always returns true. In Firefox, if you type in "a" then delete "a" and then click show text, you will get undefined. This should return an empty string since the validate-characters directive checks to see if the value is undefined and if it is, returns an empty string. In Firefox's case, in validate-characters it is an empty string and is not undefined so an empty string is returned. However, if you click "show text" search.test is undefined.

@caitp
Copy link
Contributor

caitp commented Dec 4, 2013

@jonricaurte: $scope.$observe() will interpolate a string --- it does not $eval an expression. The result of the interpolation is always a string (and false.toString() does not evaluate to false, most things don't. The only strings that come to mind that would evaluate to false are the empty string and numbers with the value 0)

So, with that in mind, having the function is not going to do what you think it will --- It never gets evaluated by the directive or anything (it does get evaluated twice during bootstrapping, for whatever reason)

It should work if you have an expression like {{test()}} where test returns the empty string, or something which would evaluate to the empty string.

These comments weren't really regarding the issue itself, more a comment on your code.

However, it is likely that the cross-browser weirdness you're seeing with validation is related to http://dev.w3.org/html5/spec-preview/constraints.html, as they've decided that invalid values (such as an empty string with the required attribute set, added by ng-required) will cause us to see undefined, rather than a proper value. Unfortunately there isn't anything that can be done about this unless someone convinces the W3/WHATWG to change this (I've heard that there have been attempts to do this, which were not well received).

I've got a patch to try and work around this particular issue for the [input=number] type, which may or may not be merged some day #4293, you're welcome to see if you can get a similar solution to work for your stuff, and offer any criticism or review of that patch in particular if you see anything I've done rather stupidly ;)

@jonricaurte
Copy link
Author

@caitp In this example it looks like it is evaluating the function:

http://plnkr.co/edit/3NtsgZPCz55j3QHzWFgt?p=preview

The function returns true if there is something written in either textbox and then applies the css box-shadow. If there is nothing, it doesn't apply any css. This is in Chrome.

@jonricaurte
Copy link
Author

It seems like in the latest version of angular something was fixed and now Firefox returns an empty string. It works sometimes in IE, but sometimes doesn't.

@caitp
Copy link
Contributor

caitp commented Dec 4, 2013

It only evaluates the function during bootstrap, not during every digest. For one thing, it's not even $observing the ngRequired attribute, it's only observing required --- thus even if the value of the ngRequired expression changed, it would have no effect as far as I can tell.

But again, this is not really the point of the comment, it's a nitpick about misuse of the directive.

If the browser decides that an input is invalid with browser constraint validation, the parsers see undefined, in every browser which supports constraint validation. This is why I expect that your problem is related to the link I pasted above.

@jonricaurte
Copy link
Author

Ok sorry for going back and forth between two different things. The undefined I understand that it could be for browsers which support constraint validation and angular must handle it differently in later versions for it to perform differently in Firefox (haven't been able to look to see where the changes are). I sort of switched to a different problem relating to ng-required. You are right its not observing ngRequired, however the observe does get fired if you put a breakpoint in the plunker. If you put a breakpoint in angular.js at line 16455 and 16468 you will see the observe is getting fired and recalling the function. However, if I take out the validate-characters directive, the observe doesn't get fired:

@jonricaurte
Copy link
Author

http://plnkr.co/edit/YG91eM8dHSvhEBI3NVGZ?p=preview

Sorry for the split comment.

@jonricaurte
Copy link
Author

Where the observe is getting fired is in this plunker on those lines:

http://plnkr.co/edit/7dRK8HxHG6GpQLIneWOt?p=preview

@jonricaurte
Copy link
Author

Interesting. In the last plunker, the flow is:

  1. In angular.js, it hits the ctrl.$parsers.unshift with the view value "a"
  2. In app.js, it hits the ctrl.$parsers.unshift with a view value "a"
  3. In app.js, it hits the $scope.test function that is bound to ng-required and returns true
  4. In angular.js the attr.$observe('required' is fired with the view value "a"
  5. In app.js, it hits the $scope.test function that is bound to ng-required and returns true
  6. In angular.js the attr.$observe('required' is fired with the view value undefined

@ghost ghost assigned tbosch Jan 9, 2014
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
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
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants