Closed
Bug 668756
Opened 13 years ago
Closed 11 years ago
decimal separator according to TB locale version
Categories
(Thunderbird :: Mail Window Front End, defect)
Thunderbird
Mail Window Front End
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 19.0
People
(Reporter: mstanke, Assigned: aceman)
References
Details
(Keywords: polish, ux-consistency)
Attachments
(2 files, 2 obsolete files)
9.58 KB,
image/png
|
Details | |
3.48 KB,
patch
|
standard8
:
review+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (Windows NT 5.1; rv:5.0) Gecko/20100101 Firefox/5.0 Build ID: 20110615151330 Actual results: Wherever TB shows a filesize, decimal separator is used. But in some countries people are using decimal "comma" instead of decimal point. TB uses decimal point always and everywhere. (reported http://forum.mozilla.cz/viewtopic.php?f=4&t=4385) Expected results: The decimal separators should be 'localizable' or customizable - '.' or ','.
Updated•13 years ago
|
Status: UNCONFIRMED → NEW
Component: General → Mail Window Front End
Ever confirmed: true
Keywords: polish,
ux-consistency
QA Contact: general → front-end
Reporter | ||
Updated•13 years ago
|
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → DUPLICATE
Comment 2•13 years ago
|
||
Not a duplicate. Bug 161116 is a core issue, and (probably) wouldn't fix this one. I have a fix in mind for this, but it's super-hacky (basically find "." in the size and replace it with "," if need be). Incidentally, that's how Firefox handles the decimal in file sizes too.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
I'll implements squib's solution in comment 2 if there is no opposition.
Assignee: nobody → acelists
OS: Other → All
Attachment #648048 -
Flags: review?(squibblyflabbetydoo)
Comment on attachment 648048 [details] [diff] [review] patch Neil, please see if this fixes the problem in Seamonkey too.
Attachment #648048 -
Flags: review?(neil)
Comment 7•12 years ago
|
||
Comment on attachment 648048 [details] [diff] [review] patch Review of attachment 648048 [details] [diff] [review]: ----------------------------------------------------------------- We should probably get the decimal separator from the system, like so: http://mxr.mozilla.org/comm-central/source/mozilla/js/src/jsnum.cpp#1069 We'll also want to check if the thousands separator is used (try formatFileSize on something >1000 GB), and convert that if so.
Attachment #648048 -
Flags: review?(squibblyflabbetydoo) → review-
I am not sure ssnprintf will return anything containing a thousands separator. Also, what can be 1000GB insize TB? :)
Comment 9•12 years ago
|
||
(In reply to :aceman from comment #8) > I am not sure ssnprintf will return anything containing a thousands > separator. Also, what can be 1000GB insize TB? :) It turns out there's an off-by-one error in formatFileSize, so >999.5 GB sizes can't be printed. I fixed that in a local patch, which I will post later today. However, you don't need to worry about the thousands separator.
![]() |
Assignee | |
Comment 10•12 years ago
|
||
OK, this almost works, however I do not know how to get HAVE_LOCALECONV defined. It seems undefined on my Linux build.
Attachment #648048 -
Attachment is obsolete: true
Attachment #648048 -
Flags: review?(neil)
Attachment #648073 -
Flags: feedback?(squibblyflabbetydoo)
Comment 11•12 years ago
|
||
Comment on attachment 648073 [details] [diff] [review] patch v2 This seems ok. Standard8 might know more about the build system and what we need to do for that...
Attachment #648073 -
Flags: feedback?(squibblyflabbetydoo) → feedback+
![]() |
Assignee | |
Comment 12•12 years ago
|
||
Comment on attachment 648073 [details] [diff] [review] patch v2 The feature worked in my Linux build when I loaded localeconv() unconditionally (no ifdef). But I have no idea how to get proper HAVE_LOCALECONV value from the build system.
Attachment #648073 -
Flags: feedback?(mbanner)
Comment 13•12 years ago
|
||
Filed bug 779736 for the issue I mentioned in comment 9.
![]() |
Assignee | |
Comment 14•12 years ago
|
||
Thanks. Your patch will bitrot me so I set a dependency.
Depends on: 779736
![]() |
||
Comment 15•12 years ago
|
||
This might be overkill http://mxr.mozilla.org/comm-central/source/mozilla/gfx/cairo/cairo/src/cairo-output-stream.c#293
![]() |
||
Comment 16•12 years ago
|
||
> But I have no idea how to get proper HAVE_LOCALECONV value from the build system. Perhaps something like this? http://mxr.mozilla.org/comm-central/source/mozilla/configure.in#3790
Status: REOPENED → ASSIGNED
Comment 17•12 years ago
|
||
Comment on attachment 648073 [details] [diff] [review] patch v2 I'd support this, Philip is right, that you'd need to port something like: http://hg.mozilla.org/mozilla-central/annotate/c676b554c7bb//configure.in#l3737 to the c-c configure (which is actually part of the patch from bug 565089).
Attachment #648073 -
Flags: feedback?(mbanner) → feedback+
Comment 18•12 years ago
|
||
Comment on attachment 648073 [details] [diff] [review] patch v2 >+ nsString decimalSeparator = NS_ConvertUTF8toUTF16(nsDependentCString(decimalPoint)); Nit: nsDependentCString isn't null-safe.
![]() |
Assignee | |
Comment 19•11 years ago
|
||
(In reply to neil@parkwaycc.co.uk from comment #18) > >+ nsString decimalSeparator = NS_ConvertUTF8toUTF16(nsDependentCString(decimalPoint)); > Nit: nsDependentCString isn't null-safe. So what would be the solution to that?
![]() |
Assignee | |
Comment 20•11 years ago
|
||
Neil?
![]() |
Assignee | |
Comment 21•11 years ago
|
||
Comment on attachment 648073 [details] [diff] [review] patch v2 (In reply to neil@parkwaycc.co.uk from comment #18) > >+ nsString decimalSeparator = NS_ConvertUTF8toUTF16(nsDependentCString(decimalPoint)); > Nit: nsDependentCString isn't null-safe. So what would be the solution to that?
Attachment #648073 -
Flags: feedback?(neil)
Comment 22•11 years ago
|
||
Comment on attachment 648073 [details] [diff] [review] patch v2 >+ // The ssprintf may return a decimal number using a dot (.) as the decimal >+ // separator. Now we try to localize the separator. Nit: no point localising the separator if there is no dot. >+ nsString decimalSeparator = NS_ConvertUTF8toUTF16(nsDependentCString(decimalPoint)); NS_ConvertUTF8toUTF16 decimalSeparator(decimalPoint);
Attachment #648073 -
Flags: feedback?(neil) → feedback+
![]() |
Assignee | |
Comment 23•11 years ago
|
||
Attachment #648073 -
Attachment is obsolete: true
Attachment #666257 -
Flags: review?(mbanner)
Updated•11 years ago
|
Attachment #666257 -
Flags: review?(mbanner) → review+
Keywords: checkin-needed
Comment 24•11 years ago
|
||
https://hg.mozilla.org/comm-central/rev/f993893786c0 Seems like this is something that should have tests?
Status: ASSIGNED → RESOLVED
Closed: 13 years ago → 11 years ago
Flags: in-testsuite?
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 19.0
Comment 25•11 years ago
|
||
Also see Bug 819271 on this, the test_formatFileSize.js xpcshell test fails on systems which use a locale that does not use "." as decimal separator. Maybe that test can be modified to check different locales.
You need to log in
before you can comment on or make changes to this bug.
Description
•