544414 - Use locale info when converting numbers to string in JS on Android

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Michael Wu [:mwu]]

Android doesn't have a useful localeconv so Number.prototype.toLocaleString can't be handled correctly. Need some kind of workaround.

Comment 1

[Alex Pakhotin (:alexp)]

As a possible solution we could add some environment variables, like LOCALE_DECIMAL_POINT, LOCALE_THOUSANDS_SEP, LOCALE_GROUPING, and check them here: http://mxr.mozilla.org/mozilla-central/source/js/src/jsnum.cpp#1010

I've been also thinking about having some kind of a shunt like we had for WinCE. But this would make sense only if there are some other things, which could be implemented there.

Any other suggestions?

Comment 2

[Alex Pakhotin (:alexp)]

Attachment: Fix

Proposed fix using environment variables.

Comment 3

[Alex Pakhotin (:alexp)]

Changed description to be more specific for this one as there is another meta bug 616377.

Comment 4

[Alex Pakhotin (:alexp)]

Comment on attachment 493477 [details] [diff] [review]
Fix

After further investigation of related bugs this fix looks like a sensible approach. Let's review it.
Someone from JS team will need to have a look at this as well.

Comment 5

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 493477 [details] [diff] [review]
Fix

> #ifndef HAVE_LOCALECONV
>-    rt->thousandsSeparator = JS_strdup(cx, "'");
>-    rt->decimalSeparator = JS_strdup(cx, ".");
>-    rt->numGrouping = JS_strdup(cx, "\3\0");
>+    const char* thousands_sep = getenv("LOCALE_THOUSANDS_SEP");
>+    const char* decimal_point = getenv("LOCALE_DECIMAL_POINT");
>+    const char* grouping = getenv("LOCALE_GROUPING");
>+
>+    rt->thousandsSeparator =
>+        JS_strdup(cx, thousands_sep ? thousands_sep : "'");
>+    rt->decimalSeparator =
>+        JS_strdup(cx, decimal_point ? decimal_point : ".");
>+    rt->numGrouping =
>+        JS_strdup(cx, grouping ? grouping : "\3\0");
we might only want to do this for android (i.e. #ifdef ANDROID), but someone from the js team should make that call

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

this was pushed:
http://hg.mozilla.org/mozilla-central/rev/06a91c14588d