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

fix(ngModel): validate pattern against the viewValue #12640

Closed
wants to merge 1 commit into from

Conversation

Narretz
Copy link
Contributor

@Narretz Narretz commented Aug 20, 2015

Since the HTML5 pattern validation constraint validates the input value,
we should also validate against the viewValue. This allows e.g. input[date]
to validate both the input format of the date and the date object itself.

Fixes #12344

(Also needs to be backported to 1.3)

@lgalfaso
Copy link
Contributor

Would it be possible to know when this broke? Would like to know this to know to what extend this is a regression vs a breaking change.

@Narretz
Copy link
Contributor Author

Narretz commented Aug 20, 2015

It's not completely clear-cut. It worked last in 1.2.x, see #12618. In 1.3.x we changed two things: validation is done in the $validators, not the $parsers, and input[number/date] parse the input to Number/Date, so validating the pattern against the modelValue doesn't work anymore. That's the regression. The potential bc is that pattern now explicitly validates the viewValue, not the modelValue.

I actually had a bc note with a workaround for the latter case, but I removed it because the actual use case being broken is a regression. We can add it as a potential breaking change:



it('should validate the viewValue and not the modelValue', function() {
var inputElm = helper.compileInput('<input type="text" name="test" ng-model="value" pattern="\\d{4}$">');
Copy link
Member

Choose a reason for hiding this comment

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

I suspect this is a copy-paste from the previous test, but the trailing $ doesn't seem to be necessary (neither here nor in other tests).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, you got me. Guilty of c&p! ;)

@Narretz Narretz force-pushed the fix-pattern-no-bc branch from 459daca to 4a02403 Compare August 21, 2015 10:18
@lgalfaso
Copy link
Contributor

I am ok for this landing into the 1.4 and 1.3 branches with a BC notice. If you think this is not needed, then ping @petebacondarwin

@Narretz Narretz force-pushed the fix-pattern-no-bc branch 3 times, most recently from cd7be20 to a363bcd Compare August 22, 2015 20:05
Since the HTML5 pattern validation constraint validates the input value,
we should also validate against the viewValue. While this worked in
core up to Angular 1.2, in 1.3, we changed not only validation,
but the way `input[date]` and `input[number]` are handled - they parse
their input values into `Date` and `Number` respectively, which cannot
be validated by a regex.

Fixes angular#12344

BREAKING CHANGE:

The `ngPattern` and `pattern` directives will validate the regex
against the `viewValue` of `ngModel`, i.e. the value of the model
before the $parsers are applied. Previously, the modelValue
(the result of the $parsers) was validated.

This fixes issues where `input[date]` and `input[number]` cannot
be validated because the viewValue string is parsed into
`Date` and `Number` respectively (starting with Angular 1.3).
It also brings the directives in line with HTML5 constraint
validation, which validates against the input value.

This change is unlikely to cause applications to fail, because even
in Angular 1.2, the value that was validated by pattern could have
been manipulated by the $parsers, as all validation was done
inside this pipeline.

If you rely on the pattern being validated against the modelValue,
you must create your own validator directive that overwrites
the built-in pattern validator:

```
.directive('patternModelOverwrite', function patternModelOverwriteDirective() {
  return {
    restrict: 'A',
    require: '?ngModel',
    priority: 1,
    compile: function() {
      var regexp, patternExp;

      return {
        pre: function(scope, elm, attr, ctrl) {
          if (!ctrl) return;

          attr.$observe('pattern', function(regex) {
            /**
             * The built-in directive will call our overwritten validator
             * (see below). We just need to update the regex.
             * The preLink fn guaranetees our observer is called first.
             */
            if (isString(regex) && regex.length > 0) {
              regex = new RegExp('^' + regex + '$');
            }

            if (regex && !regex.test) {
              //The built-in validator will throw at this point
              return;
            }

            regexp = regex || undefined;
          });

        },
        post: function(scope, elm, attr, ctrl) {
          if (!ctrl) return;

          regexp, patternExp = attr.ngPattern || attr.pattern;

          //The postLink fn guarantees we overwrite the built-in pattern validator
          ctrl.$validators.pattern = function(value) {
            return ctrl.$isEmpty(value) ||
              isUndefined(regexp) ||
              regexp.test(value);
          };
        }
      };
    }
  };
});
```
@Narretz
Copy link
Contributor Author

Narretz commented Aug 22, 2015

Updated - please take a look

@petebacondarwin
Copy link
Contributor

LGTM!!

@lgalfaso
Copy link
Contributor

LGTM!

@gkalpak
Copy link
Member

gkalpak commented Aug 27, 2015

Landed as 0e00108.

@gkalpak gkalpak closed this Aug 27, 2015
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.

5 participants