Bug 19776

Summary: Number.toExponential() is incorrect for numbers between 0.1 and 1
Product: WebKit Reporter: webkit
Component: JavaScriptCoreAssignee: Cameron Zwarich (cpst) <zwarich>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, eric, zwarich
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.4   
Attachments:
Description Flags
simple html/javascript showing the behaviour
none
Proposed patch
none
Proposed patch with tests darin: review+

webkit
Reported 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
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
DetailsFormatted DiffDiff
Proposed patch with tests (3.58 KB, patch)
2008-07-02 16:53 PDT, Cameron Zwarich (cpst) 		darin: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
webkit
Comment 1 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.
Alexey Proskuryakov
Comment 2 2008-06-26 05:26:48 PDT
Looks like fixes made for Acid3 in bug 16640 weren't sufficient.
Cameron Zwarich (cpst)
Comment 3 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
Cameron Zwarich (cpst)
Comment 4 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.
Eric Seidel (no email)
Comment 5 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. :)
Cameron Zwarich (cpst)
Comment 6 2008-07-02 16:53:41 PDT
Created attachment 22054 [details] Proposed patch with tests
Darin Adler
Comment 7 2008-07-02 16:54:56 PDT
Comment on attachment 22054 [details] Proposed patch with tests r=me
Cameron Zwarich (cpst)
Comment 8 2008-07-02 17:05:17 PDT
Landed in r34961.
Note You need to log in before you can comment on or make changes to this bug.