Input number with required validation is broken on chrome

When you have a input[number] with required validation, number validation stops working correctly.

If the field in pristine mode and you start typing letters instead of numbers things get weird and validity.badInput and validity.valueMissing are set as true at the same time, $error.required is set as true and $error.number set as false.

Reproduction: http://plnkr.co/edit/nGjpNLyT2RJ3URbT9Apg
Tested in Chrome Version 33.0.1750.152

fix(input): use ValidityState to determine validity

I'm not sure this issue is a particularly big deal, it keeps things simpler to avoid updating validity when valueMissing is true. But if anyone wants to write a patch for this, I'm happy to review it.

This is very close to #6304

As workaround for people like me with tight deadlines, I've managed to get this validations to work with a few changes.

// Changes in Html5Validators

function isInvalidState(validity) {
    return validity.badInput || validity.customError || validity.typeMismatch;
}

function addNativeHtml5Validators(ctrl, validatorName, element) {
  var validity = element.prop('validity');
  if (isObject(validity)) {
    var validator = function(value) {
      // Don't overwrite previous validation, don't consider valueMissing to apply (ng-required can
      // perform the required validation)
      if (!ctrl.$error[validatorName] && isInvalidState(validity)) {
        ctrl.$setValidity(validatorName, false);
        return;
      }
      return value;
    };
    ctrl.$parsers.push(validator);
    ctrl.$formatters.push(validator);
  }
}

and the requiredDirective..

var requiredDirective = function() {
  return {
    require: '?ngModel',
    link: function(scope, elm, attr, ctrl) {
      if (!ctrl) return;
      attr.required = true; // force truthy in case we are on non input element
      var validity = elm.prop('validity');

      var validator = function(value) {
        if ((attr.required && ctrl.$isEmpty(value)) && !isInvalidState(validity)) {
          ctrl.$setValidity('required', false);
          return;
        } else {
          ctrl.$setValidity('required', true);
          return value;
        }
      };

      ctrl.$formatters.push(validator);
      ctrl.$parsers.unshift(validator);

      attr.$observe('required', function() {
        validator(ctrl.$viewValue);
      });
    }
  };
};

Edit: forgot this

// Listener from textInputType
var listener = function() {
    if (composing) return;
    var value = element.val();
    var validity = element.prop('validity');

    // By default we will trim the value
    // If the attribute ng-trim exists we will avoid trimming
    // e.g. <input ng-model="foo" ng-trim="false">
    if (toBoolean(attr.ngTrim || 'T')) {
      value = trim(value);
    }

        // Force validations to run again if is in Invalid State
    if (ctrl.$viewValue !== value || isInvalidState(validity)) {
      if (scope.$$phase) {
        ctrl.$setViewValue(value);
      } else {
        scope.$apply(function() {
          ctrl.$setViewValue(value);
        });
      }
    }
  };

Here is the working test http://plnkr.co/edit/MELexufCFKrmn4gpfvus

That particular change will be broken in other ways. Anyways, I don't see this as much of a user pain, but if you want to send a pull request to fix this (which would require the textInputType to be aware of the validity errors that we care about, as well), then by all means, send it

@caitp

@caitp any updates on this one? there is PR for that, but it's been left with no feedback

I don't see this as much of a user pain

Put a user's hat on and think what your feelings would be if you saw "Value is missing" instead of "Value is not a number" when you entered "Hello World!" into input[number]?

Thanks!

@xaka

@xaka there are a couple things which are probably broken currently.

if the value hasn't changed, we're only running an update if there is a validity state and valueMissing is false. This is probably wrong, we should probably update even if valueMissing is set.
ngRequired can check if there is a badInput flag set in the validity state.

The spec could possibly change to make this suck a bit less, but it also might not. Hixon seems pretty set about the way it works now. If the spec does change, then we can fix this in browsers and be a bit more performant and less annoying. But again, there's a chance that it will never come to this.

However, by fixing the points mentioned above, we can work around those limitations (with a pretty minor performance cost)

Changing specs and applying those changes in browsers seems like a dead end to me :) I mean there is a chance it could happen, but not in the nearest future, so in my opinion the only option we have is to rely on a framework.

Is there anything that should be addressed in that PR? Would you like to see another PR for those points you mentioned? I'm trying to figure out the plan to solve as it gives me nightmares, hehe.

comments have been left here and on the PR regarding ways to work around it, but it's all going to get dropped once we refactor how validation works anyways.

However, changing the spec is certainly not something that is totally impossible, we've made pretty good cases for changing this, and it is pretty trivial to implement in the 3 major engines. IE is the only one that would be hard, and they're buggy regardless (WRT constraint validation), so it doesn't really matter there.

Are you planning to backport those changes to 1.2.x?

At this time it's looking highly unlikely the HTML5 spec will be updated to enforce rules against allowing valueMissing to be true when the only valid error is actually badInput.

I'v tried several approaches to fixing this today, and it has been far from elegant. While checking the ValidityState object for (valueMissing && !badInput) does allow AngularJS to return the 'number' error within the validation responses, the 'required' is still set to true, which means nasty checks for displaying only the "This is not a number" message in our views.

These changes also break several test depending on where you check the ValidityState object, i.e. 8 test break if done directly inside ngRequire, and 2 break if we override the method 'numberInputType.ctrl.$isEmpty'.

