Bug 10820 – Add StringImpl::toDouble() and remove uses of .deprecatedString().toDouble()

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Eric Seidel (no email)]

Attached is my (broken) patch to do this.  Don't know why it fails, but it does.  I'll come back to it eventually.  Posting now for posterity (in case I forget, or someone else wishes to give it a go).

[Eric Seidel (no email)]

Created attachment 10509 [details]
Broken patch (breaks layout tests for svg gradients)

[Alexey Proskuryakov]

Perhaps String/StringImpl need a clear division between locale-sensitive and insensitive routines (number parsing can obviously be either). Not something for this bug, although a little comment in the header could be useful.

[Anders Carlsson]

Created attachment 10778 [details]
Non-broken patch

I found two problems with the toDouble implementation:

+double StringImpl::toDouble(bool* ok) const
+{
+    if (empty()) {

this should be isEmpty(). empty() returns the special empty string singleton so this is always true.

+        if (ok)
+            *ok = false;
+        return 0;
+    }
+    char *end;
+    double val = kjs_strtod(Latin1Encoding().encode(characters(), length()).data(), &end);
+    if (ok)
+        *ok = end == 0 || *end == '\0';
TextEncoding::encode returns a CString which is destroyed after calling kjs_strtod. This makes the end pointer dangling so *end could potentially crash.

+    return val;
+}

I've uploaded a patch that doesn't break any layout tests.

[Maciej Stachowiak]

Comment on attachment 10778 [details]
Non-broken patch

r=me

[Anders Carlsson]

Committed revision 16568.

[Dave Hyatt]

Please remember in the future that this kind of header include:

#include <JavaScriptCore/dtoa.h>

is Mac-specific.  I fixed it to be

#include <kjs/dtoa.h>

instead to fix Win32.