Bug 16162 - [gtk] parseFloat depends on locale setting
Summary: [gtk] parseFloat depends on locale setting
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 523.x (Safari 3)
Hardware: PC Linux
: P2 Major
Assignee: Alexey Proskuryakov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-11-27 11:54 PST by Pierre-Luc Beaudoin
Modified: 2007-12-16 10:15 PST (History)
2 users (show)

See Also:


Attachments
proposed fix (2.69 KB, patch)
2007-12-16 09:09 PST, Alexey Proskuryakov
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Pierre-Luc Beaudoin 2007-11-27 11:54:07 PST
Parsing a float with parseFloat() doesn't return the same result for all locales.    

Sample script:
<script>
var input = prompt("Enter 0,5 or 0.5");
var test = 0.5;
if(input != null)
    test = parseFloat(input);
alert(eval("test == 0.5"));
alert(eval("test == '0.5'"));
if(test == 0.5)
    alert("Works!");
else
    alert("Pain!");
</script>

Tests data:
en_US: 
Entering 0.5 will produce true, true, then Works!
fr_CA:
Entering 0.5 will produce false, true then Pain!
Entering 0,5 will produce true, false, then Works!

Expected results (if parseFloat shouldn't use locale):
en_US: 
Entering 0.5 will produce true, true, then Works!
fr_CA:
Entering 0.5 will produce true, true, then Works!
Entering 0,5 will produce false, false, then Pain! (I guess)
Comment 1 Alexey Proskuryakov 2007-11-28 12:44:21 PST
I can see how this can happen if USE_LOCALE is defined. Is it defined on Linux somehow?

We should probably just remove USE_LOCALE code from dtoa.cpp.
Comment 2 Alp Toker 2007-12-09 11:14:06 PST
ap: USE_LOCALE isn't defined in my build and I don't see immediately how it could be defined.

Pierre-Luc: Could I get a bit more information on this? Can you check to see if USE_LOCALE is defined in your dtoa.cpp?
Comment 3 Pierre-Luc Beaudoin 2007-12-10 08:39:51 PST
I don't have USE_LOCALE define in my build.  ( I added 
#ifdef USE_LOCALE 
garbage  
#endif
in dtoa.cpp and it still compiles). 

This bug should affect anyone using a locale that uses a coma as the decimal point: French, Finish... all non-imperial unit locales I guess.
Comment 4 Alexey Proskuryakov 2007-12-16 08:49:05 PST
Actually, I'm quite sure that parseFloat works correctly. It seems that literals are parsed wrong!

In lexer.cpp, see:
    dval = strtod(m_buffer8.data(), 0L);

It should be kjs_strtod. I'd also recommend removing USE_LOCALE code from dtoa.cpp, to reduce future confusion.
Comment 5 Alexey Proskuryakov 2007-12-16 09:09:17 PST
Created attachment 17933 [details]
proposed fix

This bug could affect Mac, too, if an application using JavaScriptCore has set a C locale.

We also use strtol in date_object.cpp, but they all seem to be safe (it's only whitespace-skipping that can be affected AFAIK).
Comment 6 Alexey Proskuryakov 2007-12-16 09:11:25 PST
Note: I didn't test this.
Comment 7 Darin Adler 2007-12-16 09:19:39 PST
Comment on attachment 17933 [details]
proposed fix

r=me
Comment 8 Darin Adler 2007-12-16 09:20:18 PST
(In reply to comment #6)
> Note: I didn't test this.

Oh, we should add a regression test then!
Comment 9 Alexey Proskuryakov 2007-12-16 09:32:00 PST
Committed revision 28771.
Comment 10 Alexey Proskuryakov 2007-12-16 10:15:23 PST
I would expect a lot of js tests to fail on Linux with non-U.S. locales before this patch.