224546 - Change Win 'Regional Settings' -> lose button functions & text

Status: RESOLVED FIXED

(Core :: Internationalization, defect)

Description

[gioalhaz]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.5) Gecko/20030925
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.5) Gecko/20030925

in WinXP, I have "Regional Options" set to Georgian. The mozilla dialog buttons
are without text (title) and are not responding.

Reproducible: Always

Steps to Reproduce:
1. Set "Regional Options" to Georgian
2. Call any dilog. For examlpe "Account Wizard" from mozilla mail
3.

Actual Results:  
The first dialog of "Account Wizard" appear. The buttons on the bottom are
without text (title) and not responding

Expected Results:  
buttons must have titles "< Back", "Next >", "Cancel"

When I change "Regional Options" from Georgian to English, everything works OK.

Comment 1

[guanxi]

Attachment: Blank buttons

Comment 2

[guanxi]

Confirmed Moz 1.5 final on WinXP Pro

Would the lack of a language pack cause this bug?

Reporter: Do you have the Georgian language pack installed and selected? 





TO REPRODUCE:

In WinXP Pro, at least:
1)  Click Control Panel | Regional and Language Options | Regional Options |
Standards and formats (top section) | Georgian

2)  Close & reopen Moz

3)  Click MailNews | Edit | Mail & News Acct Settings | Add Acct


You'll see the problem other places, too:
 - buttons missing text (See image.)
 - no response when click buttons -- though I do get a response with one of the
buttons in Edit | Prefs.


Triage:
- confirming
- rewrite summary to clarify
- severity -> major
- Compnent -> Browser Internationalization (where do I put Internationalization
bugs affecting more than one component?)

Comment 3

[Jungshik Shin]

Does this still happen with the latest nightly? 

The lack of Georgian language pack shouldn't affect it. There's no 'legacy'
Windows code page for Georgian and that may have something to do with this
because Mozilla's transition to Unicode is not yet complete on Windows. However,
that's rather a remote possibility. 

I'll try it on Windows XP tomorrow.

Comment 4

[Jungshik Shin]

It seems to be easier to fix than I thought originally. At first I thought all
the locales that hadn't been supported by Win 9x/ME would have this problem
(e.g. Georgian, Armenian?, Hindi, Tamil, etc). That might still be the case, but
some languages/scripts supported on Win 9x/ME don't work either (e.g.
Azeri(Latin) or Azeri(Cyrillic)) because they're not listed in our language list. 

Comment 5

[Jungshik Shin]

Attachment: patch

I added most of the remaining languages in the Win32 primary and sublanguage
list. 

For Azeri and Serbian, two scripts(Cyrillic and Latin) are supported on Win32 (
Arabic is also used for Azeri, I guess) and I left them alone for the now. In
addition, I made Mozilla fallback to en-US if the mapping from Win32
locale/language to XPLocale fails. This is better than blank buttons. 
 
I also turned on a few languages in accept-language list. I considered turning
on more languages (e.g. Hindi, Tamil and other languages in India), but decided
to file a new bug for that issue because the support is platform dependent. On
Win2k/XP, Mozilla renders scripts used to writing them well, but on other
platforms we have issues. So, we need platform-dependent language.properties
files.

Comment 6

[Jungshik Shin]

Attachment: update (with fb/sb/tb patch)

basically the same. Two languages I turned on by mistake are turned off in
intl/locale/src/language.properties. toolkit/locale patch is included for
firebird/tb/sb. The indentation was fixed.

Comment 7

[Jungshik Shin]

Comment on attachment 135638 [details] [diff] [review]
update (with fb/sb/tb patch)

asking for r/sr

Comment 8

[Simon Montagu :smontagu]

I am OK with this in principle, but are all the added langcodes present in all
versions of MSVC that we aim to support? I have a relatively old MSVC at home,
so I can check that myself :)

Comment 9

[Jungshik Shin]

Yeah, I was worried about it, but just tried to get away thinking that everybody
has VC6 nowadays (which is apparently not true). 

It'd be nice if you could check new additions with your old VC. Mine is VC 6
with the latest (I believe) service pack installed. If you can identify those
not supported by old MS Win SDK, we can define them as 'MOZ_LANG/SUBLANG_BLAH'
(as was done before).  We have to support VC5, right? Even with VC6, not
everyone has the newest SDK installed, I guess. In that case, you may not have
to bother. 

To be on a safe side, I'll use MOZ_LANG/SUBLANG_BLAH macros for all the new
entries as well as four(?) old ones defined with MOZ_ macros in the current
tree. That's one more trip to MSDN :-)

Comment 10

[Simon Montagu :smontagu]

It turns out that my "old" version at home is also VC 6, so I rechecked
http://www.mozilla.org/build/win32.html and it says "Version 5.0 is not
supported and probably will not build Mozilla."

Comment 11

[Simon Montagu :smontagu]

Bah. No sooner had I written that than the build failed with the patch on VC 6! 

c:/mozwork/mytree/mozilla/intl/locale/src/windows/nsIWin32LocaleImpl.cpp(216):
error C2065: 'LANG_GALICIAN' : undeclared identifier
c:/mozwork/mytree/mozilla/intl/locale/src/windows/nsIWin32LocaleImpl.cpp(291):
error C2065: 'LANG_KYRGYZ' : undeclared identifier
c:/mozwork/mytree/mozilla/intl/locale/src/windows/nsIWin32LocaleImpl.cpp(307):
error C2065: 'LANG_MONGOLIAN' : undeclared identifier

It's a pain, but maybe we should thing about going the same route as e.g.
http://www.tug.org/ftp/tex/texinfo/intl/localename.c, with a whole pile of
 
# ifndef LANG_AFRIKAANS
# define LANG_AFRIKAANS 0x36
# endif

etc., etc., etc., especially in view of the comment in that file:

/* Mingw headers don't have latest language and sublanguage codes.  */

Comment 12

[Jungshik Shin]

Attachment: update

Thanks. The texinfo way seems cleaner (although longer) than the way it's done
in the current tree. Because mingw porters haven't complained about the current
list (as far as I know), I guess we just have to define macros for newly added
lang/sublangs and those with MOZ_ macros. (texinfo list seems to be more
conservative than our current tree). 

BTW, I took this opportunity to make the array (iso_map) const static.

Comment 13

[Jungshik Shin]

Comment on attachment 135813 [details] [diff] [review]
update 

asking for r/sr with the update

Comment 14

[Simon Montagu :smontagu]

Comment on attachment 135813 [details] [diff] [review]
update 

>+#if 0  // turn it on when three letter lang code is supported.
>+        { "div", LANG_DIVEHI, {
>+                { "MV", SUBLANG_DEFAULT }, 
>+                { "", 0}}
>+        },
>+#endif

Why not use "dv"? It's a valid ISO-639-1 Alpha-2 code. Is there a bugzilla bug
for adding support for three letter lang codes? (I had thought we supported
them already, at least in some contexts).

> 	{"id",	LANG_INDONESIAN, {
> 		{ "ID", SUBLANG_DEFAULT },
> 		{"", 0}}
> 	},
> 	{"in",	LANG_INDONESIAN, {
> 		{ "ID", SUBLANG_DEFAULT },
> 		{ "", 0}}

Do we need to keep both of these for backwards compatibility? See below for
more comments on Indonesian in languageName.properties.

-	{"no",	LANG_NORWEGIAN, {
+	{"no",	LANG_NORWEGIAN, {     // XXX : Nynorsk vs Bokmal 
		{ "NO",  SUBLANG_DEFAULT },
		{ "", 0}}
	},

Do we want to add "nn" and "nb" for SUBLANG_NORWEGIAN_BOKMAL and
SUBLANG_NORWEGIAN_NYNORSK?

> 	{"sr",	LANG_SERBIAN, {
>-		{ "YU", SUBLANG_DEFAULT },
>+		{ "YU", SUBLANG_DEFAULT }, // XXX Latin vs Cyrillic
> 		{ "", 0}}
> 	},

Hasn't "yu" been deprecated? I thought it should now be "cs". sr_Latn and
sr-Cyrl were recently registered in
http://www.iana.org/assignments/language-tags, but I don't know if we can parse
them.

>+	{"uz",	LANG_UZBEK, {
>+		{ "LATIN", SUBLANG_UZBEK_LATIN },
>+		{ "CYRILLIC", SUBLANG_UZBEK_CYRILLIC },
> 		{ "", 0}}
> 	},


Surely these should be "Latn" and "Cyrl"? I am beginning to realize that I
don't understand where these values are expected to be coming from and who is
parsing them. Please don't say nsIWin32LocaleImpl::ParseLocaleString ;-)

>+++ xpfe/global/resources/locale/en-US/languageNames.properties	18 Nov 2003 16:52:47 -0000
>@@ -183,7 +183,8 @@ wen = Sorbian
> wo = Wolof
> xh = Xhosa
> yi = Yiddish
> yo = Yoruba
> za = Zhuang
> zh = Chinese
> zu = Zulu
>+in = Indonesian

Are you restoring "in" for backwards compatibility reasons? It was removed in
the last check-in to this file (bug 178491), having been removed from ISO-639
in 1989. In any case, please preserve the alphabetical ordering.

>+++ xpfe/global/resources/locale/en-US/regionNames.properties	18 Nov >+mo	=	Makao 

Macao is the spelling in ISO 3166

Comment 15

[Jungshik Shin]

Thanks for review.

> Please don't say nsIWin32LocaleImpl::ParseLocaleString ;-)

  Sorry it's :-). And, that's why I didn't add three letter codes.
'ParseLocaleString' was written under the assumption that all country/language
codes are two letters only. (in other places of Mozilla, three letter codes and
even 'i-*' and 'x-*' are supported) Some of your reservations about the patch
can be resolved by fixing that. However, others are not. I'm aware of 'nb' and
'nn', but mapping to and from Windows 'language' is tricky because Windows
treats 'nb' and 'nn' as 'sub languages' of Norwegian. The problem here is that
mapping 'language-country' to/from 'lang-sublang' doesn't always work for the
obvious reason. 

 In case of Uzbek, I didn't mean to add both Latin and Cyrillic, which is not on
par with the way we deal, at the moment, with other languages for which multiple
scripts are used. So, I'll use the default sublang there, too.
This is another area (multiple scripts per language) in which our
locale/language model and Windows model don't work with each other well (as
mentioned in nsIWin32....cpp). 

So, in this bug, I decided to deal with easy ones (adding some missing
lang/sublangs and mapping the 'unlisted/unmatched' to the default so that we at
least don't lose 'button' labels when they're selected in Windows regional
setting). 

I'll replace 'yu' (the one for the country that doesn't exist any more) with 'cs'. 
  

As for 'in', I added it (back) to languageName.properties because I found that
it's listed without 'Country' name at the top of  Edit | Pref | Navigator |
Language | Add. Both 'in' and 'id' are listed with accept set to true in
language.properties. If we don't want to support 'in' any more, we have to
remove it from both files. 'sb' was also not listed in languageName.properties
but is set to 'true' in language.properties. Because I couldn't find 'sb'  in
ISO 639, I removed it from language.properties file. BTW, keeping 'in' in
nsIWin32 may not be a bad idea even though we decide to remove it from
languageName.properties and language.properties files.

As for 'div vs dv', I couldn't find 'dv' in Everson's list at w3.org (I don't
have ISO 639 priced at ~50 CHF?) so that I didn't use it. (it's not listed in
the IANA lang list, either) Do you know where I can see a more updated list for
free?  

Anyway, let's fix some easier problems here and think more about tricky cases
for a separate bug.

Comment 16

[Simon Montagu :smontagu]

The reference I use for ISO-639 is http://www.loc.gov/standards/iso639-2/

Comment 17

[Jungshik Shin]

Attachment: update with smontagu's concerns addressed

Thanks for the reference. I used to know that and then forgot (happens so often
these days :-)). I updated the reference in the patch, too.

I thought I had uploaded this, but I haven't. I could have added two Norwegian
'languages' by special-casing them, but left them out for the now. I want to
deal with simple  entries first.

Comment 18

[Jungshik Shin]

Comment on attachment 136434 [details] [diff] [review]
update with smontagu's concerns addressed

asking for r/sr. perhaps not this weekend although i'd not complain if you
could review this weekend :-)

Comment 19

[Simon Montagu :smontagu]

Comment on attachment 136434 [details] [diff] [review]
update with smontagu's concerns addressed

>+        { "dv", LANG_DIVEHI, {
>+                { "MV", SUBLANG_DEFAULT }, 
>+                { "", 0}}
>+        },
> 	{"el",	LANG_GREEK, {
> 		{ "GR", SUBLANG_DEFAULT},
> 		{ "", 0}}
> 	},

Nit: regularize the indentation here.

>+#if 0
>+	{ "kok", LANG_KONKANI, {
>+		{ "IN", SUBLANG_DEFAULT }, 
>+		{ "", 0}}
>+	},
>+#endif

Nit: Please restore the comment 
  // turn it on when three letter lang code is supported."
which got lost changing "div" to "dv", or change to #ifdef SOMETHING in all
cases. This will make life easier for some lucky person in the future  who gets
to rewrite ParseLocaleString()

With these, r=smontagu

Comment 20

[Jungshik Shin]

Thanks for r. I'll replace '#if 0' with '#ifdef
THREE_LETTER_LANG_CODE_SUPPORTED' and add the following lines before iso_map
iso_list[].  

// Turn this on when ParseLocaleString(), GetXPLocale(), and 
// GetPlatformLocale() are fixed to support three letter lang codes.
#ifdef THREE_LETTER_LANG_CODE_SUPPORTED
#undef THREE_LETTER_LANG_CODE_SUPPORTED
#endif

Comment 21

[Jungshik Shin]

Asking for blocking 1.6b. Losing 'OK', 'Cancel' and so forth is serious and it's
easy (and safe)  to fix. 

Comment 22

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 136434 [details] [diff] [review]
update with smontagu's concerns addressed

>Index: intl/locale/src/windows/nsIWin32LocaleImpl.cpp
> struct iso_pair 
> {
> 	char*	iso_code;

How about |const char*|, while you're here?

> 	DWORD	win_code;
> };
> typedef struct iso_pair iso_pair;

typedef unneeded in C++

>@@ -64,27 +63,118 @@ struct iso_map
> {
> 	char*				iso_code;

Likewise.

> 	DWORD				win_code;
> 	iso_pair			sublang_list[20];
> };
> typedef struct iso_map iso_map;

Likewise.


>+#define LENGTH_MAPPING_LIST		(sizeof(iso_list) / sizeof(iso_map))

How about NS_ARRAY_LENGTH(iso_list)?

With that, sr=dbaron.

Comment 23

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 136434 [details] [diff] [review]
update with smontagu's concerns addressed

>@@ -64,27 +63,118 @@ struct iso_map
> {
> 	char*				iso_code;
> 	DWORD				win_code;
> 	iso_pair			sublang_list[20];

and changing this to
+	const iso_pair* 		sublang_list;

should greatly reduce codesize.

Comment 24

[Asa Dotzler [:asa]]

Comment on attachment 136434 [details] [diff] [review]
update with smontagu's concerns addressed

a=asa (on behalf of drivers) for checkin to 1.6beta

Comment 25

[Jungshik Shin]

fix checked in with dbaron's comment addressed (except for the last one which 
somehow VC++6 doesn't like to my and his surprise that we had to back out).
thank you all

Comment 26

[Jungshik Shin]

forgot to mark as fixed