Bug 154530

This is likely the same issue as Bug 152448. It is because Intl uses POSIX locale. $ LANG=en_US.UTF-8 ./jsc -e "print(Infinity.toLocaleString())" ∞ $ LANG=en_US_POSIX.UTF-8 ./jsc -e "print(Infinity.toLocaleString())" INF The proposed patch in Bug 152448 should fix it.

The patch for Bug 152448 likely hasn't resolved this issue, because it only provided a hook for defaultLanguage that is implemented in WebCore.

There are two separate questions here: One is how to make WebKit and JavaScriptCore properly choose a default locale for each platform. I don’t know what the right way to do that is for Linux in general. Fixing that for WebCore is what bug 152448 is all about. But it almost certainly needs to be fixed for direct JavaScriptCore use too. At least I know it does need to be done on the Apple platforms. So we may need a bug to track that. The other is how to make the test harness set a consistent language so that tests are reliable, rather than being sensitive to the language the user running the tests happens to prefer. That's something we need to address separately. There are lots of calls to setenv in various test tools, so it could be as simple as doing a setenv("LANG") in the right place for the appropriate platforms in some of the test tools.

(In reply to comment #3) > There are two separate questions here: > > One is how to make WebKit and JavaScriptCore properly choose a default > locale for each platform. I don’t know what the right way to do that is for > Linux in general. Fixing that for WebCore is what bug 152448 is all about. > But it almost certainly needs to be fixed for direct JavaScriptCore use too. > At least I know it does need to be done on the Apple platforms. So we may > need a bug to track that. > With glibc (GNU/Linux) the default locale is defined on the environment variable LANG. If LANG is not defined, then the locale defaults to 'C' (also known as 'POSIX') which is the default portable locale. This default locale can be overridden with the environment variable LC_ALL. So perhaps we can just do setenv("LC_ALL", "C") before starting the tests. This will override any locale defined by the user, and it will be portable across operating systems Some quick tests: # Linux $ LANG=es_ES.UTF-8 WebKitBuild/Release/bin/jsc -e "print(Infinity.toLocaleString())" ∞ $ LC_ALL=POSIX LANG=es_ES.UTF-8 WebKitBuild/Release/bin/jsc -e "print(Infinity.toLocaleString())" INF $ LC_ALL=C LANG=es_ES.UTF-8 WebKitBuild/Release/bin/jsc -e "print(Infinity.toLocaleString())" INF # MacOS $ LANG=es_ES.UTF-8 WebKitBuild//Release/JavaScriptCore.framework/Versions/A/Resources/jsc -e "print(Infinity.toLocaleString())" ∞ $ LC_ALL=POSIX LANG=es_ES.UTF-8 WebKitBuild/Release/JavaScriptCore.framework/Versions/A/Resources/jsc -e "print(Infinity.toLocaleString())" INF $ LC_ALL=C LANG=es_ES.UTF-8 WebKitBuild/Release/JavaScriptCore.framework/Versions/A/Resources/jsc -e "print(Infinity.toLocaleString())" INF > The other is how to make the test harness set a consistent language so that > tests are reliable, rather than being sensitive to the language the user > running the tests happens to prefer. That's something we need to address > separately. There are lots of calls to setenv in various test tools, so it > could be as simple as doing a setenv("LANG") in the right place for the > appropriate platforms in some of the test tools. As explained above, just doing setenv("LC_ALL", "C") before starting the tests should work for all use cases. More info: http://man7.org/linux/man-pages/man3/setlocale.3.html and https://developer.apple.com/library/mac/documentation/Darwin/Reference/ManPages/man1/locale.1.html

Seems that my previous suggestion of setting LC_ALL is not going to help. This test js/intl-numberformat.html is manually setting the locales and trying some numeric and monetary conversions and checking that it returns the right values. For example see the follow examples: linux # WebKitBuild/Release/bin/jsc -e "print(Intl.NumberFormat('es').format(1234.567))" 1 234,567 linux # $ WebKitBuild/Release/bin/jsc -e "print(Intl.NumberFormat('en').format(1234.567))" 1,234.567 macos # WebKitBuild/Release/JavaScriptCore.framework/Versions/A/Resources/jsc -e "print(Intl.NumberFormat('es').format(1234.567))" 1.234,567 macos # WebKitBuild/Release/JavaScriptCore.framework/Versions/A/Resources/jsc -e "print(Intl.NumberFormat('en').format(1234.567))" 1,234.567 So it seems there is fundamental difference here. In MacOS we are getting a '.' in the thousands decimal mark for the spanish locale (right), meanwhile in Linux we are getting an space (wrong). The above checks are part of the test js/intl-numberformat.html therefore on Linux we are getting: -PASS Intl.NumberFormat('es').format(1234.567) is '1.234,567' +FAIL Intl.NumberFormat('es').format(1234.567) should be 1.234,567. Was 1 234,567. Not sure what is wrong, at least with printf on Linux I get the right values LC_NUMERIC=en_US.UTF8 /usr/bin/printf "%'.2f\n" '1234.567' 1,234.57 LC_NUMERIC=es_ES.UTF8 /usr/bin/printf "%'.2f\n" '1234.567' 1.234,57 So this don't seems a problem on the Linux glibc, but at first it looks that something on jsc is not picking/setting the right values for the locales when running on Linux This is the full text difference for the test js/intl-numberformat.html with WebKitGTK+ (Linux): --- /home/clopez/webkit/webkit/layout-test-results/js/intl-numberformat-expected.txt +++ /home/clopez/webkit/webkit/layout-test-results/js/intl-numberformat-actual.txt @@ -140,7 +140,7 @@ PASS Intl.NumberFormat('en').format(Number.MIN_VALUE) is '0' PASS Intl.NumberFormat('en').format(Number.MAX_VALUE) is '179,769,313,486,232,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000' PASS Intl.NumberFormat('en').format(1234.567) is '1,234.567' -PASS Intl.NumberFormat('es').format(1234.567) is '1.234,567' +FAIL Intl.NumberFormat('es').format(1234.567) should be 1.234,567. Was 1 234,567. PASS Intl.NumberFormat('fr').format(1234.567) is '1 234,567' PASS Intl.NumberFormat('en-u-nu-latn').format(1234.567) is '1,234.567' PASS Intl.NumberFormat('en-u-nu-fullwide').format(1234.567) is '１,２３４.５６７' @@ -156,14 +156,14 @@ PASS Intl.NumberFormat('en', {style: 'currency', currency: 'USD'}).format(Infinity) is '$∞' PASS Intl.NumberFormat('en', {style: 'currency', currency: 'JPY'}).format(4.2) is '¥4' PASS Intl.NumberFormat('en', {style: 'currency', currency: 'xXx'}).format(4.2) is 'XXX4.20' -PASS Intl.NumberFormat('en', {style: 'currency', currency: 'USD', currencyDisplay: 'code'}).format(4) is 'USD4.00' +FAIL Intl.NumberFormat('en', {style: 'currency', currency: 'USD', currencyDisplay: 'code'}).format(4) should be USD4.00. Threw exception Error: Failed to format a number. PASS Intl.NumberFormat('en', {style: 'currency', currency: 'USD', currencyDisplay: 'symbol'}).format(4) is '$4.00' -PASS Intl.NumberFormat('en', {style: 'currency', currency: 'USD', currencyDisplay: 'name'}).format(4) is '4.00 US dollars' -PASS Intl.NumberFormat('en', {style: 'currency', currency: 'JPY', currencyDisplay: 'code'}).format(-4.2) is '-JPY4' +FAIL Intl.NumberFormat('en', {style: 'currency', currency: 'USD', currencyDisplay: 'name'}).format(4) should be 4.00 US dollars. Threw exception Error: Failed to format a number. +FAIL Intl.NumberFormat('en', {style: 'currency', currency: 'JPY', currencyDisplay: 'code'}).format(-4.2) should be -JPY4. Threw exception Error: Failed to format a number. PASS Intl.NumberFormat('en', {style: 'currency', currency: 'JPY', currencyDisplay: 'symbol'}).format(-4.2) is '-¥4' -PASS Intl.NumberFormat('en', {style: 'currency', currency: 'JPY', currencyDisplay: 'name'}).format(-4.2) is '-4 Japanese yen' -PASS Intl.NumberFormat('fr', {style: 'currency', currency: 'USD', currencyDisplay: 'name'}).format(4) is '4,00 dollars des États-Unis' -PASS Intl.NumberFormat('fr', {style: 'currency', currency: 'JPY', currencyDisplay: 'name'}).format(4) is '4 yens japonais' +FAIL Intl.NumberFormat('en', {style: 'currency', currency: 'JPY', currencyDisplay: 'name'}).format(-4.2) should be -4 Japanese yen. Threw exception Error: Failed to format a number. +FAIL Intl.NumberFormat('fr', {style: 'currency', currency: 'USD', currencyDisplay: 'name'}).format(4) should be 4,00 dollars des États-Unis. Threw exception Error: Failed to format a number. +FAIL Intl.NumberFormat('fr', {style: 'currency', currency: 'JPY', currencyDisplay: 'name'}).format(4) should be 4 yens japonais. Threw exception Error: Failed to format a number. PASS Intl.NumberFormat('en', {minimumIntegerDigits: 4}).format(12) is '0,012' PASS Intl.NumberFormat('en', {minimumIntegerDigits: 4}).format(12345) is '12,345' PASS Intl.NumberFormat('en', {minimumFractionDigits: 3}).format(1) is '1.000' @@ -184,7 +184,7 @@ PASS Intl.NumberFormat('en', {maximumSignificantDigits: 4}).format(0.1234567) is '0.1235' PASS Intl.NumberFormat('en', {maximumSignificantDigits: 4}).format(1234567) is '1,235,000' PASS Intl.NumberFormat('en', {useGrouping: true}).format(1234567.123) is '1,234,567.123' -PASS Intl.NumberFormat('es', {useGrouping: true}).format(1234567.123) is '1.234.567,123' +FAIL Intl.NumberFormat('es', {useGrouping: true}).format(1234567.123) should be 1.234.567,123. Was 1 234 567,123. PASS Intl.NumberFormat('fr', {useGrouping: true}).format(1234567.123) is '1 234 567,123' PASS Intl.NumberFormat('en', {useGrouping: false}).format(1234567.123) is '1234567.123' PASS Intl.NumberFormat('es', {useGrouping: false}).format(1234567.123) is '1234567,123'

Many other tests started to fail after r201066, but the problem seems to be the same. run-javascriptcore-tests does $ENV{LANG}="en_US.UTF-8"; but we are not actually honoring the environment variables at all when using jsc binary. We are using setlocale with a nullptr locale to get the current one, but the current one is always "C", because to set the locale according to the environment variables we need to call setlocale with an empty string as locale. That's done by gtk_init() which is called by all our binaries (web process, network process, etc.), but not by jsc (because jsc doesn't depend on GTK+). The reason why it has always worked for EFL is because they call ecore_init() in jsc that calls setlocale. I'll submit a patch to fix this and the new tests failing.

