Bug 19776 - Number.toExponential() is incorrect for numbers between 0.1 and 1
Summary: Number.toExponential() is incorrect for numbers between 0.1 and 1
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: Cameron Zwarich (cpst)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-06-26 00:44 PDT by webkit
Modified: 2008-07-02 17:05 PDT (History)
3 users (show)

See Also:


Attachments
simple html/javascript showing the behaviour (952 bytes, text/html)
2008-06-26 02:56 PDT, webkit
no flags Details
Proposed patch (1.45 KB, patch)
2008-07-02 10:40 PDT, Cameron Zwarich (cpst)
no flags Details | Formatted Diff | Diff
Proposed patch with tests (3.58 KB, patch)
2008-07-02 16:53 PDT, Cameron Zwarich (cpst)
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description webkit 2008-06-26 00:44:36 PDT
f number is say 1.1 then numbertoexponential(4) = 1.1000e+0 -- which is fine.

number is 0.999 then numbertoexponential(4) = 9.9900e+1 -- which isn't.

numbers less than 0.1 & greater than 0.9999 give expected results.


tested on safari 2.0.4 & 3.1.1 & 4.beta
Comment 1 webkit 2008-06-26 02:56:37 PDT
Created attachment 21943 [details]
simple html/javascript showing the behaviour

camino,firefox,opera give  'expected' result, webkit browsers don't.
Comment 2 Alexey Proskuryakov 2008-06-26 05:26:48 PDT
Looks like fixes made for Acid3 in bug 16640 weren't sufficient.
Comment 3 Cameron Zwarich (cpst) 2008-07-02 10:24:21 PDT
The value of decimalAdjust in numberProtoFuncToExponential() is wrong when the value is between 0 and 1.

Consider what happens with n = 0.1. If the number of digits is undefined, we get:

decimalPoint = 0
decimalAdjust = 0

If the number of digits is defined, we get:

decimalPoint = 1
decimalAdjust = -1

Comment 4 Cameron Zwarich (cpst) 2008-07-02 10:40:20 PDT
Created attachment 22049 [details]
Proposed patch

Actually, the problem isn't with decimalPoint and decimalAdjust, it's with what is done with the decimalPoint value in exponentialPartToString(). The sign check on the exponent is done on the decimalPoint value before subtracting 1 to get the exponent. Here is a patch that fixes it.

I'll write some tests later and put it up for review, but I don't have the time before my calculus tutorial.
Comment 5 Eric Seidel (no email) 2008-07-02 12:24:50 PDT
Comment on attachment 22049 [details]
Proposed patch

Makes sense, but needs the tests (hopefully in the fast/js .js file-only form. :)
Comment 6 Cameron Zwarich (cpst) 2008-07-02 16:53:41 PDT
Created attachment 22054 [details]
Proposed patch with tests
Comment 7 Darin Adler 2008-07-02 16:54:56 PDT
Comment on attachment 22054 [details]
Proposed patch with tests

r=me
Comment 8 Cameron Zwarich (cpst) 2008-07-02 17:05:17 PDT
Landed in r34961.