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

View value not updated after $parsers run when allowInvalid: true #10126

Closed
trevcor opened this issue Nov 19, 2014 · 4 comments
Closed

View value not updated after $parsers run when allowInvalid: true #10126

trevcor opened this issue Nov 19, 2014 · 4 comments

Comments

@trevcor
Copy link

trevcor commented Nov 19, 2014

When creating a form that has the setting allowInvalid: true I noticed that I was getting false positives for ng-minlength validation. I have a directive that adds a parser that filters out invalid characters that the user may try to input. In the example below '!' is an invalid character. If the user types in 'a!' the view is updated to just 'a' but minlength=2 returns true.

This happening because this.$$parseAndValidate sets the viewValue variable before the parsers run and never updates it. This cause the original viewValue to be passed to the validators. So the minlength validator sees 'a!' instead of the current value of 'a'.

I was able to correct this behavior by updating the first line of the parsers loop to

modelValue = viewValue = ctrl.$parsers[i](modelValue);

This keeps the viewValue variable in sync but I'm unclear what side effects this might cause. Any advice is appreciated.

I'm using v1.3.0-rc.1 but I see the same behavior in 1.3.3 which is what I used for the fiddle.

Here is a example of this behavior.
http://jsfiddle.net/bq9r7b98/

@Narretz
Copy link
Contributor

Narretz commented Nov 19, 2014

Angular is not included in your jsfiddle. Can you please fix it?

It looks like the check inside $setViewValue is assuming that ngModelOptions is only used for debouncing / triggering viewValue commits, but since we added allowInvalid this doesn't work anymore.

However, $parsers are not designed to update the viewValue, and they shouldn't do that automatically, so we cannot use the fix you provided.

In the meantime you should be able to get your result by adding $commitViewValue after $setViewValue.

@trevcor
Copy link
Author

trevcor commented Nov 20, 2014

Sorry about that. Here is the correct version of the fiddle.

http://jsfiddle.net/bq9r7b98/4/

Can you help me understand why $parsers shouldn't change the view value? If there is a better solution for blocking the user from inserting certain characters into an input I would like to update my code. I've used this pattern for awhile but it does seem to create a lot of overhead.

I'll try adding $commitViewValue in the meantime.

Thanks

@Narretz
Copy link
Contributor

Narretz commented Nov 20, 2014

Can you help me understand why $parsers shouldn't change the view value?

It's a design decision. They were previously overloaded with validation, but this was changed, too. So we shouldn't put anything into core that guarantees updatingthe viewValue when parsing simply because it would mix concerns, but you are of course free to do it, simply because there is currently no better way. In the future, there will maybe be a better way to do it.

@pkozlowski-opensource pkozlowski-opensource modified the milestones: 1.3.5, 1.3.6 Dec 1, 2014
Narretz added a commit to Narretz/angular.js that referenced this issue Dec 5, 2014
…dation

This fixes issues where a parser calls $setViewValue. This is a common
strategy for manipulating the $viewValue while the user is entering
data into an input field.

When the $viewValue was changed inside the parser, the new viewValue
would be committed, parsed and used for validation. The original parser
however would run after that and pass the original (outdated) viewValue
on to the validators, which could cause false positives, e.g. for
minlength.

Fixes angular#10126
Fixes angular#10299
@shahata
Copy link
Contributor

shahata commented Dec 5, 2014

BTW, I'd recommend also invoking ngModelCtrl.$commitViewValue() in such parsers so that the value will be committed even if ng-model-options was used to debounce or update on some trigger.

ngModelCtrl.$setViewValue(clean);
ngModelCtrl.$commitViewValue();
ngModelCtrl.$render();

Narretz added a commit to Narretz/angular.js that referenced this issue Dec 5, 2014
…dation

This fixes issues where a parser calls $setViewValue. This is a common
strategy for manipulating the $viewValue while the user is entering
data into an input field.

When the $viewValue was changed inside the parser, the new viewValue
would be committed, parsed and used for validation. The original parser
however would run after that and pass the original (outdated) viewValue
on to the validators, which could cause false positives, e.g. for
minlength.

Fixes angular#10126
Fixes angular#10299
@Narretz Narretz closed this as completed in 2d6a0a1 Dec 5, 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

4 participants