455738 - Add support for new Vista locales

Status: RESOLVED FIXED

(Core :: Internationalization, defect)

Description

[Rain [:rain1] (inactive)]

Windows Vista has several new locales that aren't currently supported by Mozilla code. They aren't present in the locale table at <http://mxr.mozilla.org/mozilla-central/source/intl/locale/src/windows/nsIWin32LocaleImpl.cpp#158>, so people in these locales have their dates and times formatted incorrectly, amongst other issues.

One way would be to just add the new locales that can be added to the table. Another possible option would be to investigate adding support for using locale names on Vista instead of locale identifiers (locale identifiers aren't unique to locales any more). There are -Ex functions and GetSystemDefaultLocaleName / GetUserDefaultLocaleName to support locale names on Vista.

GetSystemDefaultLocaleName: <http://msdn.microsoft.com/en-us/library/ms776369(VS.85).aspx>
GetUserDefaultLocaleName: <http://msdn.microsoft.com/en-us/library/ms776371(VS.85).aspx>

Comment 1

[Rain [:rain1] (inactive)]

More potentially useful functions, these basically do whatever's done in GetPlatformLocale/GetXPLocale.

LCIDToLocaleName: <http://msdn.microsoft.com/en-us/library/ms776386(VS.85).aspx>
LocaleNameToLCID: <http://msdn.microsoft.com/en-us/library/ms776388(VS.85).aspx>

Comment 2

[Rain [:rain1] (inactive)]

Attachment: Patch to call LocaleNameToLCID/LCIDToLocaleName if available

Comment 3

[Rain [:rain1] (inactive)]

Comment on attachment 343286 [details] [diff] [review]
Patch to call LocaleNameToLCID/LCIDToLocaleName if available

I've tested this patch, and it works as expected on Vista. Haven't tested on XP, though I don't think it should change anything there.

Comment 4

[Simon Montagu :smontagu]

Comment on attachment 343286 [details] [diff] [review]
Patch to call LocaleNameToLCID/LCIDToLocaleName if available

r+moa=smontagu. It would be nice to have some tests for this, particularly to ensure that we get the same results on <=XP and Vista for the locales that are special-cased in the existing code, but I don't know if that is practical.

Comment 5

[Rain [:rain1] (inactive)]

It seems like those function don't support some locales that the current supports, should I add a fallback to that instead of just returning NS_ERROR_FAILURE?

Comment 6

[Simon Montagu :smontagu]

Yes, please add the fallback.

Comment 7

[Rain [:rain1] (inactive)]

Attachment: Updated patch, with fallback

I've also changed the LPWSTR to LPCWSTR, as that's what the actual function definition is, despite what MSDN says. That eliminated the need for the clone data function.

Comment 8

[Rain [:rain1] (inactive)]

Attachment: Updated patch, using nsAutoString instead of a direct pointer [Checkin: Comment 9]

I made a mistake -- nsAString isn't guaranteed to be null-terminated.

About tests: since this interface isn't scriptable, the only way to do this seems to be to write a C++ test.

Comment 9

[Serge Gautherie (:sgautherie)]

Comment on attachment 343370 [details] [diff] [review]
Updated patch, using nsAutoString instead of a direct pointer
[Checkin: Comment 9]

http://hg.mozilla.org/mozilla-central/rev/0e4ab2fd22e7

Comment 10

[Brian Crowder]

Is there a good alternative for this sort of thing on Windows Mobile, or should I just #ifndef WINCE this code?

Comment 11

[[e-mail]]

Comment on attachment 343370 [details] [diff] [review]
Updated patch, using nsAutoString instead of a direct pointer
[Checkin: Comment 9]

>+    WCHAR ret_locale[LOCALE_NAME_MAX_LENGTH];
>+    int rv = lcidToLocaleName(winLCID, ret_locale, LOCALE_NAME_MAX_LENGTH, 0);
This must be a pretty new constant, my SDK doesn't have it...

Comment 12

[Brian Crowder]

Yeah, windows mobile doesn't have it at all.

Comment 13

[Rain [:rain1] (inactive)]

Attachment: Define LOCALE_NAME_MAX_LENGTH if it hasn't already been done [Checkin: Comment 16]

Another approach is to put the offending block under an ifndef MOZ_DISABLE_VISTA_SDK_REQUIREMENTS.

smontagu: sorry about bugging you for another review :(

Comment 14

[Serge Gautherie (:sgautherie)]

Same error
{
.../intl/locale/src/windows/nsIWin32LocaleImpl.cpp(688) : error C2065: 'LOCALE_NAME_MAX_LENGTH' : undeclared identifier
}
on my Windows 2000 with
{
Visual C++ 8 Express directory: [...]\Microsoft Visual Studio 8\VC
Platform SDK directory: [...]\Microsoft Platform SDK for Windows Server 2003 R2
}

Comment 15

[Simon Montagu :smontagu]

Comment on attachment 343517 [details] [diff] [review]
Define LOCALE_NAME_MAX_LENGTH if it hasn't already been done
[Checkin: Comment 16]

r+moa=smontagu

Comment 16

[Serge Gautherie (:sgautherie)]

Comment on attachment 343517 [details] [diff] [review]
Define LOCALE_NAME_MAX_LENGTH if it hasn't already been done
[Checkin: Comment 16]

http://hg.mozilla.org/mozilla-central/rev/28fc43e4394c

Comment 17

[Serge Gautherie (:sgautherie)]

(In reply to comment #13)
> Another approach is to put the offending block under an ifndef
> MOZ_DISABLE_VISTA_SDK_REQUIREMENTS.

Maybe be this could be added as a comment ?

Comment 18

[Rain [:rain1] (inactive)]

(In reply to comment #17)
> 
> Maybe be this could be added as a comment ?

I don't think that's necessary, as the file already includes several other #ifdefs which aren't there in older SDKs.