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

Step validation incorrect for large numbers in input[type=number] #16486

Closed
1 of 3 tasks
jchilleo opened this issue Mar 12, 2018 · 5 comments · Fixed by #16573, angular-indonesia/angular.js#141 or javascript-indonesias/angular.js#86

Comments

@jchilleo
Copy link

I'm submitting a ...

  • bug report
  • feature request
  • other

Current behavior:

If you have a form with number input boxes and the form has novalidate enabled, and you put a max value on the input with a step of 1.

If a user enters something larger than max value you get the error message for $error.max (as expected). However if the user enters in twenty-two or digits (except 9's, see below for more info), then the error message for $error.max and $error.step will be displayed.

However with a step of 1 a user cannot enter in decimals, so the decimal error message should not be displayed at all. My assumption is maybe it's a bit overflow/rounding error. Which is they the 9's have an issue, but that is just a theory and not verified in any way.

Expected / new behavior:

The expected behavior is that I have a Step of 1, which means I don't want decimals. So the $error.step should not activate, as it will show an error message for bad decimal when they cannot add a decimal to begin with.

A simple don't include the span that has the $error.step with your input box is not sufficient. As I have an imported module that holds the boilerplate code for form objects. So I am using a directive and cannot remove the span.

Minimal reproduction of the problem with instructions:

I created a codepen repo to show the issue.
If you have a form, with novalidate on, and create a number input box. Put a max value of 9999999999, and a step of 1, on the input box.

Then create a couple spans below the input box so we can change the error message wording. For this example just make two spans, one for $error.max and $error.step. Only show the span if there is an error in $error for those items.

So what happens when you enter 99999999999? The span for the error message $error.max is displayed. If you entered in twenty-one 1's (111111111111111111111) still get the expected $error.max. Now if you entered in twenty-two or more 1's (1111111111111111111111) you will get the error message for both $error.max and $error.step. This double error message holds true for all cases except all 9's.
However if you take twenty-two 9's like: 9 999 999 999 999 999 999 999 (Spaces for visual)
YYY XXX XXX XXX XXX XXX 999 999 where X and Y both = 9. However if you replace any one X with a value 0-8 it will produce the double error. If you replace any one Y with two numbers 0-8 it will produce the error. Changing the last 6 does not reproduce the error.

AngularJS version: 1.x.y

Browser: [all | Chrome XX | Firefox XX | Edge XX | IE XX | Safari XX | Mobile Chrome XX | Android X.X Web Browser | iOS XX Safari | iOS XX UIWebView | iOS XX WKWebView ]

OS
- Windows 10 Enterprise 64-bit 10.0.16299
Tested Browsers
- IE 11.248
- Chrome 64.0.3282
- Fire Fox 58.0.2
- Edge 41.16299
+ EdgeHtml 16.16299

Anything else:

In a perfect world, it would be nice if we could pass in an error object to something like errorValues to a form object. The object and replace the error messages of the $error parts. This way we wouldn't have to use novalidate.

ex:

ctrl.updatedErrorMessages = {
max: 'new string'
step: 'new string'
}
<input
     id = "example"
     type = "number"
     error = "{{ctrl.updatedErrorMessages}}">

Ideally in that case only max and step would have updated values and the remaining options would use their default.

@gkalpak
Copy link
Member

gkalpak commented Mar 12, 2018

This is a valid issue. Thx for the detailed write-up. The root cause is that the methods we use to detect whether a value is an integer and whether a value is a multiple of step, do not work for too large numbers:

function isNumberInteger(num) {
// See http://stackoverflow.com/questions/14636536/how-to-check-if-a-variable-is-an-integer-in-javascript#14794066
// (minus the assumption that `num` is a number)
// eslint-disable-next-line no-bitwise
return (num | 0) === num;
}

return (value - stepBase) % step === 0;

Unfortunately, I am not sure what the best alternative would be. I have tried several options from this SO question and nothing seems to work. Thus far, Math.floor(int) === int worked best, but still starts failing for large values.

Open to suggestions on what a proper way of validating step is. Maybe we could use the browser's validity.stepMismatch value (but tht would only work on browsers with native support).
I wonder how browsers handle the step validation.

@gkalpak gkalpak added this to the Backlog milestone Mar 12, 2018
@petebacondarwin
Copy link
Contributor

Perhaps we could allow the developer to plugin their own arithmetic library (such as https://github.com/MikeMcl/big.js/) to handle such situations?

@gkalpak
Copy link
Member

gkalpak commented Mar 14, 2018

It is possible to overwrite the built-in step validator today. E.g.:

.directive('input', () => ({
  require: '?ngModel',
  link: (scope, elem, attrs, ctrl) => {
    if (!ctrl) return;
    if (angular.isUndefined(attrs.step) && !attrs.ngStep) return;
      
    ctrl.$validators.step = (modelValue, viewValue) =>
      // Use your custom logic here.
      true;
  },
}))

And afaict, the main problem is that the browser itself can't handle such big numbers anyway. (I mean the step validation might work, but it can't represent them or manipulate them in a meaningful way.)

So, I doubt you are using a number input to get such numbers as input. If you do need the numbers you would probably use a text input and handle them with a specialized library (like big.js); in which case the step validator is irrelevent (as it only applies to number and range inputs).

So, my guess is that the main scenario here is using a number input with a max limit and the user enters a very big number, which is already invalid due to max. And the issue is that the step error is also shown (incorrectly).
If that is indeed the case, I think the best solution is to not show a step error unless the number is valid wrt to min and max.

WDYT?

@petebacondarwin
Copy link
Contributor

I would like to see the real world use case.

@Narretz Narretz changed the title $error showing wrong error message on form inputs. Step validation incorrect for large numbers in input[type=number] Apr 4, 2018
@Narretz Narretz modified the milestones: Backlog, 1.7.x May 3, 2018
@Narretz Narretz self-assigned this May 16, 2018
@petebacondarwin
Copy link
Contributor

@Narretz is going to update the docs to indicate that this is not supported.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.