Created attachment 279758 [details] Patch I'm not sure this is also needed for jsc-only port. In that case it would be a matter of updating the ifdef

Comment on attachment 279758 [details] Patch OK. Not sure this is the right way to do things. Seems unfortunate to have significant platform dependencies in a command line tool. Unpleasant to have iOS, EFL, Windows, and GTK specifics. I suppose I should be grateful we don’t have anything like this for Mac yet?

hmm, I was wrong, even with this patch intl-numberformat.js is filing, but all others that started to fail after r201066 are passing now, so I'll submit a separate bug.

(In reply to comment #7) > Created attachment 279758 [details] > Patch > > I'm not sure this is also needed for jsc-only port. In that case it would be > a matter of updating the ifdef I'll check if it fixes failures on jsc-only ARM bots. ( without guard, of course :) ) Could you wait ~2-3 hours for my testing before landing?

Comment on attachment 279758 [details] Patch Clearing flags, I moved the patch to bug #158066

(In reply to comment #10) > (In reply to comment #7) > > Created attachment 279758 [details] > > Patch > > > > I'm not sure this is also needed for jsc-only port. In that case it would be > > a matter of updating the ifdef > > I'll check if it fixes failures on jsc-only ARM bots. > ( without guard, of course :) ) > > Could you wait ~2-3 hours for my testing before landing? Sure

intl-numberformat.js works for me locally, but not in the bots

In my laptop: $ LANG=en_US.UTF-8 bin/jsc -e "print(Intl.NumberFormat('es', {useGrouping: true}).format(1234567.123))" 1.234.567,123 In the bot: $ LANG=en_US.UTF-8 bin/jsc -e "print(Intl.NumberFormat('es', {useGrouping: true}).format(1234567.123))" 1 234 567,123 There must be something else, doesn't look like a WebKit problem.

Are we using ICU for this? could it be the ICU version? it's the only (relevant) thing that looks different between my laptop and the bot

Created attachment 279882 [details] Patch for GTK+ So it was indeed the ICU version installed in the bots. Confirmed that using a newer version in the bots fixes the tests.

I tried the patch on a clean build in a tests bot we have and it worked, so I'm not sure about he EWS failure, maybe a clan build would fix it.

Comment on attachment 279882 [details] Patch for GTK+ Please look at the compile failure in GTK EWS.

Darin, I think it's a clean build issue, because I've tried in a test bot and it worked. I'll trigger clean builds in the bots and watch them after landing.

Committed r201449: <http://trac.webkit.org/changeset/201449>

While your change solves the JSC tests (Great!!), the layout-tests in GTK ports pose many new failing tests instead. Do you already know the reason?

Ah, no, these tests are previously failing. Sorry.

It broke the GTK ARM bot.

(In reply to comment #23) > It broke the GTK ARM bot. I already talked to Gustavo, who was going to look at it. For some reason wget is failing to download from sourceforge in the ARM bot. In any case, it's a problem of the bot itself, not the patch.

Committed r201509: <http://trac.webkit.org/changeset/201509>