558394 - Thousands separators display as apostrophes in the page info window

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[u88484]

Attachment: Screenshot of trunk and 3.6

Commas are displayed as apostrophes in the page info window.  See the screenshot for a comparison of Firefox 3.6.3 and the latest trunk.

Comment 2

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

I see this as well. Have no idea when it regressed.

Is it all commas, or just for the thousands separators? (Would an alt-text that includes a comma show up as an apostrophe? I don't have time to check that at the moment.)

Comment 3

[Jean-Yves Perrier [:teoli]]

Note that this should obey the locale. I expect to see apostrophes in that case as my locale is defined so (and is its default).

Comment 4

[Warner Young]

Commas in alt-text are fine.  It seems to affect only the thousands separators, as far as I can tell.

Comment 5

[Philip Chee]

This affects SeaMonkey trunk as well so something in number.toLocaleString()changed.

Comment 6

[Tanner M. Young [:tmyoung]]

My guess would be a regression around here: http://mxr.mozilla.org/mozilla-central/source/js/src/jsnum.cpp#424

This is a simple testcase for it: javascript:var test = 1000000;document.write(test.toLocaleString());

Comment 7

[Tanner M. Young [:tmyoung]]

Attachment: Testcase

Here is an attached version of the testcase in html.

Comment 8

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

I think the problem is using ifndef HAVE_LOCALECONV when it should be ifdef.

Comment 9

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Basing that on https://bugzilla.mozilla.org/show_bug.cgi?id=542146#c6, I haven't actually tested this or anything.

Comment 10

[Tanner M. Young [:tmyoung]]

I am quickly running a build to test Kyle's theory.

Comment 11

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

bz points out that the issue is not ifndef, but rather HAVE_LOCALECONV.  It needs to be defined on all non-Android platforms.

Comment 12

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

So the issue appears to be that AC_HAVE_FUNC(localeconv) does not find localeconv on Windows (other platforms too, maybe?) even though it's definitely in locale.h ...

Comment 14

[Michael Wu [:mwu]]

Attachment: s/HAVE_LOCALECONV/ANDROID/ for now

I won't be able to investigate the autoconf brokenness on windows until later this week, so here's a patch to special case localeconv for the only current user of this ifdef.

Comment 15

[Jim Blandy :jimb]

I don't think a patch like that should ever land; the people who are inconvenienced by it should apply it locally for a few days.

Comment 16

[Jim Blandy :jimb]

Attachment: Define HAVE_LOCALECONV on Windows.

Comment 17

[Jim Blandy :jimb]

That would be the fix I'd recommend.

Comment 18

[Jason Orendorff [:jorendorff]]

Comment on attachment 438542 [details] [diff] [review]
Define HAVE_LOCALECONV on Windows.

Well, that seems marginally better-- but why do we SKIP_COMPILER_CHECKS and SKIP_LIBRARY_CHECKS on Windows? Isn't that kind of bad?

(Also unrelated to the specific bug at hand, it seems a little odd what we're doing with HAVE_SYSTEMTIMETOFILETIME in the context there.)

Comment 19

[Michael Wu [:mwu]]

Fine with me. I probably would have done the same.

Comment 20

[Jim Blandy :jimb]

Autoconf can't run the VS compiler correctly, so we don't do any dynamic detection on Windows.  Is my understanding.

At one point I thought I had a hack that would make it work; bsmedberg's reaction was a mildly surprised "that would be nice".  But I never pursued it.

Comment 21

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/1cdb9c6e0206

Comment 22

[u88484]

Verified fixed using Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a5pre) Gecko/20100424 Minefield/3.7a5pre

Comment 23

[(not currently active) Ted Mielczarek]

(In reply to comment #18)
> (From update of attachment 438542 [details] [diff] [review])
> Well, that seems marginally better-- but why do we SKIP_COMPILER_CHECKS and
> SKIP_LIBRARY_CHECKS on Windows? Isn't that kind of bad?

It's a pain, but it's not really bad, since there aren't that many variants of VC++ floating around. It's not generally that hard to hardcode the right answers for VC++ in configure.

(Also even if we could get them working, configure tests are *much* slower on Windows with msys, so it'd make our already-slow configure run even worse there.)