When trying to get the system locale (dates, currency etc.), GetSystemLocale always return "en-US", even when I have set he-IL in System Preferences->International->Formats. To reproduce, try the following JS code (It does work properly on win32): ---- var localService = Components.classes["@mozilla.org/intl/nslocaleservice;1"].getService(Components.interfaces.nsILocaleService); var systemLocale = localService.getSystemLocale().getCategory("NSILOCALE_MESSAGES"); alert("NSILOCALE_MESSAGES: " + systemLocale); ---- (I'll attach an XPI to test the above code soon.) Also notice I have the same result from each category here: http://lxr.mozilla.org/seamonkey/source/intl/locale/idl/nsILocale.idl#41

This adds "What's my locale?" at the end of the View menu.

This must be the cause of quite a few bugs about Date and Time format: bug #231280 and bug #233631 are just the two to mention.

(In reply to comment #2) > This must be the cause of quite a few bugs about Date and Time format: bug > #231280 and bug #233631 are just the two to mention. So how did they get fixed..!? Maybe this is for some languages only? (bug 120401 comes in mind).

(In reply to comment #3) > (In reply to comment #2) > > This must be the cause of quite a few bugs about Date and Time format: bug > > #231280 and bug #233631 are just the two to mention. > > So how did they get fixed..!? Maybe this is for some languages only? (bug 120401 > comes in mind). I think it's for some OS X distributions only. I.e. users with Japanese OS X didn't seem to have the problem with those bugs. I fixed it by excluding the explicit instruction of what locale must be used. What practical problems do you have due to this bug?

> > I think it's for some OS X distributions only. I.e. users with Japanese OS X > didn't seem to have the problem with those bugs. > I use the primary dist. > What practical problems do you have due to this bug? There is work being done for bug 85420. We are going to enable the additional UI there (see http://bidiui.mozdev.org for what UI) only for some specific system locales, this works properly on moz/win, but on OS X, I always get "en-US". Can you please check if you can port your changes for Hebrew & Arabic?

(In reply to comment #5) > Can you please check if you can port your changes for Hebrew & Arabic? My changes were on the call-sites only. Have nothing to port, sorry. :-(

Thanks to Nir Soffer for good searching abilities :-) http://developer.apple.com/documentation/CoreFoundation/Conceptual/CFLocales/Articles/CFLocaleWorking.html#//apple_ref/doc/uid/20002241/BBCEFFGC ....but it's 10.3+ only: http://developer.apple.com/documentation/CoreFoundation/Reference/CFLocaleRef/Reference/FunctionGroupIndex.html#//apple_ref/c/func/CFLocaleCopyCurrent Javier?

OK, here's what I've found: If you change the language/format in the International pref panel, then you will need to logout and back in again for the Script Manager to register the changes (the script manager calls are at http://lxr.mozilla.org/mozilla/source/intl/locale/src/nsLocaleService.cpp#283). However, even this doesn't seem to work perfectly. For example, if I keep the Language as 'English', but set the Format to 'United Kingdom', I still get back the same values as when Format was set to 'United States'. But I get the correct locale is I set it to 'Español'/'Argentina'. So I went searching for the correct way to get the user's selected locale. According to much of the discussion on the carbon-dev mailing list (and we aren't the only ones having this problem), there was never really an official way to the user's locale. From an Apple manager: > 1. If you are running on a pre-10.3 system (determined at runtime), use > the old WorldScript calls (Script Manager, Text Utilities, > Date/Time/Measurement Utilities) and convert the results to Unicode. > 2. If you are running on 10.3 or later, then use the new APIs > (CFDateTimeFormatter, CFNumberFormatter, etc.). You will need to > weak-link these so your app loads on pre-10.3 systems. So it looks like the Script Manager calls will need to stay for 10.1/10.2, but we need to update the code properly for 10.3. I'll look at putting together a patch.

Hmm, it looks like it uglier than I thought. It seems that the Script Manager calls return values based only on the selected Language in the International pref pane; changing Region in the Format tab does nothing. So every region returns 'en_US' when 'English' is set as the Language; they all return 'it_IT' when Italian is set, etc. I guess for the most part, the end user won't notice anything wrong when the Language is set correctly, but it's still not very accurate.

Thanks for working on that! Javier, users can't always choose associated lang to their prefered locale (That's becuase Mac OS X has a lot of issues with preferd language, such as problems with the Software Update app). At least on 10.3+, we need to use the new API (see comment 7).

This seems to work pretty well. CFLocaleGetIdentifier returns a string that is very close to what Mozilla wants; just need to change the underscore to a dash. The two CFLocale functions need to be weak-linked. I think that's what I did, but I can't check since I don't have 10.2 to test this on.

Javier, it works fine, Thanks! (See screenshot (i added "NSLOCALE_MESSAGES: ")).

Now, there is a problem which "PageInfo" etc. does not work in a Japanese environment.(bug 239535) Does anything improve by this correction?

Javier, can you request for r/sr?

crot0: No, this patch doesn't seem to make any difference with regards to bug 239535. But there are some other places in the code that use the Script Manager; these may be at fault. I'll investigate when I get the change.

+ if ((err == noErr) && (systemVersion >= 0x00001030)) is there a version# of foundation we can check against instead? + CFStringRef userLocaleStr = ::CFLocaleGetIdentifier(userLocaleRef); + ::CFRetain(userLocaleStr); ... + ::CFRelease(userLocaleStr); I don't get this. Why retain/release the string? Also, I was under the impression that anything with "Copy" in the name returned a retained instance (that we're responsible for releasing). That's how it generally goes in appKit, and since it's all the same under the hood... + char* p = strchr(buffer, '_'); + if (p) + *p = '-'; are we guaranteed that CFStringGetCString will null terminate the string even if it fills up the buffer?

The documentation for CFLocaleGetIdentifier says: "You are responsible for retaining and releasing it as needed." So I figured I would retain it before messing around with it, just to be safe. I have another patch that addresses the other issues.

Using a macro for the system version. Getting the unicode chars from CFStringRef rather than converting to a C string; makes the code simpler.

Comment on attachment 154783 [details] [diff] [review] patch v1.1 + if ((err == noErr) && (systemVersion >= MAC_OS_X_VERSION_10_3)) hm... Maybe I'm missing something, but is this macro defined in <= 10.2? If it is not, this is not even going to compile, isn't it? :-)

No, I think it's all right. Even the 10.1.5 SDK defines macros for version 10.0 through 10.5, so we should be alright.

Actually, can someone with MacOSX 10.2 with the developer tools installed check the file /Developer/SDKs/MacOSX10.2.8.sdk/Developer/Headers/CFMCarbon/AvailabilityMacros.h and make sure that, at the top of the file, "MAC_OS_X_VERSION_10_3" is defined.

FWIW, the SDKs don't exist on 10.2-based dev systems. Only on panther/XCode. So, no, that won't be defined on a 10.2 machine.

Removed system version macro.

Comment on attachment 154787 [details] [diff] [review] patch v1.2 >Index: nsLocaleService.cpp >+// These functions are only available in 10.3+, so weak-link to them. >+extern CFLocaleRef CFLocaleCopyCurrent(void) __attribute__((weak_import)); >+extern CFStringRef CFLocaleGetIdentifier(CFLocaleRef locale) __attribute__((weak_import)); >+ long systemVersion; >+ OSErr err = ::Gestalt(gestaltSystemVersion, &systemVersion); >+ if ((err == noErr) && (systemVersion >= 0x00001030)) How about using |CFBundleGetFunctionPointerForName| as in http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&f ile=nsLocalFileOSX.cpp&branch=&root=/cvsroot&subdir=mozilla/xpcom/io&command=DI FF_FRAMESET&rev1=1.21&rev2=1.22 ? Perhaps, that's not such a good idea here... >+ // Get string representation of locale >+ int size = ::CFStringGetLength(userLocaleStr); >+ UniChar *buffer = (UniChar*) PR_Malloc(size * sizeof(UniChar) + 1); >+ CFRange range = ::CFRangeMake(0, size); >+ ::CFStringGetCharacters(userLocaleStr, range, buffer); >+ buffer[size] = 0; >+ >+ if (buffer) >+ { >+ // Convert the locale string to the format that Mozilla expects >+ nsAutoString xpLocale(buffer); >+ xpLocale.ReplaceChar('_', '-'); >+ >+ nsresult rv = NewLocale(xpLocale, getter_AddRefs(mSystemLocale)); >+ if (NS_SUCCEEDED(rv)) { >+ mApplicationLocale = mSystemLocale; >+ } >+ >+ PR_Free(buffer); >+ } You may want to use 'nsAutoBuffer' here to avoid malloc/free. If not, three lines following |PR_Malloc| should go inside |if (buffer)|, shouldn't it?

I think I like using 'weak_import' rather than |CFBundleGetFunctionPointerForName|; the code looks cleaner to me. But if you think |CFBundleGetFunctionPointerForName| is better, I can create a patch for that too. Thanks for pointing me to nsAutoBuffer. Had forgotten about that class.

(In reply to comment #25) > Created an attachment (id=154810) > patch v1.3 > > I think I like using 'weak_import' rather than > |CFBundleGetFunctionPointerForName|; the code looks cleaner to me. Yeah, it seems so. > Thanks for pointing me to nsAutoBuffer. Had forgotten about that class. No problem. BTW, you don't need to include "prmem.h" any more.

Comment on attachment 154810 [details] [diff] [review] patch v1.3 I'll remove the prmem.h include when I checkin.

(In reply to comment #27) > (From update of attachment 154810 [details] [diff] [review]) > I'll remove the prmem.h include when I checkin. > A cleaner way to compile using weak link, is using AvailabilityMacros. Instead of: // These functions are only available in 10.3+, so weak-link to them. extern CFLocaleRef CFLocaleCopyCurrent(void) __attribute__((weak_import)); extern CFStringRef CFLocaleGetIdentifier(CFLocaleRef locale) __attribute__((weak_import)); Use: #include <AvailabilityMacros.h> extern CFLocaleRef CFLocaleCopyCurrent(void) AVAILABLE_MAC_OS_X_VERSION_10_3_AND_LATER; extern CFStringRef CFLocaleGetIdentifier(CFLocaleRef locale) AVAILABLE_MAC_OS_X_VERSION_10_3_AND_LATER; Just like its done on most Apple headers. I did not check with Mozilla, but used these few month ago. About confirming that you did get a weak link, you dont have to test on 10.2, just run nm on your executable: % nm -mg |grep frameworkname (undefined) weak external _Symbol (from frameworkname) ... More info: * http://developer.apple.com/technotes/tn2002/tn2064.html * /usr/include/AvailabilityMacros.h Hope it helps.

Nir, thanks for the link. That looks nicer.

So it turns out that weak-linking is only supported in 10.2+, so the last patch would break on 10.1. This new patch uses jshin's suggestion of manually loading those functions.

Comment on attachment 155212 [details] [diff] [review] patch v1.4 r=jshin

Jshin, any chance you can also sr it?

No, I can't. See http://www.mozilla.org/hacking/reviewers.html

Oh, i was sure your'e there :-)

maybe blizzard?

Patch checked in to trunk (with change to use function pointers rather than actual functions!). Now for aviary.

Comment on attachment 155212 [details] [diff] [review] patch v1.4 requesting approvals

The patch didn't build on the tinderbox, which has 10.2 installed (I think). So I made these changes to the file in order to get it to build: #ifdef XP_MACOSX +#if !defined(__COREFOUNDATION_CFLOCALE__) +typedef void* CFLocaleRef; +#endif typedef CFLocaleRef (*fpCFLocaleCopyCurrent_type) (void); typedef CFStringRef (*fpCFLocaleGetIdentifier_type) (CFLocaleRef); #endif |__COREFOUNDATION_CFLOCALE__| is only defined in the 10.3 SDK. I tested this on my 10.3 machine by changing all |CFLocaleRef| instances to |void*|, and it worked just fine.

Comment on attachment 155212 [details] [diff] [review] patch v1.4 a=mkaply for 1.7 and aviary

javier: please use resolution:fixed for trunk.

Checked in everywhere. ->FIXED

Thanks.