Closed Bug 866369 Opened 11 years ago Closed 11 years ago

Incorrect rounding in Intl.NumberFormat


(Core :: JavaScript: Internationalization API, defect)

Not set





(Reporter: mozillabugs, Assigned: mozillabugs)




(3 files)

ECMA-402 specifies the rounding behavior in Intl.NumberFormat.prototype.format to round towards the closest number that can be represented with the specified number of digits; in cases where two such numbers exist, it rounds away from zero. See the specifications of ToRawPrecision step 3.a and ToRawFixed step 2, both of which handle only the absolute value of the number to be formatted:

The current implementation in SpiderMonkey doesn't handle this correctly - it doesn't always round away from zero:
js> 123.5.toLocaleString("en", {maximumFractionDigits: 0})
js> 124.5.toLocaleString("en", {maximumFractionDigits: 0}) 
js> 124.4.toLocaleString("en", {maximumFractionDigits: 0})    
js> 124.6.toLocaleString("en", {maximumFractionDigits: 0}) 
js> (-123.5).toLocaleString("en", {maximumFractionDigits: 0})
js> (-124.5).toLocaleString("en", {maximumFractionDigits: 0}) 
This test is for the Test402 conformance test suite; it will also be pulled into js/src/tests.
Assignee: general → mozillabugs
Attachment #742672 - Flags: review?(jwalden+bmo)
I neglected to set the rounding mode for ICU, and the default in ICU is to break ties towards the even number.
Attachment #742673 - Flags: review?(jwalden+bmo)
Comment on attachment 742673 [details] [diff] [review]
Set ICU rounding mode

Review of attachment 742673 [details] [diff] [review]:

Huh.  I never really took the time to notice all the other number-formatting stuff in JS was round-away-from-zero, and not round-to-nearest-even as the language operations do.  Good to know!

I remember thinking about this issue existing when reviewing these patches initially, then not doing due diligence to verify.  :-\  Sorry about that!

I'll batch up these patch-lands with the Ugandan currency change, when the tree reopens.
Attachment #742673 - Flags: review?(jwalden+bmo) → review+
Attachment #742672 - Flags: review?(jwalden+bmo) → review+
...or no, I'll just push the js/src change, and we'll pick up the other in a test402 update.  :-)  Skimming too hard, escaped me that was a change not to our tree.

Keeping open for the test402 update that'll pick this up...
Whiteboard: [leave open]
Target Milestone: --- → mozilla23
The test case update for Test402, now pulled into jstests. Carrying r+jwalden.
Attachment #743396 - Flags: review+
Attachment #743396 - Flags: checkin?(jwalden+bmo)
Attachment #743396 - Flags: checkin?(jwalden+bmo) → checkin+
Whiteboard: [leave open]
Closed: 11 years ago
Resolution: --- → FIXED
Mass-moving existing Intl-related bugs to the new Core :: JavaScript: Internationalization API component.

If you think this bug has been moved in error, feel free to move it back to Core :: JavaScript Engine.

[Mass change filter: core-js-intl-api-move]
Component: JavaScript Engine → JavaScript: Internationalization API
You need to log in before you can comment on or make changes to this bug.