Summary: | Acid3 reveals broken behavior in toFixed and toExponential | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Eric Seidel (no email) <eric> | ||||||||||||||||
Component: | JavaScriptCore | Assignee: | Nobody <webkit-unassigned> | ||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||
Severity: | Normal | CC: | staikos | ||||||||||||||||
Priority: | P2 | Keywords: | EasyFix | ||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||||
Hardware: | Mac | ||||||||||||||||||
OS: | OS X 10.4 | ||||||||||||||||||
Bug Depends on: | 16643 | ||||||||||||||||||
Bug Blocks: | |||||||||||||||||||
Attachments: |
|
Description
Eric Seidel (no email)
2007-12-28 02:15:09 PST
Created attachment 18143 [details]
Test case for toFixed/toExponential
.../fast/js/resources/toFixed-and-toExponential.js | 6 ++++++
LayoutTests/fast/js/toFixed-and-toExponential.html | 13 +++++++++++++
2 files changed, 19 insertions(+), 0 deletions(-)
I don't have the energy tonight to actually make the fix, but the attached test case patch, makes it very clear what is broken. I have patches to fix bugs in our toFixed, toExponential and toPrecision implementations, as well as add test cases for toString. Created attachment 18154 [details]
Test case for toFixed/toExponential
.../fast/js/resources/toFixed-and-toExponential.js | 6 ++++++
LayoutTests/fast/js/toFixed-and-toExponential.html | 13 +++++++++++++
2 files changed, 19 insertions(+), 0 deletions(-)
Created attachment 18155 [details]
Add more toFixed and toExponential tests, fix (-0).toFixed() and refactor a little
JavaScriptCore/kjs/number_object.cpp | 37 +++++++++++--------
.../fast/js/resources/toFixed-and-toExponential.js | 38 ++++++++++++++++++++
2 files changed, 59 insertions(+), 16 deletions(-)
Created attachment 18156 [details]
Fix -0.toExponential() and printing of trailing 0s in toExponential
JavaScriptCore/kjs/number_object.cpp | 51 +++++++++++--------
.../fast/js/resources/toFixed-and-toExponential.js | 5 ++
2 files changed, 34 insertions(+), 22 deletions(-)
Created attachment 18157 [details]
Add more tests, including toPrecision
.../fast/js/resources/toFixed-and-toExponential.js | 66 ++++++++++++++++----
1 files changed, 54 insertions(+), 12 deletions(-)
Created attachment 18158 [details]
Split tests into 3 files, fixed toPrecision(nan) handling
JavaScriptCore/kjs/number_object.cpp | 2 +-
LayoutTests/fast/js/number-tofixed-expected.txt | 24 +++++
.../fast/js/number-toprecision-expected.txt | 26 +++++-
.../fast/js/resources/number-toExponential.js | 43 +++++++++
LayoutTests/fast/js/resources/number-tofixed.js | 44 ++++++++++
.../fast/js/resources/number-toprecision.js | 38 ++++++++-
.../fast/js/resources/toFixed-and-toExponential.js | 91 --------------------
LayoutTests/fast/js/toFixed-and-toExponential.html | 13 ---
8 files changed, 170 insertions(+), 111 deletions(-)
Created attachment 18159 [details]
Add toString testing for firefox compatibility
.../fast/js/number-toExponential-expected.txt | 27 ++++++++
LayoutTests/fast/js/number-toString-expected.txt | 58 +++++++++++++++++
LayoutTests/fast/js/number-toString.html | 13 ++++
LayoutTests/fast/js/resources/number-toString.js | 67 ++++++++++++++++++++
4 files changed, 165 insertions(+), 0 deletions(-)
I should note that these patches depend on the patches attached to bug 16643. *** Bug 5258 has been marked as a duplicate of this bug. *** *** Bug 5259 has been marked as a duplicate of this bug. *** Comment on attachment 18156 [details]
Fix -0.toExponential() and printing of trailing 0s in toExponential
I don't believe you can safely check against -0.0
+ if (x == -0.0) // (-0.0).toExponential() should print as 0 instead of -0
+ x = 0;
Comment on attachment 18159 [details]
Add toString testing for firefox compatibility
whoops
Addition of failures == badness
Sunspider reports somewhat wild results (some faster some slower) as a result of these changes. None seem related, and I write it off as random gcc code-gen changes. :( The overall sunspider result is: ** TOTAL **: ?? 3881.8ms +/- 0.2% 3886.3ms +/- 0.2% not conclusive: might be *1.001x as slow* |