The reason for these test to fail is because ngRequire pushed into both '$formatters' and '$parsers'. When the ngRequire runs under the '$formatters' context, ValidityState update isn't updated, because only the modelValue has changed, and I don't find it valid to force push this into the viewValue. I'm not 100% sure why validation is ran against modelValue changes (to me this is programmatically data changes done by the developer, not the user, and should have been validated in some manner already), but there may be some valid use case for such checks.

As an update, we should now have 3 browsers with this defect as Firefox just recently pushed out support for Input type number with validation. Opera is mostly a direct clone of Chrome. Which means Chrome, Opera and Firefox all have this issue. Unsure when IE will extend their support for Input type number (currently is partially works, but is still more or less a text input). Safari does support the number input, but removing Alpha values when the user removes focus, so this issue is avoided completely.

With that said, I believe currently at this point, we may have to wait for a true fix during the Form Validation Refactoring. Hopefully during that project we can split our true validation and modelValue 'validation' logic, so that we can use the ValidityState object if the browser supports and provides it.

I've had a chat with Hixon about this, it doesn't seem to get through to him that it's complete crap the way it is now, I agree.

However, it should be trivial to work around the issue, and it will be even easier once matsko's form refactoring lands

I as well have been involved in the specification defect conversation, if you are unaware my real name is Mason.

I'm not yet to the point to agree about the trivial factor for a clean work around for this. In the sense of base Javascript it is a trivial task. But in terms of AngularJS and the logic flow it has, not quite close enough to feel it is trivial yet, I feel I'm just a tad bit away from a solution though.

Although not something I find a clean way to handle this, so far I am able to get all current test to pass by adding the ValidityState check logic within ngRequire. But in order for this to work I currently have a flag passed into ngRequire's internal method validator. This flag when set to true allows the check to use the ValidityState object (as its actually been updated at this point, so its safe to use its values). The flag is only set true when passing through the $parsers chain, because $formatters is from modelValue changes, which result in a stale ValidityState object. The issue with this change is that although I am now able to get $error.number=true, $error.required is also true. Because the $commitViewValue sets the $modelValue to the $viewValue, thus triggering the $formatters to execute and set the required error state. I don't see a clean way around this problem yet. And I will not accept requiring developers to have to do [show required message if $error.required && !$error.number]. Even if I could somehow get the value the user provided, I still wouldn't find it valid to set the $modelValue to that either.

If anyone would like me to share a GIT patch with them to assist and collaborate a solution with me, just let me know. It may be possible I am just over looking something.

@matsko

It should be pretty trivial after @matsko's refactoring.

Hixie's idea of the ValidityState, is that there is a de facto ordering for applications to process it. And that ordering isn't really spelled out anywhere, which kind of sucks, but he wants you to do it like this:

1.	`valid`	If true, ignore everything else, control is valid
2.	`badInput`	If true, ignore everything else, form is suffering from bad input
3.	`valueMissing`	If true, ignore everything else, no value has been entered into a required control

Then, after all of those, the semantics for the other flags are less defined, but these are a good set to worry about for now. We can also ignore the valid flag.

I don't like this design, and I don't think it was very thoroughly considered when it went into the spec, but that's the world we live in, I guess. So, here's what we can do:

After Matias finishes refactoring how form validation in angular works at all, we should be able to have more control over the validation pipeline and the order different validators are run. So we can ask the "number" validator to run first, then the "required" validator to run after that, and so on.

Given that, we're kind of blocked on his changes, but I think it would be good to prototype this work ontop of that refactoring. If you want to start on this, that work is at #7377

@matsko

was this ever fixed?

It appears to still be an issue in beta13 as you can see here. Enter some text (not a number), we get required as true and number as false still.

I expected the validators pipeline work by @matsko to fix this.

@43081j

@43081j currently, it will still report 'required' error if there is an invalid number (only affects modern browsers --- this should be fixed soon), but it will report the number error correctly:

https://code.angularjs.org/snapshot/docs/api/ng/input/input%5Bnumber%5D

Just reanalyzed the original plunker (with the angular version of that time as the plunker includes it), and I cannot find the error described above. However, I can find the following:

In pristine mode, $error.required is true and $error.number is false, which is ok.
Enter letters instead of numbers
$error.required is set to false and $error.number is set to true, which is ok
clear the input
$error.required is still set to false (but should be true again) and $error.number is still set to true (but should be false again)

Using the current angular version this is now fixed, see this plunker: http://plnkr.co/edit/Ps5isvunBNpBx5zbmBuz?p=preview

Navigation Menu

Repository navigation

Input number with required validation is broken on chrome #6818

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Input number with required validation is broken on chrome #6818

Description

Activity

caitp commented on Mar 24, 2014

tbosch commented on Mar 24, 2014

rwtnb commented on Mar 25, 2014

caitp commented on Mar 25, 2014

xaka commented on Apr 25, 2014

caitp commented on Apr 25, 2014

xaka commented on Apr 25, 2014

caitp commented on Apr 25, 2014

xaka commented on Apr 25, 2014

starr0stealer commented on May 16, 2014

caitp commented on May 17, 2014

starr0stealer commented on May 17, 2014

caitp commented on May 17, 2014

43081j commented on Jun 27, 2014

caitp commented on Jun 27, 2014

tbosch commented on Oct 2, 2014

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions