-
Notifications
You must be signed in to change notification settings - Fork 27.4k
fix(input): format numeric string model values correctly #6684
Conversation
@@ -1160,6 +1160,9 @@ function numberInputType(scope, element, attr, ctrl, $sniffer, $browser) { | |||
} | |||
|
|||
ctrl.$formatters.push(function(value) { | |||
if (isString(value) && !isNaN(value)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isNaN might not be the best choice here, but I think it's probably ok
@@ -1160,6 +1160,9 @@ function numberInputType(scope, element, attr, ctrl, $sniffer, $browser) { | |||
} | |||
|
|||
ctrl.$formatters.push(function(value) { | |||
if (isString(value) && !isNaN(value)) { | |||
value = Number(value); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't we pin the radix to 10? without it it's easy to run into incorrect parsing issues.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe Number is more or less equivalent to parseFloat()
when the parameter is a string, I'm not sure it's actually possible to get it to parse an octal or binary number from a string. Using parseFloat() would work just as well, too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, as far as I can tell, there's no way to set a radix for an html5 number input type, so supporting this would be a step beyond what html5 input types actually do... I don't necessarily see that as a bad thing, however it's complicated, and users don't really have a way to ask for a specific radix (and if they did, and tried to enter a hex number, a browser with HTML5 constraint validation would call it invalid anyways)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, Number("010") => 10
, you can't really do octal notation this way...
Interestingly, v8 will translate 0xFF into 255, hmm, so it does most of what parseFloat() will do, but it won't stop if it encounters a character that can't be tokenized as a number (so Number("10abc") => NaN
vs parseFloat("10abc") => 10
)
Previously, when a numeric string is used as a model value, the number input type would not format the value. This patch corrects this behaviour by converting to a number before performing validation, if the value is a string and not NaN. Closes angular#6683
02dc2aa
to
fd2d6c0
Compare
Previously, when a numeric string is used as a model value, the number input
type would not format the value. This patch corrects this behaviour by
converting to a number before performing validation, if the value is a string
and not NaN.
Closes #6683