Description of bug ------------------ The formatting of the language names in 'Preferences | Browser | Languages | Add' is not localisable, which is a problem for some languages (e.g. Norwegian Nynorsk). The list is formatted this way: LanguageName/CountryName [LanguageCode] In Norwegian Nynorsk, the '/' character should never be used this way. Instead the list should look like this: LanguageName (Countryname) [LanguageCode] or LanguageName – Countryname (LanguageCode) I'm sure this is a problem for other languages too. How to fix ---------- Make the formatting of the language entries localisable, using variables. E.g.: %lang%/%country% [%code%] For Norwegian Nynorsk, we can then localise this to: %lang% (%country%) [%code%]

-> naoki.

Note that language strings are also used other places in the UI, e.g. for displaying the value of 'hreflang' attribute ('target language') when right-clicking and choosing 'Properties' on links. The language representation should be the same here. (Oddly enough, it seems to default to 'LanguageName (CountryName)'.)

I can't reproduce. Please take a look attachment.

I tested on 2002-10-18-08-1.0 on Win XP Pro. JA

Reopening. I have no idea what your attachment are meant to show.

It means no '/' is in Preferences | Browser | Languages | Add.

> It means no '/' is in Preferences | Browser | Languages | Add. Yes, there is. It's clearly visible in the screenshot for 'Portugese/Brazil' and 'Macedonian/Macedonia, F.Y.R. of'. It's currently not possible to localise how this is displayed.

This patch enables the localization of language name representations. You can change the symbol '/' between language and region names. You can also switch their order, it might be better in some languages to write e.g. "Brazil Portuguese" instead of "Portuguese/Brasil".

I modified the patch to include the language-region code part of the string in the localization. It makes it possible to set to "LanguageName – Countryname (LanguageCode)" as specified in the original bug report.

This patch includes localisation of: 1- The languages preferences language representation 2- The link properties href attribute (Target language) Karl (or anybody), can you review this patch?

Comment on attachment 127933 [details] [diff] [review] Proposed patch - Adds hreflang localisation requested in comment#2 Requesting review on attachment #127933 [details] [diff] [review]

Comment on attachment 127933 [details] [diff] [review] Proposed patch - Adds hreflang localisation requested in comment#2 >Index: xpfe/components/prefwindow/resources/content/pref-languages.js >=================================================================== >RCS file: /cvsroot/mozilla/xpfe/components/prefwindow/resources/content/pref-languages.js,v >retrieving revision 1.31 >diff -u -r1.31 pref-languages.js >--- xpfe/components/prefwindow/resources/content/pref-languages.js 2 Jul 2003 21:59:22 -0000 1.31 >+++ xpfe/components/prefwindow/resources/content/pref-languages.js 15 Jul 2003 03:28:40 -0000 >@@ -154,28 +154,39 @@ > > if (stringLangRegion[0]) { > var tit; >+ var language; >+ var region; >+ var use_region_format = false; >+ > try { >- tit = languagesBundle.GetStringFromName(stringLangRegion[0]); >+ language = >+ languagesBundle.GetStringFromName(stringLangRegion[0]); > } > > catch (ex) { >- tit = ""; >+ language = ""; > } > > > if (stringLangRegion.length > 1) { > > try { >- tit += "/" + regionsBundle.GetStringFromName(stringLangRegion[1]); >+ region = >+ regionsBundle.GetStringFromName(stringLangRegion[1]); >+ use_region_format = true; > } > > catch (ex) { >- tit += "["+str+"]"; > } >- >- } //if region >- >- tit = tit + " [" + str + "]"; >+ } >+ Nit: It looks like you need to indent the code below by 4 more spaces. Indentation in all this (old) code is rather yucky. >+ if (use_region_format) { >+ tit = prefLangBundle.formatStringFromName( >+ "languageRegionCodeFormat", [language, region, str], 3); >+ } else { >+ tit = prefLangBundle.formatStringFromName( >+ "languageCodeFormat", [language, str], 2); >+ } > > } //if language > >Index: xpfe/browser/resources/content/metadata.js >=================================================================== >@@ -557,17 +561,24 @@ > { > // We don't add it on to the result immediately > // because we want to get the spacing right. >- tokens[0] = gRegionBundle.getString(tokens[0].toLowerCase()); >+ region = gRegionBundle.getString(tokens[0].toLowerCase()); At this point do we not care about tokens[1..n-1]? The spec seems to suggest otherwise. The old code joined all of them back together again with spaces, the new code only does it in the catch case. > } > catch (e) > { > // Region not present in region bundle >+ region = tokens.join(" "); > } > >- result += " (" + tokens.join(" ") + ")"; >+ is_region_set = true; > } > } >Index: xpfe/browser/resources/locale/en-US/metadata.properties >=================================================================== >@@ -10,3 +10,5 @@ > imageSizeUnknown=Unknown (not cached) > imageWidth=%Spx > imageHeight=%Spx >+# LOCALIZATION NOTE: first %S is language, second is region >+languageRegionFormat=%S (%S) You could use %1$S and %2$S here like you did in the other .properties file to make your comment slightly better.

Includes modifications asked for in comment#13. In pref-languages.js, the only modifications added from previous attachment is the indentation which has been completeley remade in the ReadAvailableLanguages() function.

Comment on attachment 128210 [details] [diff] [review] Updated patch sr=jag

Comment on attachment 128210 [details] [diff] [review] Updated patch Requesting approval for 1.5b. I think it is important to have this bug fixed (as any l12y bug). I don't think this could be breaking anything since it is almost only GUI.

Comment on attachment 128210 [details] [diff] [review] Updated patch fixing flag to make it a request rather than an approval

Sorry for the mistake about the 1.5b+ instead of 1.5b?, I should learn to verify before clicking on submit...

Comment on attachment 128210 [details] [diff] [review] Updated patch a=asa (on behalf of drivers) for checkin to 1.5beta. Time is short, please land ASAP. Thanks.

.

checked-in on behalf of Marcel

Nit: why the extra whitespace in this declaration in pref-languages.js? + availLanguageDict = new Array();

I simply did not see it when doing the patch. This line was already like this, I just corrected the indentation. There is also a missing space before the 0 in + var i =0;