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

fix(ngModel): don't parse before validating if viewValue is undefined #9260

Closed
wants to merge 1 commit into from

Conversation

caitp
Copy link
Contributor

@caitp caitp commented Sep 24, 2014

Prevents unintentionally reporting an undefined view value as a parse error...

Closes #9106

@caitp
Copy link
Contributor Author

caitp commented Sep 24, 2014

@tbosch this should be really quick to review, can you take a quick look?


scope.$apply("required = false");

expect(inputElm).toBeValid();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this assertion does fail without this change

@caitp caitp changed the title fix(input): don't parse if viewValue is undefined fix(ngModel): don't parse if viewValue is undefined Sep 24, 2014
@caitp caitp changed the title fix(ngModel): don't parse if viewValue is undefined fix(ngModel): don't parse before validating if viewValue is undefined Sep 24, 2014
var parserValid = isUndefined(modelValue) ? undefined : true;

if (parserValid) {
for(var i = 0; i < ctrl.$parsers.length; i++) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ii = ctrl.$parsers.length ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are no tests, but it's technically possible for the parsers array to grow while a parser is running, so changing that could be a breaking change.

We can do it but it might not be a good commit to do it in

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fair enough

@btford
Copy link
Contributor

btford commented Sep 24, 2014

LGTM

@caitp caitp closed this in 92f05e5 Sep 24, 2014
@caitp
Copy link
Contributor Author

caitp commented Sep 24, 2014

thanks, that is one less bug on the checklist \o/

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" invalid on load if ng-required is used
3 participants