Language name representation not localisable

RESOLVED FIXED

Status

()

RESOLVED FIXED
16 years ago
14 years ago

People

(Reporter: karl, Assigned: marcel.gosselin)

Tracking

({l12y})

Trunk
x86
All
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 3 obsolete attachments)

(Reporter)

Description

16 years ago
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%]

Updated

16 years ago
Keywords: l12y
QA Contact: ruixu → kasumi

Comment 1

16 years ago
-> naoki.
Assignee: rchen → nhotta
(Reporter)

Comment 2

16 years ago
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)'.)

Comment 3

16 years ago
I can't reproduce.
Please take a look attachment.
Status: NEW → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → WORKSFORME

Comment 4

16 years ago
Created attachment 103607 [details]
2002-10-18-08-1.0

Comment 5

16 years ago
I tested on 2002-10-18-08-1.0 on Win XP Pro. JA
(Reporter)

Comment 6

16 years ago
Reopening.

I have no idea what your attachment are meant to show.
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---

Comment 7

16 years ago
It means no '/' is in Preferences | Browser | Languages | Add.
(Reporter)

Comment 8

16 years ago
> 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.
(Assignee)

Comment 9

16 years ago
Created attachment 127723 [details] [diff] [review]
Patch - enables localization of language name representations

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".

Updated

16 years ago
QA Contact: kasumi → marina
(Assignee)

Comment 10

16 years ago
Created attachment 127777 [details] [diff] [review]
Adds the language-region code to the localisation

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.
Attachment #127723 - Attachment is obsolete: true
(Assignee)

Comment 11

16 years ago
Created attachment 127933 [details] [diff] [review]
Proposed patch - Adds hreflang localisation requested in comment#2

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?
Attachment #127777 - Attachment is obsolete: true
(Assignee)

Comment 12

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

Requesting review on attachment #127933 [details] [diff] [review]
Attachment #127933 - Flags: review?(nhotta)

Updated

16 years ago
Attachment #127933 - Flags: review?(nhotta) → review+

Comment 13

16 years ago
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.
Attachment #127933 - Flags: superreview-
(Assignee)

Comment 14

16 years ago
Created attachment 128210 [details] [diff] [review]
Updated patch

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.
Attachment #127933 - Attachment is obsolete: true
(Assignee)

Updated

16 years ago
Attachment #128210 - Flags: review?(nhotta)

Updated

16 years ago
Attachment #128210 - Flags: review?(nhotta) → review+
(Assignee)

Updated

16 years ago
Attachment #128210 - Flags: superreview?(jaggernaut)

Comment 15

16 years ago
Comment on attachment 128210 [details] [diff] [review]
Updated patch

sr=jag
Attachment #128210 - Flags: superreview?(jaggernaut) → superreview+
(Assignee)

Comment 16

16 years ago
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.
Attachment #128210 - Flags: approval1.5b+

Comment 17

16 years ago
Comment on attachment 128210 [details] [diff] [review]
Updated patch

fixing flag to make it a request rather than an approval
Attachment #128210 - Flags: approval1.5b+ → approval1.5b?
(Assignee)

Comment 18

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

Comment 19

15 years ago
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.
Attachment #128210 - Flags: approval1.5b? → approval1.5b+

Comment 20

15 years ago
.
Assignee: nhotta → marcel.gosselin
Status: REOPENED → NEW

Comment 21

15 years ago
checked-in on behalf of Marcel
Status: NEW → RESOLVED
Last Resolved: 16 years ago15 years ago
Resolution: --- → FIXED
Nit: why the extra whitespace in this declaration in pref-languages.js?

+  availLanguageDict   = new Array();
(Assignee)

Comment 23

14 years ago
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;
You need to log in before you can comment on or make changes to this bug.