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

ng-pattern binding is not working in v1.4 #12344

Closed
aquisio opened this issue Jul 14, 2015 · 10 comments
Closed

ng-pattern binding is not working in v1.4 #12344

aquisio opened this issue Jul 14, 2015 · 10 comments

Comments

@aquisio
Copy link

aquisio commented Jul 14, 2015

I understand that v1.3 introduced some changes in the recommended usage of ng-pattern when the validation pattern / RegExp is exposed as a model property. This change is documented in the migration notes and works correctly in v1.3, as demonstrated in this JSFiddle.

However, using the same implementation in v1.4 does not work, as this JSFiddle shows. Instead, pattern validation always fails.

@gkalpak
Copy link
Member

gkalpak commented Jul 14, 2015

It doesn't actually have to do with ngPattern as much as it has to do with input[type="date"].

@aquisio: the 1.3.0-beta.8 version of Angular that you where using, didn't have support for date inputs, which means that they where treated as text inputs (i.e. reading the value from the input as the browser sets it). Per the spec, date input values are use the yyyy-MM-dd format, so ngPattern can match the string against the regex.
Now Angular has support for date inputs and has logic in place in order to convert the yyyy-MM-dd string value to an actual Date object. ngPattern tries to match the Date object to the regex (and fails).

@team (especially forms people): There seems to be a need for custom handling of pattern/ngPattern on date inputs (date, datetime-local, week, month).
We do this for min/ngMin and max/ngMax anyway.

@Narretz
Copy link
Contributor

Narretz commented Jul 14, 2015

hm. ngPattern should probably only try to match the viewValue, not the modelValue. Because that's what pattern does.

@Narretz Narretz added this to the Backlog milestone Jul 14, 2015
@gkalpak
Copy link
Member

gkalpak commented Jul 14, 2015

Thinking about it again, you can't convert a regex to a date for comparing with the $modelValue :)

