Bug 5258

Summary:

toFixed() doesn't handle enough cases

Product:

WebKit

Reporter:

George Staikos <staikos>

Component:

JavaScriptCore

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED DUPLICATE

Severity:

Normal

CC:

ian

Priority:

Keywords:

HasReduction, InRadar

Version:

420+

Hardware:

Other

OS:

All

Attachments:

Description	Flags
Testcase	none
Test case	none

George Staikos

Reported 2005-10-04 10:52:18 PDT

Number.toFixed() doesn't handle any case besides 0..20 which is not what FireFox does. See the testcases below. I don't have a patch to fix all of this at this time. Here are some testcases: print(eval("Number(1234.567).toFixed(undefined)")); print("expected 1235"); print(eval("Number(1234.567).toFixed(0)")); print("expected 1235"); print(eval("Number(1234.567).toFixed(1)")); print("expected 1235.6"); print(eval("Number(1234.567).toFixed(2)")); print("expected 1235.57"); print(eval("Number(1234.567).toFixed(-1)")); print("expected 1230"); print(eval("Number(1234.567).toFixed(-4)")); print("expected 0"); print(eval("Number(1234.567).toFixed(-5)")); print("expected 0"); print(eval("Number(1234.567).toFixed(1/0)")); print("expected exception");

Attachments
Testcase (870 bytes, text/html) 2006-02-15 14:53 PST, Joost de Valk (AlthA)	no flags	Details
Test case (298 bytes, text/html) 2007-06-26 13:11 PDT, ivo	no flags	Details
View All Add attachment proposed patch, testcase, etc.

Ricci Adams

Comment 1 2006-01-03 20:02:36 PST

Section 15.7.4.5 of ECMA-262 documents the correct behavior of toFixed(). Step 2 specifically states that a RangeError exception should be thrown for cases other than 0..20. Thus, I believe that JavaScriptCore's behavior for the last 7 test cases presented in the above comment is correct. However, step 1 states that toFixed(undefined) should be the same as toFixed(0). This does not appear to be the case in our implementation.

Ricci Adams

Comment 2 2006-01-03 20:20:37 PST

Oops. toFixed(undefined) was fixed in http://bugzilla.opendarwin.org/show_bug.cgi?id=6267 . I accidentally tested in latest-released Safari instead of ToT.

Maks Orlovich

Comment 3 2006-01-04 11:49:39 PST

Both behavior are valid, actually. At the second of the section, you'll noticed: "An implementation is permitted to extend the behaviour of toFixed for values of fractionDigits less than 0 or greater than 20. In this case toFixed would not necessarily throw RangeError for such values. "

Gavin Kistner

Comment 4 2006-01-17 07:14:35 PST

...which means that this is less of a bug report, and more of a feature request. (To the extent that not matching some non-standard behavior of browser X is a feature mis-match and not a bug.)

Joost de Valk (AlthA)

Comment 5 2006-02-13 16:04:26 PST

Reassigning to webkit-unassigned, to make sure more people see this.

Joost de Valk (AlthA)

Comment 6 2006-02-15 14:53:27 PST

This is an enhancement request, as i agree with #c4, attaching testcase in a sec.

Joost de Valk (AlthA)

Comment 7 2006-02-15 14:53:45 PST

Created attachment 6512 [details] Testcase

ivo

Comment 8 2007-06-26 13:08:41 PDT

toFixed as well as toPrecision do have failures which seem related to this issue. The following testcase demonstrates one of the very clear ones. <html> <head> <title>Test</title> <script type="text/javascript"> var zahl1=1.0; var zahl2=1.000000001; var zahl3=-0.0; var zahl4=-0.00001; alert("1.0=="+zahl1.toPrecision(6)+"=="+zahl2.toPrecision(6)); alert("0.0=="+zahl3.toFixed(3)+"=="+zahl4.toFixed(3)); </script> </head> <body> </body> </html> This issue was Bug 136734 in kde and is fixed meanwhile in kde(don't ask me how. I did not have a look at the code till now)

ivo

Comment 9 2007-06-26 13:11:15 PDT

Created attachment 15258 [details] Test case

David Kilzer (:ddkilzer)

Comment 10 2007-07-05 20:44:29 PDT

(In reply to comment #9) > Created an attachment (id=15258) [edit] > Test case This test case assert()'s running a local debug build of WebKit r24013 with Safari 3.0 (522.12) on Mac OS X 10.4.10 (8R218): /path/to/WebKit/JavaScriptCore/kjs/number_object.cpp:371: failed assertion `n < intPow10(p)' Abort trap

David Kilzer (:ddkilzer)

Comment 11 2007-07-05 20:46:34 PDT

<rdar://problem/5316243>

Eric Seidel (no email)

Comment 12 2007-12-28 15:16:27 PST

The patches attached to bug 16640 make us pass these tests. I'm going to dup this to bug 16640 unless there are objections. *** This bug has been marked as a duplicate of 16640 ***

Note You need to log in before you can comment on or make changes to this bug.