234865 - when "Regional Options / General" locale is set to "Serbian (Latin)" or "Serbian (Cyrillic)" wrong display of date in Page Info (Ctrl + J)

Status: VERIFIED FIXED

(Core :: Internationalization, defect)

Description

[Aleksandar Radulovic]

User-Agent:       
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.6) Gecko/20040206 Firefox/0.8

When "Regional Options / General" locale in Windows 2000 is set to "Serbian
(Latin)" or "Serbian (Cyrillic)", wrong display of date in Page Info (Ctrl + J)
. Actually, "Croatian" date is displayed instead of "Serbian".

My W2K is SP4, v 5.00.2195  but I suppose that this is Windows independant.


Reproducible: Always
Steps to Reproduce:
1. go to "Control Panel / Regional Options"
2. in tab "General", set "Your locale (location)" to "Serbian (Latin)" or
"Serbian (Cyrillic)"
3. click "Apply"
4. restart Firefox and press Ctrl + J or menu "Tools / Page Info"

Actual Results:  
In field "Modified:" or "Expire:" stands (e.g.):
19. veljača 2004 9:12:33

Expected Results:  
19. februar 2004 9:12:33   (for "Serbian (Latin)")
          or
19. фебруар 2004 9:12:33   (for "Serbian (Cyrillic)")

Programmer's note:
It is a very strange behavior because Windows API 
        GetDateFormat(LOCALE_USER_DEFAULT, DATE_LONGDATE, ...... )
correctly displays locale's date.
I suppose that you use some other platform than C++ ?

Don't hesitate to e-mail me for additional information or help.

Comment 1

[Asa Dotzler [:asa]]

->page info.

Comment 2

[Daniel Brooks [:db48x]]

Page Info doesn't format the date, it gets it from nsIScriptableDateFormat

Comment 3

[Simon Montagu :smontagu]

We need to add the proper SUBLANG codes for Serbian at
http://lxr.mozilla.org/seamonkey/source/intl/locale/src/windows/nsIWin32LocaleImpl.cpp#452

LANG_SERBIAN with SUBLANG_DEFAULT creates an LCID of 0x041A, which is Croatian
(see
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/script56/html/vsmsclcid.asp)

Comment 4

[Simon Montagu :smontagu]

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

Comment 5

[Simon Montagu :smontagu]

Attachment: Patch

This solves the problem -- rather inelegantly, but the situation is inelegant:
the ISO-639 code for Serbo-Croatian was deprecated some time ago and replaced
by separate codes "hr" for Croation and "sr" for Serbian, but the Windows
LANG_ID remains the same for both.

Comments are welcome, but I'd like to test this on older version(s) of Windows
before asking for review.

While I was there, I've refactored GetXPLocale() a bit, including removing the
assumption that all language and country codes are 2 characters long.

Comment 6

[Simon Montagu :smontagu]

With the patch on WinXP I get the following results on
http://www.mozilla.org/start/ (in UTF-8)

Croatian:           10. listopad 2004 19:47:47
Serbian (Latin):    10. oktobar 2004 19:47:47
Serbian (Cyrillic): 10. октобар 2004 19:47:47

Aleksandar, can you confirm that these are correct?

Comment 7

[Ivan Ičin]

Serbian is my native language, I can confirm that this is correct.

Comment 8

[Simon Montagu :smontagu]

Comment on attachment 162030 [details] [diff] [review]
Patch

Asking for review. I've tested the patch on XP and 98.

Comment 9

[Jungshik Shin]

Comment on attachment 162030 [details] [diff] [review]
Patch

r=jshin

>       for(j=0;iso_list[i].sublang_list[j].win_code;j++) {
>         if (sublang_id == iso_list[i].sublang_list[j].win_code) {
>-          PR_snprintf(rfc_locale_string,9,"%s-%s%c",iso_list[i].iso_code,
>-            iso_list[i].sublang_list[j].iso_code,0);
>-          CopyASCIItoUTF16(nsDependentCString(rfc_locale_string), locale);
>-          return NS_OK;
>+          len = strlen(iso_list[i].sublang_list[j].iso_code);
>+          PR_snprintf(locale_string_ptr, len+2,"-%s",
>+                      iso_list[i].sublang_list[j].iso_code);
>         }
>       }

Some sublang lists (e.g. English, Arabic, Spanish,French)are rather long so
that it's better to break out of the inner loop ('j' loop) when a match is
found. 

BTW,  you didn't turn on 'THREE_LETTER_LANG_CODE_SUPPORTED' because
'ParseLocaleString' hasn't been fixed, right?

Comment 10

[Simon Montagu :smontagu]

Comment on attachment 162030 [details] [diff] [review]
Patch

Thanks for r, requesting sr
(In reply to comment #9)
> Some sublang lists (e.g. English, Arabic, Spanish,French)are rather long so
> that it's better to break out of the inner loop ('j' loop) when a match is
> found. 

OK, I'll put a |break| in the |if| -- I won't attach a new patch just for that.

> BTW,  you didn't turn on 'THREE_LETTER_LANG_CODE_SUPPORTED' because
> 'ParseLocaleString' hasn't been fixed, right?

Right :) I'll do that in a separate bug.

Comment 11

[[e-mail]]

Comment on attachment 162030 [details] [diff] [review]
Patch

I don't see the point of duplicating the entries in the list as you will never
reach the Serbian entry. Now if you had made it so that unless you had
LANG_CROATIAN, SUBLANG_DEFAULT you skipped to Serbian then I could understand
that. You also need to update the debug list before you check in.

>+          PR_snprintf(locale_string_ptr, len+1, "%s", CROATIAN_ISO_CODE);
Isn't this the same as strcpy(locale_string_ptr, CROATIAN_ISO_CODE); - if it is
then please fix the three cases before you check in.

As it happens, we now have some useful functions which can append ascii strings
directly to nsAStrings, so in this case you would use
locale.AssignLiteral(CROATIAN_ISO_CODE); (or SERBIAN_ISO_CODE) or
locale.AssignASCII(iso_list[i].iso_code); then later you would use
locale.Append(PRUnichar('-'));
locale.AppendASCII(iso_list[i].sublang_list[j].iso_code);

As a passing node I must say I find this data structure inefficient. By the
time you add up all the space wasted by 21 pointers for each country code you
would probably be better off listing all the iso codes with their windows
language pairs, i.e. struct iso_map { WORD lang; BYTE sublang; char
iso_code[9]; } iso_list[] = { { LANG_AFRIKAANS, SUBLANG_DEFAULT, "af-ZA" } , {
LANG_AFRIKAANS, SUBLANG_NEUTRAL, "af" } ... }; Or perhaps an array of arrays,
i.e. struct iso_map { WORD wincode; char iso_code[6]; } iso_list[][] = { { {
LANG_AFRIKAANS, "af" }, { SUBLANG_DEFAULT, "ZA" }, { SUBLANG_NEUTRAL , "" } } ,
{ { LANG_ARABIC, "ar" }, ... } };

Comment 12

[Simon Montagu :smontagu]

Fix checked in.

Comment 13

[Simon Montagu :smontagu]

Comment on attachment 162030 [details] [diff] [review]
Patch

Asking branch approvals. Getting people's languages wrong is not something we
should be doing.

Comment 14

[chris hofmann]

Comment on attachment 162030 [details] [diff] [review]
Patch

a=chofmann

Comment 15

[Andreas Kunz]

Could this have caused VC.NET 2003 to not compile anymore?
I get the error:

nsIWin32LocaleImpl.cpp
m:/o1\mozilla\intl\locale\src\windows\nsIWin32LocaleImpl.cpp(672) : error C2784:
 'void nsAString::AssignLiteral(char (&)[N])' : could not deduce template argume
nt for 'char (&)[N]' from 'const char *'

originating from the line(s):

        locale.AssignLiteral((sublang_id == SUBLANG_DEFAULT) ?
                             CROATIAN_ISO_CODE : SERBIAN_ISO_CODE);

Bug 226439 comment #123 seems to be about the same issue, however, I cannot find
the "other bug" mentioned there yet.

Comment 16

[Simon Montagu :smontagu]

I've checked in a fix for the VC.net bustage

Comment 17

[Aleksandar Radulovic]

(I am sorry I couldn't respond earlier)

I installed the newest version of FireFox (Mozilla/5.0 (Windows; U; Windows NT 
5.0; rv:1.7.3) Gecko/20041001 Firefox/0.10.1) today on Windows 2000. However, 
there is still Croatian date instead of Serbian, neither latin or cyrillic.
I can't involve deep into the problem now, but it seems that the problem is in 
SUBLANG_DEFAULT which generates Croatian. There is no default language for 
Serbian, only SUBLANG_SERBIAN_LATIN or SUBLANG_SERBIAN_CYRILLIC.

Comment 18

[Simon Montagu :smontagu]

(In reply to comment #17)

> I installed the newest version of FireFox (Mozilla/5.0 (Windows; U; Windows NT 
> 5.0; rv:1.7.3) Gecko/20041001 Firefox/0.10.1) today on Windows 2000.
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

This is not the latest version. Serbian works for me on 1.0RC1

Comment 19

[Aleksandar Radulovic]

Now it works EXACTLY as it should. Thanks!

(There was an old version on the server when I was checking the fix (1.0 PR). 
Now I installed version 1.0)