-
Notifications
You must be signed in to change notification settings - Fork 27.4k
fix(input): format numeric string model values correctly #6684
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. I believe Number is more or less equivalent to There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, 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 |
||
} | ||
return validate(ctrl, 'number', ctrl.$isEmpty(value) || isNumber(value), value); | ||
}); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1538,6 +1538,54 @@ describe('input', function() { | |
}); | ||
|
||
|
||
it('should translate numeric base-10 strings in model to numbers', function() { | ||
compileInput('<input type="number" ng-model="age" />'); | ||
|
||
scope.$apply(function() { | ||
scope.age = '10'; | ||
}); | ||
|
||
expect(scope.age).toBe('10'); | ||
expect(inputElm.val()).toBe('10'); | ||
}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we need a test case for octal number... |
||
|
||
|
||
it('should translate numeric base-16 strings in model to numbers', function() { | ||
compileInput('<input type="number" ng-model="age" />'); | ||
|
||
scope.$apply(function() { | ||
scope.age = '0xFf'; | ||
}); | ||
|
||
expect(scope.age).toBe('0xFf'); | ||
expect(inputElm.val()).toBe('255'); | ||
}); | ||
|
||
|
||
it('should translate scientific notation from numeric strings in model to numbers', function() { | ||
compileInput('<input type="number" ng-model="age" />'); | ||
|
||
scope.$apply(function() { | ||
scope.age = '0.1e6'; | ||
}); | ||
|
||
expect(scope.age).toBe('0.1e6'); | ||
expect(inputElm.val()).toBe('100000'); | ||
}); | ||
|
||
|
||
it('should remove leading zeros from numeric strings in model', function() { | ||
compileInput('<input type="number" ng-model="age" />'); | ||
|
||
scope.$apply(function() { | ||
scope.age = '010'; | ||
}); | ||
|
||
expect(scope.age).toBe('010'); | ||
expect(inputElm.val()).toBe('10'); | ||
}); | ||
|
||
|
||
describe('min', function() { | ||
|
||
it('should validate', function() { | ||
|
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