22310 - Locale conversion to support 3 letter language

Status: RESOLVED FIXED

(Core :: Internationalization, defect)

Description

[nhottanscp]

Platform locale (e.g., LCID) to the XP locale (internally used) conversion needs
change to support 3 letter language (ISO-639-2).
There are platform specific changes needed.
Unix - change not to assume 2 letters for language
Windows/Macintosh - change the mapping table to be extensible

Comment 1

[Alexey Chernyak]

*** Bug 69561 has been marked as a duplicate of this bug. ***

Comment 2

[Jungshik Shin]

I may have filed another bug on the same issue.

Comment 3

[Jesper Kristensen]

Attachment: Windows locale parser [Checkin: Comment 6]

Add support for locales ja-JP-mac, kok-IN, nn-NO and nb-NO and allow error handling on Windows. Patch copied from bug 441167.

Comment 4

[Jesper Kristensen]

now what is next? should it be checked in or something else? by who?

Comment 5

[Jesper Kristensen]

Attachment: Windows locale parser test

This is the JavaScript I used to test the Windows patch.

The Mac code seems quite similar to the Windows code, so I guess it needs some fixing too, but I don't have a Mac, so I cannot test.

I tried a script similar to this one on Linux, but it seems to return the date in a machine readable form ignoring the locale completely, but at least it gives a date for all tested locales.

Comment 6

[Reed Loden [:reed]]

Pushed in 15867:085edeb5e56d.

Comment 7

[Jesper Kristensen]

reed: my patch was Windows-only. Linux already worked, but it looks like Mac still needs a patch. Correct me if I a am wrong, but I don't think this is fixed on Mac.

Comment 8

[Atsushi Sakai]

(In reply to comment #7)
> I don't think this is
> fixed on Mac.

That's right. Mac still has problem.
"ja-JP-mac" should be converted to "ja-JP".

Requesting blocking because Bug 441167 has been exposed this issue and causes regression.
(See Bug 472666)

Comment 10

[Jesper Kristensen]

Attachment: Mac OS locale parser (not tested)

This is a direct conversion of the Windows patch to the Mac code. As I don't have a Mac, I haven't tested it. It might not even compile.

Comment 11

[Atsushi Sakai]

(In reply to comment #10)
> +    if (strcmp(lang_list[i].iso_code, language_code)==0) {
>        *scriptCode = lang_list[i].mac_script_code;
>        *langCode = lang_list[i].mac_lang_code;
>        return NS_OK;
>      }

I have not tested it either, but I guess |*regionCode| should be initialized.

Comment 12

[Atsushi Sakai]

(In reply to comment #10)
> Created an attachment (id=356005) [details]
> Mac OS locale parser (not tested)

The patch does not compile.

intl/locale/src/mac/nsMacLocale.cpp: In member function ‘virtual nsresult nsMacLocale::GetPlatformLocale(const nsAString_internal&, short int*, short int*, short int*)’:
intl/locale/src/mac/nsMacLocale.cpp:244: error: ‘j’ was not declared in this scope
intl/locale/src/mac/nsMacLocale.cpp:244: error: ‘NS_StringGetData’ was not declared in this scope

Comment 13

[Jesper Kristensen]

(In reply to comment #12)
> The patch does not compile.

As I said: I haven't tried, and I can't.

Comment 14

[Atsushi Sakai]

(In reply to comment #11)
> I have not tested it either, but I guess |*regionCode| should be initialized.

Filed this issue as Bug 472960.

I think Bug 441167 should be backed out (or add #ifndef XP_MACOSX) unless both this bug and Bug 472960 get fixed.

Comment 15

[Atsushi Sakai]

I've changed my mind.
Clearing requesting blocking on this, and nominating Bug 441167 instead.

Comment 16

[Jesper Kristensen]

I will be happy to work together to make a patch, if you like. Just mail me off bugzilla.

Comment 17

[Atsushi Sakai]

Attachment: Mac OS locale parser (modified) [Checkin: Comment 22]

Works fine for ja-JP-mac, but I don't test for other locales because I don't know expected results.

(In reply to comment #16)
> I will be happy to work together to make a patch, if you like. Just mail me off
> bugzilla.

We should use Bugzilla for other developers to see it.

Comment 18

[Jesper Kristensen]

It seems we have a patch in here. Is it ready for review?

Comment 19

[Simon Montagu :smontagu]

I tried running the testcase in attachment 327471 [details] on Mac, and I don't really follow what's happening: ja-JP outputs "2008年 6月 30日 (月) 0:56:34 PM", but all the other locales in the testcase (including ja-JP-mac) output "Mon, Jun 30, 2008 12:56:34 PM".

Comment 20

[Jesper Kristensen]

(In reply to comment #19)
Exactly which locales did you test with? Note that some of the English fallbacks might be caused by this bug while other English fallbacks might be caused by bug 472960.

Comment 21

[Simon Montagu :smontagu]

Comment on attachment 356712 [details] [diff] [review]
Mac OS locale parser (modified)
[Checkin: Comment 22]

Thanks for the pointer. I discovered that bug 472960 is fixed by the patch in bug 360018; and that with that patch all the testcases in attachment 327471 [details] and more work correctly.

So this patch is fine for now, but in the long term we probably want to rip out most of the existing nsMacLocale.cpp and any other places that use the deprecated Carbon system functions.

Comment 22

[Serge Gautherie (:sgautherie)]

Comment on attachment 356712 [details] [diff] [review]
Mac OS locale parser (modified)
[Checkin: Comment 22]


http://hg.mozilla.org/mozilla-central/rev/e42e2fbb98fa

Comment 23

[Simon Montagu :smontagu]

Attachment 327471 [details] could be the basis of an xpcshell test (though as far as I know it will pass only on Windows and Mac right now).

Comment 24

[Simon Montagu :smontagu]

Tests: http://hg.mozilla.org/mozilla-central/rev/81dc4af9e961

Comment 25

[Serge Gautherie (:sgautherie)]

(In reply to comment #24)
> Tests: http://hg.mozilla.org/mozilla-central/rev/81dc4af9e961

1.9.1 landing: bug 360018 comment 59!

*****

NB: Mac patch did not land on 1.9.1 (yet).

Comment 26

[Pulsebot]

Pushed by ffxbld-merge:
https://hg.mozilla.org/releases/mozilla-beta/rev/1e36422bad38
[fenix] For https://github.com/mozilla-mobile/fenix/issues/22310 - Change tab holder's checkbox state when pressing "Select All"/"Deselect All"