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

fix(ngModel): fix issues when parserName is same as validator key #10850

Closed
wants to merge 2 commits into from

Conversation

Narretz
Copy link
Contributor

@Narretz Narretz commented Jan 23, 2015

For $validate(), it is necessary to store the parseError state
in the controller. Otherwise, if the parser name equals a validator
key, $validate() will assume a parse error occured if the validator
is invalid.

Also, setting the validity for the parser now happens after setting
validity for the validator key. Otherwise, the parse key is set,
and then immediately afterwards the validator key is unset
(because parse errors remove all other validations).

Fixes #10698
Closes #10828

For $validate(), it is necessary to store the parseError state
in the controller. Otherwise, if the parser name equals a validator
key, $validate() will assume a parse error occured if the validator
is invalid.

Also, setting the validity for the parser now happens after setting
validity for the validator key. Otherwise, the parse key is set,
and then immediately afterwards the validator key is unset
(because parse errors remove all other validations).

Fixes angular#10698
Closes angular#10850
@@ -518,7 +519,7 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$

// Check if the there's a parse error, so we don't unset it accidentially
var parserName = ctrl.$$parserName || 'parse';
Copy link
Contributor

Choose a reason for hiding this comment

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

This variable doesn't seem to be used now. What does this mean for the default of 'parse'?

Copy link
Contributor

Choose a reason for hiding this comment

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

I see that the errorKey variable sets the default below on line 563

@Narretz
Copy link
Contributor Author

Narretz commented Feb 8, 2015

I updated the test. I removed the variables that handle the validity of parser / validators. Instead, they now directly react to the input.
I didn't use the even / odd / largerThanTen scheme, because the parser and one validator need to have the same name, but would have validated different things. This is imo even more confusing. Now it's very verbose, but also quite clear what is tested.

If you're okay with the test, can we merge this, and push the parseName refactor back? The bug is pretty gnarly and we can fix the parser name later.

@@ -1221,6 +1221,96 @@ describe('ngModel', function() {
expect(ctrl.$validators.mock).toHaveBeenCalledWith('a', 'ab');
expect(ctrl.$validators.mock.calls.length).toEqual(2);
});

iit('should validate correctly when $parser name equals $validator key', function() {
Copy link
Contributor

Choose a reason for hiding this comment

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

iit

petebacondarwin pushed a commit to petebacondarwin/angular.js that referenced this pull request Feb 12, 2015
For $validate(), it is necessary to store the parseError state
in the controller. Otherwise, if the parser name equals a validator
key, $validate() will assume a parse error occured if the validator
is invalid.

Also, setting the validity for the parser now happens after setting
validity for the validator key. Otherwise, the parse key is set,
and then immediately afterwards the validator key is unset
(because parse errors remove all other validations).

Fixes angular#10698
Closes angular#10850
petebacondarwin added a commit that referenced this pull request Feb 13, 2015
For $validate(), it is necessary to store the parseError state
in the controller. Otherwise, if the parser name equals a validator
key, $validate() will assume a parse error occured if the validator
is invalid.

Also, setting the validity for the parser now happens after setting
validity for the validator key. Otherwise, the parse key is set,
and then immediately afterwards the validator key is unset
(because parse errors remove all other validations).

Fixes #10698
Closes #10850
Closes #11046
@petebacondarwin
Copy link
Contributor

Oh! Sorry @Narretz - when I squashed I attributed it to me instead of you. I am sorry about that - I will not do it again. Great work.

netman92 pushed a commit to netman92/angular.js that referenced this pull request Aug 8, 2015
For $validate(), it is necessary to store the parseError state
in the controller. Otherwise, if the parser name equals a validator
key, $validate() will assume a parse error occured if the validator
is invalid.

Also, setting the validity for the parser now happens after setting
validity for the validator key. Otherwise, the parse key is set,
and then immediately afterwards the validator key is unset
(because parse errors remove all other validations).

Fixes angular#10698
Closes angular#10850
Closes angular#11046
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.