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

VERIFIED FIXED

Status

()

Core
Internationalization
--
trivial
VERIFIED FIXED
14 years ago
13 years ago

People

(Reporter: Aleksandar Radulovic, Assigned: smontagu)

Tracking

({fixed-aviary1.0, fixed1.7})

Trunk
x86
Windows 2000
fixed-aviary1.0, fixed1.7
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

4.24 KB, patch
Jungshik Shin
: review+
neil@parkwaycc.co.uk
: superreview+
chris hofmann
: approval-aviary+
chris hofmann
: approval1.7.5+
Details | Diff | Splinter Review
(Reporter)

Description

14 years ago
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

13 years ago
->page info.
Assignee: firefox → db48x
Component: General → Page Info
Product: Firefox → Browser
Summary: when "Regional Options / General" locale is set to "Serbian (Latin)" or "Serbian (Cyrillic)" wrong display of date in Page Info (Ctrl + J) → when "Regional Options / General" locale is set to "Serbian (Latin)" or "Serbian (Cyrillic)" wrong display of date in Page Info (Ctrl + J)
Version: unspecified → Trunk
Page Info doesn't format the date, it gets it from nsIScriptableDateFormat
Assignee: db48x → smontagu
Component: Page Info → Internationalization
QA Contact: amyy
(Assignee)

Comment 3

13 years ago
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)
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
(Assignee)

Comment 4

13 years ago
*** Bug 203737 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 5

13 years ago
Created attachment 162030 [details] [diff] [review]
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.
(Assignee)

Comment 6

13 years ago
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

13 years ago
Serbian is my native language, I can confirm that this is correct.
(Assignee)

Comment 8

13 years ago
Comment on attachment 162030 [details] [diff] [review]
Patch

Asking for review. I've tested the patch on XP and 98.
Attachment #162030 - Flags: review?(jshin)

Comment 9

13 years ago
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?
Attachment #162030 - Flags: review?(jshin) → review+
(Assignee)

Comment 10

13 years ago
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.
Attachment #162030 - Flags: superreview?(neil.parkwaycc.co.uk)

Comment 11

13 years ago
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" }, ... } };
Attachment #162030 - Flags: superreview?(neil.parkwaycc.co.uk) → superreview+
(Assignee)

Comment 12

13 years ago
Fix checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
(Assignee)

Comment 13

13 years ago
Comment on attachment 162030 [details] [diff] [review]
Patch

Asking branch approvals. Getting people's languages wrong is not something we
should be doing.
Attachment #162030 - Flags: approval1.7.x?
Attachment #162030 - Flags: approval-aviary?

Comment 14

13 years ago
Comment on attachment 162030 [details] [diff] [review]
Patch

a=chofmann
Attachment #162030 - Flags: approval1.7.x?
Attachment #162030 - Flags: approval1.7.x+
Attachment #162030 - Flags: approval-aviary?
Attachment #162030 - Flags: approval-aviary+
(Assignee)

Updated

13 years ago
Keywords: fixed-aviary1.0, fixed1.7

Comment 15

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

Comment 16

13 years ago
I've checked in a fix for the VC.net bustage
(Reporter)

Comment 17

13 years ago
(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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 18

13 years ago
(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
(Reporter)

Comment 19

13 years ago
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)
Status: REOPENED → RESOLVED
Last Resolved: 13 years ago13 years ago
Resolution: --- → FIXED
(Reporter)

Updated

13 years ago
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.