Closed Bug 558394 Opened 14 years ago Closed 14 years ago

Thousands separators display as apostrophes in the page info window

Categories

(Core :: JavaScript Engine, defect)

x86
Windows Vista
defect
Not set
normal

Tracking

()

VERIFIED FIXED
Tracking Status
blocking2.0 --- beta1+

People

(Reporter: u88484, Assigned: jimb)

References

()

Details

(Keywords: regression, Whiteboard: fixed-in-tracemonkey)

Attachments

(3 files, 1 obsolete file)

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.
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.)
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).
Commas in alt-text are fine.  It seems to affect only the thousands separators, as far as I can tell.
This affects SeaMonkey trunk as well so something in number.toLocaleString()changed.
Assignee: nobody → general
Component: Page Info → JavaScript Engine
Product: Firefox → Core
QA Contact: page.info → general
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());
Attached file Testcase
Here is an attached version of the testcase in html.
OS: Windows 7 → Windows Vista
I think the problem is using ifndef HAVE_LOCALECONV when it should be ifdef.
Assignee: general → mwu
Blocks: 542146
I am quickly running a build to test Kyle's theory.
blocking2.0: --- → ?
bz points out that the issue is not ifndef, but rather HAVE_LOCALECONV.  It needs to be defined on all non-Android platforms.
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 ...
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.
Attachment #438421 - Flags: review?(jorendorff)
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.
Attachment #438542 - Flags: review?(jorendorff)
That would be the fix I'd recommend.
Attachment #438421 - Flags: review?(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.)
Attachment #438542 - Flags: review?(jorendorff) → review+
Fine with me. I probably would have done the same.
Assignee: mwu → jim
Attachment #438421 - Attachment is obsolete: true
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.
Summary: Commas display as apostrophes in the page info window → Thousands separators display as apostrophes in the page info window
Status: NEW → ASSIGNED
Whiteboard: fixed-in-tracemonkey
blocking2.0: ? → beta1+
http://hg.mozilla.org/mozilla-central/rev/1cdb9c6e0206
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Verified fixed using Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a5pre) Gecko/20100424 Minefield/3.7a5pre
Status: RESOLVED → VERIFIED
(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.)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: