Incorrect rounding in Intl.NumberFormat

RESOLVED FIXED in mozilla23

Status

()

RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: mozillabugs, Assigned: mozillabugs)

Tracking

(Blocks: 1 bug)

Trunk
mozilla23
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

(Assignee)

Description

6 years ago
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:
http://www.ecma-international.org/ecma-402/1.0/#ToRawPrecision
http://www.ecma-international.org/ecma-402/1.0/#ToRawFixed

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})
"124"
js> 124.5.toLocaleString("en", {maximumFractionDigits: 0}) 
"124"
js> 124.4.toLocaleString("en", {maximumFractionDigits: 0})    
"124"
js> 124.6.toLocaleString("en", {maximumFractionDigits: 0}) 
"125"
js> (-123.5).toLocaleString("en", {maximumFractionDigits: 0})
"-124"
js> (-124.5).toLocaleString("en", {maximumFractionDigits: 0}) 
"-124"
(Assignee)

Comment 1

6 years ago
Created attachment 742672 [details] [diff] [review]
Test case for Test402 conformance test suite

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)
(Assignee)

Comment 2

6 years ago
Created attachment 742673 [details] [diff] [review]
Set ICU rounding mode

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 3

6 years ago
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+

Updated

6 years ago
Attachment #742672 - Flags: review?(jwalden+bmo) → review+

Comment 4

6 years ago
...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.

Comment 5

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/00275de9314a

Keeping open for the test402 update that'll pick this up...
Whiteboard: [leave open]
Target Milestone: --- → mozilla23
(Assignee)

Comment 6

6 years ago
Created attachment 743396 [details] [diff] [review]
Test case for jstests

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+
(Assignee)

Updated

6 years ago
Whiteboard: [leave open]
https://hg.mozilla.org/mozilla-central/rev/75db6ad43aa1
Status: NEW → RESOLVED
Last Resolved: 6 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.