So, for the date input it does make sense for ngPattern to compare against the $viewValue.
(Not so sure about other inputs though, if that's what you meant.)

kacecode added a commit to kacecode/angular.js that referenced this issue Aug 20, 2015
kacecode added a commit to kacecode/angular.js that referenced this issue Aug 20, 2015
Set patternDirective validator to test against viewValue instead of
modelValue.

Closes angular#12344, angular#12618
Narretz added a commit to Narretz/angular.js that referenced this issue 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 angular#12344
Narretz added a commit to Narretz/angular.js that referenced this issue 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 angular#12344
Narretz added a commit to Narretz/angular.js that referenced this issue Aug 21, 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 angular#12344
Narretz added a commit to Narretz/angular.js that referenced this issue Aug 22, 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.

Until Angular 1.3, the validated value was

Fixes angular#12344

BREAKING CHANGE:

`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]` could not be validated because the viewValue string
had been parsed into `Date`/`Number` respectively. This change also
brings the directives in line with HTML5 constraint validation,
which validates against the value of the input.

The change is unlikely to cause applications to fail, because even
before Angular 1.3, the value that was validated by pattern could have
been manipulated by the $parsers array, 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 added a commit to Narretz/angular.js that referenced this issue Aug 22, 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 angular#12344

BREAKING CHANGE:

`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]` could not be validated because the viewValue string
had been parsed into `Date`/`Number` respectively. This change also
brings the directives in line with HTML5 constraint validation,
which validates against the value of the input.

The change is unlikely to cause applications to fail, because even
before Angular 1.3, the value that was validated by pattern could have
been manipulated by the $parsers array, 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 added a commit to Narretz/angular.js that referenced this issue Aug 22, 2015
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 added a commit to Narretz/angular.js that referenced this issue Aug 22, 2015
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 added a commit that referenced this issue Aug 28, 2015
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 #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);
          };
        }
      };
    }
  };
});
```
@everdimension
Copy link

I think this was a stupid decision to make.

It completely brakes the idea of ngModel's $parsers and $validators pipeline.

Imagine I have a mask directive applied to the input to prettify some digital value. The masking directive formats the input data with some dashes and spaces, displaying it in the desired format, but surely it saves the $modelValue as a stripped string without any unnecessary symbols, only numbers.

I might want to apply a pattern attribute which would specify a regex to accept only numbers. Because why not! Maybe my masking directive is written to allow old-fashioned phone numbers with both letters and digits, but i want this particular input element to allow only digits. My mask directive combined with the pattern directive would be perfect together, because pattern directive would be checking the value returned from the masking directive.

And you broke that! And you added some questionable reasoning like HTML5 pattern constraint validates the input value, so we validate the viewValue. What? If i wanted native html5 validation I wouldn't add novalidate to my form and perhaps I wouldn't even use angular in the first place.

That's why we add the novalidate attribute — because we want our customary logic validating the inputs.


We could look at the problem from another angle. Ng directives should be prefixed with ng-, right? How come the pattern directive became an exception? Pattern is a native html attribute which I don't want angular to mess with. There are good semantic reasons why I'd want to add this attribute and not have angular intervene. For example, I wish the mobile users to use a numeric keyboard — then I would add pattern="\d*" attribute. The mobile OS would know what to do. If I want angular to kick off its validation powers I'd use the ng-pattern directive. That's expected, consisted and desired behavior.

@gkalpak
Copy link
Member

gkalpak commented Jan 28, 2016

@everdimension, there are two different subjects touched here:

  1. Changing the pattern/ngPattern directive to match against the $viewValue (instead of the $modelValue).
  2. Having the native HTML5 validators (such as pattern, max, min, required etc) also be recognized by Angular as validators.

Regarding (1), while it is true that no solution is perfect (i.e. not everyone will be happy with any one solution), we picked the one that would be useful in most usecases. Now that Angular supports several types of inputs (e.g. number, date, time etc), it's not uncommon for the $modelValue to be non-string. Since a RegExp (such that the one applied by ngPattern) can only be tested against strings, we figured it would be more useful to use the $viewValue (which is always a string).

Of course, the usecase you describe is still valid and you can support it via a custom validator. I understand this is more work than just using a built-in validator, but it's still pretty trivial and the usecase is not very common.
IMO, it makes more sense for a framework to support the most common usecases out-of-the-box and require a little extra work for the less common ones than the other way around.

Regarding (2), indeed one could have gone the other way around - both approaches have pros and cons. At this point though, I believe that, altering the current behavior, would be too much of a breaking change to justify the benefit.

Also, note that [novalidate] doesn't mean that the native validators are not applied (e.g. the control's validity property is still updated as usual); it just doesn't natively prevent the form from being submitted.

@everdimension
Copy link

Of course, the usecase you describe is still valid and you can support it via a custom validator. I understand this is more work than just using a built-in validator, but it's still pretty trivial and the usecase is not very common.

This would be appropriate solution if all I wanted was to validate a field using a regex pattern.

But like I said, there is a place where I use pattern="\d*" with a different intention — to bring up the numeric keyboard, which a valid practice

So basically my solution would be to overwrite the existing pattern directive, which is a kind of dirty solution.

Yes, there are two different issues, and in my opinion both of them are done wrong.

The (1) breaks the native angular pipeline, killing an important part of the framework. Date, number inputs? Handle them like a special case! They are a special case anyway, in my opinion.

The (2) interferes with expected behavior. Although this one is more understandable as there are several places where angular creates directives for native html attributes. This is kind of how angular is built. But still I never expected pattern to be controlled by angular since there is an ng-pattern specially for that.

@Narretz
Copy link
Contributor

Narretz commented Jan 29, 2016

@everdimension I don't think it was ever intended that using the HTML5 validation attributes keeps Angular from adding validation rules based on them. Even going back to 1.3, the docs state clearly that using the pattern attribute adds a validator.

@gkalpak
Copy link
Member

gkalpak commented Jan 29, 2016

So basically my solution would be to overwrite the existing pattern directive, which is a kind of dirty solution.

You can do something simple, such as apply the pattern attribute through a custom directive (in the post-link phase). Demo
It's not ideal, but again applying a pattern on a number or date field don't seem less common that using an input mask or applying pattern="\d*" on a text field to bring up a numeric keyboard.
Unfortunately we can't make everyone happy: We chose some usecase to work out-of-the-box and some to need a little more code. Could be the other way around, but there is no solution where every usecase works out-of-the-box - there are always going to be compromizes.

Regarding (2), whether it was a good or bad decision is a different discussion, but at this point changing it doesn't justify the breaking change, so it might be a pointless discussion 😉

@diegomrsantos
Copy link
Contributor

diegomrsantos commented Feb 28, 2017

why isn't it documented in the migration guide the ng-pattern directive code changed from

1.3.16

ctrl.$validators.pattern = function(value) {
    return ctrl.$isEmpty(value) || isUndefined(regexp) || regexp.test(value);
 };

to

1.4.14

ctrl.$validators.pattern = function(modelValue, viewValue) {
    // HTML5 pattern constraint validates the input value, so we validate the viewValue
     return ctrl.$isEmpty(viewValue) || isUndefined(regexp) || regexp.test(viewValue);
 };

@gkalpak
Copy link
Member

gkalpak commented Mar 1, 2017

For future reference: #15765

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