35480 - Handling of setlocale() is wrong on certain platforms

Status: VERIFIED FIXED

(Core :: Internationalization, defect, P3)

Description

[Jim Dunn]

the setlocale(LC_ALL, "") calls in intl/locale/src nsLocaleFactory.cpp
and nsLocaleService.cpp expect a single locale to be returned.  However
on AIX & HPUX this is not the case.  Nor is it the case if any of
the localetypes (LC_TIME...) have been individually modified.

AIX
Locale(LC_ALL, NULL) => C C C C C C
Locale(LC_ALL, "") => C C C C C C
Locale(LC_COLLATE, "en_US") => en_US
Locale(LC_CTYPE, "C") => C
Locale(LC_MONETARY, "POSIX") => POSIX
Locale(LC_NUMERIC, "en_JP") => 
Locale(LC_TIME, "en_us") => 
Locale(LC_ALL, NULL) => en_US C POSIX C C C

Linux
Locale(LC_ALL, NULL) => C
Locale(LC_ALL, "") => en_US
Locale(LC_COLLATE, "en_US") => en_US
Locale(LC_CTYPE, "C") => C
Locale(LC_MONETARY, "POSIX") => C
Locale(LC_NUMERIC, "en_JP") => 
Locale(LC_TIME, "en_us") => 
Locale(LC_ALL, NULL) => 
LC_CTYPE=C;LC_NUMERIC=en_US;LC_TIME=en_US;LC_COLLATE=en_US;LC_MONETARY=C;LC_MESS
AGES=en_US

SunOS
Locale(LC_ALL, NULL) => C
Locale(LC_ALL, "") => C
Locale(LC_COLLATE, "en_US") => en_US
Locale(LC_CTYPE, "C") => C
Locale(LC_MONETARY, "POSIX") => POSIX
Locale(LC_NUMERIC, "en_JP") => 
Locale(LC_TIME, "en_us") => 
Locale(LC_ALL, NULL) => /C/C/C/en_US/POSIX/C

HPUX
Locale(LC_ALL, NULL) => C C C C C C
Locale(LC_ALL, "") => C C C C C C
Locale(LC_COLLATE, "en_US") => 
Locale(LC_CTYPE, "C") => C
Locale(LC_MONETARY, "POSIX") => POSIX
Locale(LC_NUMERIC, "en_JP") => 
Locale(LC_TIME, "en_us") => 
Locale(LC_ALL, NULL) => C C POSIX C C C

Irix
Locale(LC_ALL, NULL) => C
Locale(LC_ALL, "") => C
Locale(LC_COLLATE, "en_US") => en_US
Locale(LC_CTYPE, "C") => C
Locale(LC_MONETARY, "POSIX") => POSIX
Locale(LC_NUMERIC, "en_JP") => 
Locale(LC_TIME, "en_us") => 
Locale(LC_ALL, NULL) => /C/C/C/en_US/POSIX/C

Comment 1

[Jim Dunn]

Attachment: Testcase to produce setlocale() results

Comment 2

[Jim Dunn]

here is my suggested diff.  basically just remove the block that
assumes that setlocale(LC_ALL) will return a single value.
I tested this on Linux, AIX & HPUX.  (Solaris acts like Linux)
Since the default path is through the ELSE clause of "if (lang==nsnull) {"

Index: nsLocaleService.cpp
===================================================================
RCS file: /cvsroot/mozilla/intl/locale/src/nsLocaleService.cpp,v
retrieving revision 1.20
diff -c -b -r1.20 nsLocaleService.cpp
*** nsLocaleService.cpp 2000/04/03 06:11:52     1.20
--- nsLocaleService.cpp 2000/04/19 03:49:22
***************
*** 235,254 ****
      nsresult result =
nsComponentManager::CreateInstance(kPosixLocaleFactoryCID,
                             NULL,kIPosixLocaleIID,(void**)&posixConverter);
      if (NS_SUCCEEDED(result) && posixConverter!=nsnull) {
-         char* lc_all = setlocale(LC_ALL,"");
          char* lang = getenv("LANG");
  
-         if (lc_all!=nsnull) {
-             result = posixConverter->GetXPLocale(lc_all,&xpLocale);
-             if (NS_FAILED(result)) { posixConverter->Release(); return; }
-             PRUnichar* loc = xpLocale.ToNewUnicode();
-             result = NewLocale(loc, &mSystemLocale);
-             nsCRT::free(loc);
-             if (NS_FAILED(result)) { posixConverter->Release(); return; }
-             mApplicationLocale=mSystemLocale;
-             mApplicationLocale->AddRef();
-             posixConverter->Release();
-         } else {
              if (lang==nsnull) {
                  xpLocale = "en-US";
                  PRUnichar* loc = xpLocale.ToNewUnicode();
--- 235,242 ----
***************
*** 277,283 ****
                 
(void)resultLocale->QueryInterface(kILocaleIID,(void**)&mSystemLocale);
                 
(void)resultLocale->QueryInterface(kILocaleIID,(void**)&mApplicationLocale);
                  posixConverter->Release();
-             }
          }
      }
  #endif // XP_UNIX || XP_BEOS
--- 265,270 ----

Comment 3

[Jim Dunn]

doh... I just found a case where this fails... still loking

Comment 4

[John Gaunt (redfive)]

after more poking around in the locale stuff than I wanted and trying some fixes 
it looks like the LC variables are getting set to c c c c c c ( which is what HP 
returns with a call to setlocale(LC_ALL, "") in a default setting ) before we 
even enter our code. However it still looks like changes will need to be made to 
the way we are storing the locale information, because if a system has different 
values for LC variables it is possible for them to be ignored in the current 
code. 

Two theories on where the problem is: configure scripts, gtk libraries

Comment 5

[nhottanscp]

>However it still looks like changes will need to be made to 
>the way we are storing the locale information, because if a system has 
>different values for LC variables it is possible for them to be ignored in 
>the current code. 

I agree, please make the change.

Comment 6

[Jim Dunn]

changing qacontact and target

Comment 7

[Rob Jaworski]

changing QA contact to barrettl.

Comment 8

[kzhou]

Jim, do you this type of fix will work? please let me know.

#include <stdio.h>
#include <locale.h>
#include <nl_types.h>
#include <string.h>

main()
{
  char *locale;

  locale = setlocale(LC_ALL, NULL);
  printf("Locale(LC_ALL, NULL) => %s\n",(locale)?locale:"");

  locale = setlocale(LC_ALL, "");
  printf("Locale(LC_ALL, \"\") => %s\n",(locale)?locale:"");

  printf("The lengnth of string locale: %d\n", strlen(locale));
  if ((locale[0] == 'C') && (strlen(locale)>1)) {
  locale[1]='\0';
  printf("The lengnth of string locale: %d\n", strlen(locale));
  printf("Locale(LC_ALL, \"\") => %s\n",(locale)?locale:"");
  }

output on HP-UXpalermo>locale_hp
Locale(LC_ALL, NULL) => C C C C C C
Locale(LC_ALL, "") => C C C C C C
The lengnth of string locale: 11
The lengnth of string locale: 1
Locale(LC_ALL, "") => C
Locale(LC_COLLATE, "en_US") => 
Locale(LC_CTYPE, "C") => C
Locale(LC_MONETARY, "POSIX") => POSIX
Locale(LC_NUMERIC, "en_JP") => 
Locale(LC_TIME, "en_us") => 
Locale(LC_ALL, NULL) => C C POSIX C C C

output on Linuxblazers.mcom.com>locale_linux
Locale(LC_ALL, NULL) => C
Locale(LC_ALL, "") => en_US
The lengnth of string locale: 5
Locale(LC_COLLATE, "en_US") => en_US
Locale(LC_CTYPE, "C") => C
Locale(LC_MONETARY, "POSIX") => C
Locale(LC_NUMERIC, "en_JP") => 
Locale(LC_TIME, "en_us") => 
Locale(LC_ALL, NULL) =>
LC_CTYPE=C;LC_NUMERIC=en_US;LC_TIME=en_US;LC_COLLATE=en_US;LC_MONETARY=C;LC_MESSAGES=en_US

Comment 9

[kzhou]

      From: 
            [e-mail] (John Gaunt)
        To: 
            Kevin Zhou <[e-mail]>
 References: 
Is your change the part in red below?  If so, that won't neccessarily work, 
because you only 
check the first variable, and it is possible for all the locale variables to be 
different. Therefore you
could reset to a different value ( "C" ) another variable that could be en_GB or 
something else. 

The solution is to pull them individually

Comment 10

[kzhou]

       From: 
            [e-mail] (John Gaunt)
        To: 
            Kevin Zhou <[e-mail]>

The scripts Jim put together were to show the difference between platforms of 
the
output for a setlocale call with NC_ALL as the parameter. The problem comes 
because Linux will
report the single lletter C if they are all C, but HP-UX ( and others ) will 
return a space
deliminated string of values. The implementation for Linux really isn't right 
either, because it will fail
if the locales are set differntly ( if i remember correctly ). 

Comment 11

[Jim Dunn]

reassigning to Kevin

Comment 12

[kzhou]

The problem is in file mozilla/intl/locale/src/unix/nsPosixLocale.cpp
and the function nsPosixLocale::GetXPLocale(const char* posixLocale, nsString*
locale).
The first test this func. does is to check if the returned value of
setlocale(LC_ALL,""); is "C" then do such and such, but in AIX and HP-UX it's
"C C C ...". I changed strcmp(posixLocale,"C")==0 to posixLocale[0] == 'C' and
it went through in HP-UX. I don't think there is another locale starting it's
category string as 'C'. If there is we can figure out something else.
For the other locale, the function nsPosixLocale::ParseLocaleString(...) should 
take care of it. I checked "en_US", it worked. So as long as ParseLocaleString()
covers all the locale categories, we are safe.
For John's concern regarding other locale variables the line of code 
result = NewLocale(loc, &mSystemLocale); "nsLocaleService.cpp" line 297
supposes to take care of that if we take look at the function
nsLocaleService::NewLocale(...) also in nsService.cpp.
inside of that function all other locale categories are created based on
return value of setlocale()
for(i=0;i<LocaleListLength;i++) {
	nsString category; category.AssignWithConversion(LocaleList[i]);
	result = resultLocale->AddCategory(category.GetUnicode(),aLocale);
		if (NS_FAILED(result)) { delete resultLocale; return result;}
	}

	return resultLocale->QueryInterface(kILocaleIID,(void**)_retval);
...

so please let me know if I'm not right. And I need the test cases, Jim what's
the test case you saw it's not working before?

napoli>cvs diff -c nsPosixLocale.cpp
Index: nsPosixLocale.cpp
===================================================================
RCS file: /cvsroot/mozilla/intl/locale/src/unix/nsPosixLocale.cpp,v
retrieving revision 1.9
diff -c -r1.9 nsPosixLocale.cpp
*** nsPosixLocale.cpp   2000/05/22 21:36:46     1.9
--- nsPosixLocale.cpp   2000/07/20 22:57:05
***************
*** 94,100 ****
    char  posix_locale[128];
  
    if (posixLocale!=nsnull) {
!     if (strcmp(posixLocale,"C")==0 || strcmp(posixLocale,"POSIX")==0) {
        locale->AssignWithConversion("en-US");
        return NS_OK;
      }
--- 94,100 ----
    char  posix_locale[128];
  
    if (posixLocale!=nsnull) {
!     if ((posixLocale[0] == 'C') || strcmp(posixLocale,"POSIX") == 0) {
        locale->AssignWithConversion("en-US");
        return NS_OK;
      }

Comment 13

[nhottanscp]

There are country code like CN, CH, CO.
How about also checking if a second character is a space when the string has 
more than two characters?

Comment 14

[kzhou]

Thanks, nhotta. I compiled this and hope it will do it.

blazers.mcom.com>cvs diff -c nsPosixLocale.cpp
Index: nsPosixLocale.cpp
===================================================================
RCS file: /cvsroot/mozilla/intl/locale/src/unix/nsPosixLocale.cpp,v
retrieving revision 1.9
diff -c -r1.9 nsPosixLocale.cpp
*** nsPosixLocale.cpp	2000/05/22 21:36:46	1.9
--- nsPosixLocale.cpp	2000/07/21 01:48:34
***************
*** 94,100 ****
    char  posix_locale[128];
  
    if (posixLocale!=nsnull) {
!     if (strcmp(posixLocale,"C")==0 || strcmp(posixLocale,"POSIX")==0) {
        locale->AssignWithConversion("en-US");
        return NS_OK;
      }
--- 94,102 ----
    char  posix_locale[128];
  
    if (posixLocale!=nsnull) {
!     if ((strlen(posixLocale)) == 1
! 	|| ((posixLocale[0] == 'C') && (posixLocale[1] == ' '))
! 	|| strcmp(posixLocale,"POSIX")==0) {
        locale->AssignWithConversion("en-US");
        return NS_OK;
      }
blazers.mcom.com>

Comment 15

[Jim Dunn]

re-assinging back to me... since we can't special case this...
we need a general solution... I will get to it eventually.

If someone needs a workaround let me know... I remember trying
setting LANG=en_us or something and that fixed it for the time
being.

Comment 16

[kzhou]

Hmm, ... what's the special case? You meant GetXPLocale() and NewLocale() 
don't work? I'll check with auther of these files and try to talk to them
if I can find them.

Comment 17

[kzhou]

erik, nhotta, do you know who's the auther of nsLocaleFactory.cpp and
nsPosixLocale.cpp? Is that NewLocale() will take care of those locale categories
creation?

Comment 18

[John Gaunt (redfive)]

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

Comment 19

[nhottanscp]

tague wrote orininal version but he is not here anymore.
nsLocaleFactory.cpp is obsolete, replaced by nsLocaleService.cpp.
 

Comment 20

[Rob Jaworski]

adding me to cc list.

Comment 21

[kzhou]

Thanks, nhotta. And thanks John, for test case. I just tested my fix in
HP-UX for C locale which returns C C C ... It passed. Will see other cases
...

Comment 22

[Shanjian Li]

Since LC_ALL query returns different result on different Unix platform,
I would suggest to skip LC_ALL and query all categories directly. 

Return value from LC_ALL is being formalized, if this formalization 
is necessary, we should do this for all category. 

Comment 23

[kzhou]

Thanks, shanjian for the review. Since NewLocale() is not working. It seems
to me the last try of Jim's direction was right, which means to handle
locale categories one by one no matter getenv("LANG") return null or not
except we should change
category.AssignWithConversion(LocaleList[i]); in line 268 to
category.AssignWithConversion(posix_locale_category[i]);
and change type of posix_loale_category from "static int" to
"cont char*" to fit nsString.AssignWithConversion() function proto type.
However, this would make setlocale(int category, const char *locale); has
problem to compile on HP-UX (don't know for AIX ...) but no problem in Linux.
It looks like HP-UX is more restrict at this point to handle int and char*
than Linux. Jim, Shanjian do you know if there is a way to convert this variabl
between int and a string or there are some other ways to do this?

Warning 495: "../../../dist/include/nsISupportsUtils.h", line 1209 #
    The linkage directive is ignored for an object or function
    declared static.
            static nsIID iid = NS_ISUPPORTS_IID; return iid;
                         ^^^                                
Error 440: "nsLocaleService.cpp", line 105 # Cannot initialize 'const
    char *' with 'int'.
      LC_TIME,
      ^^^^^^^ 
Error 440: "nsLocaleService.cpp", line 106 # Cannot initialize 'const
    char *' with 'int'.
      LC_COLLATE,
      ^^^^^^^^^^ 
Error 440: "nsLocaleService.cpp", line 107 # Cannot initialize 'const
    char *' with 'int'.
      LC_CTYPE,
      ^^^^^^^^ 
Error 440: "nsLocaleService.cpp", line 108 # Cannot initialize 'const
    char *' with 'int'.
      LC_MONETARY,
      ^^^^^^^^^^^ 
Error 440: "nsLocaleService.cpp", line 109 # Cannot initialize 'const
    char *' with 'int'.
      LC_NUMERIC,
      ^^^^^^^^^^ 
Error 440: "nsLocaleService.cpp", line 111 # Cannot initialize 'const
    char *' with 'int'.
      LC_MESSAGES
      ^^^^^^^^^^^
Error 212: "nsLocaleService.cpp", line 255 # Argument type 'const char
    *' does not match expected parameter type 'int'.
                        char* lc_temp = setlocale(posix_locale_category[i],"
                                                  ^^^^^^^^^^^^^^^^^^^^^^^^  
gmake[4]: *** [nsLocaleService.o] Error 2
gmake[4]: Leaving directory `/builds/kzhou/seamonkey/mozilla/intl/locale/src'
gmake[3]: *** [install] Error 2

napoli>cvs diff -c nsLocaleService.cpp
Index: nsLocaleService.cpp
===================================================================
RCS file: /cvsroot/mozilla/intl/locale/src/nsLocaleService.cpp,v
retrieving revision 1.22
diff -c -r1.22 nsLocaleService.cpp
*** nsLocaleService.cpp 2000/05/25 14:28:15     1.22
--- nsLocaleService.cpp 2000/07/25 00:29:38
***************
*** 100,106 ****
  #define NSILOCALE_MAX_ACCEPT_LENGTH           18
  
  #if defined(XP_UNIX) || defined(XP_BEOS)
! static int posix_locale_category[LocaleListLength] =
  {
    LC_TIME,
    LC_COLLATE,
--- 100,106 ----
  #define NSILOCALE_MAX_ACCEPT_LENGTH           18
  
  #if defined(XP_UNIX) || defined(XP_BEOS)
! const char* posix_locale_category[LocaleListLength] =
  {
    LC_TIME,
    LC_COLLATE,
***************
*** 235,254 ****
      nsresult result =
nsComponentManager::CreateInstance(kPosixLocaleFactoryCID,
                             NULL,kIPosixLocaleIID,(void**)&posixConverter);
      if (NS_SUCCEEDED(result) && posixConverter!=nsnull) {
-         char* lc_all = setlocale(LC_ALL,"");
          char* lang = getenv("LANG");
  
-         if (lc_all!=nsnull) {
-             result = posixConverter->GetXPLocale(lc_all,&xpLocale);
-             if (NS_FAILED(result)) { posixConverter->Release(); return; }
-             PRUnichar* loc = xpLocale.ToNewUnicode();
-             result = NewLocale(loc, &mSystemLocale);
-             nsCRT::free(loc);
-             if (NS_FAILED(result)) { posixConverter->Release(); return; }
-             mApplicationLocale=mSystemLocale;
-             mApplicationLocale->AddRef();
-             posixConverter->Release();
-         } else {
              if (lang==nsnull) {
                  xpLocale.AssignWithConversion("en-US");
                  PRUnichar* loc = xpLocale.ToNewUnicode();
--- 235,242 ----
***************
*** 262,271 ****
                  int i;
                  nsString category;
                  nsLocale* resultLocale = new nsLocale();
!                                       if (resultLocale==NULL) {
posixConverter->Release(); return; }
                  for(i=0;i<LocaleListLength;i++) {
                      char* lc_temp = setlocale(posix_locale_category[i],"");
!                     category.AssignWithConversion(LocaleList[i]);
                      if (lc_temp==nsnull)
xpLocale.AssignWithConversion("en-US");
                      else xpLocale.AssignWithConversion(lc_temp);
                      PRUnichar* loc = xpLocale.ToNewUnicode();
--- 250,259 ----
                  int i;
                  nsString category;
                  nsLocale* resultLocale = new nsLocale();
!               if (resultLocale==NULL) { posixConverter->Release(); return; }
                  for(i=0;i<LocaleListLength;i++) {
                      char* lc_temp = setlocale(posix_locale_category[i],"");
!                     category.AssignWithConversion(posix_locale_category[i]);
                      if (lc_temp==nsnull)
xpLocale.AssignWithConversion("en-US");
                      else xpLocale.AssignWithConversion(lc_temp);
                      PRUnichar* loc = xpLocale.ToNewUnicode();
***************
*** 277,283 ****
                 
(void)resultLocale->QueryInterface(kILocaleIID,(void**)&mSystemLocale);
                 
(void)resultLocale->QueryInterface(kILocaleIID,(void**)&mApplicationLocale);
                  posixConverter->Release();
-             }
          }
      }
  #endif // XP_UNIX || XP_BEOS
--- 265,270 ----
napoli>sh

Comment 24

[John Gaunt (redfive)]

I have a fix that worked on HP, am building on Linux right now. It's tough to 
see what is going on from the diff, so I'll outline it. basically, the existing 
situation was taking the variable stored in LC_ALL and populating all the other 
LC variables with that value. Works fine for Linux (LC_ALL=C by default), but 
other Unix-es break (LC_ALL=C C C C C C for HP-UX). So I changed the process to 
query each individual LC variable seperately and store them. There was also an 
issue with using the LANG variable that was fixed as well ( using en-US in a 
place that really called for lang ) I'm still concerned about the use of 
Locale_List and posix_locale_category, as they seem to have a different order 
for the various LC variables - am looking into that.

Looking for review and approval for this fix.

Here is the diff, I will attach the entire nsLocaleService.cpp file with the 
changes as well.




here it is:

239a240,243
>         nsString lc_default;
>         nsString category;
>         nsLocale* resultLocale;
>         int i;
241,243c245,265
<         if (lc_all!=nsnull) {
<             result = posixConverter->GetXPLocale(lc_all,&xpLocale);
<             if (NS_FAILED(result)) { posixConverter->Release(); return; }
---
>         // rewrite for bug #35480(locale handling problem) - jgaunt
>         if ( lc_all != nsnull ) 
>             lc_default.AssignWithConversion(lc_all);
>         else {
>             if ( lang != nsnull ) 
>                 lc_default.AssignWithConversion(lang);
>             else 
>                 lc_default.AssignWithConversion("en-US");
>         }
>         resultLocale = new nsLocale();
>         if (resultLocale==NULL) { 
>             posixConverter->Release(); 
>             return; 
>         }
>         for( i=0; i < LocaleListLength; i++ ) {
>             char* lc_temp = setlocale(posix_locale_category[i],"");
>             category.AssignWithConversion(LocaleList[i]);
>             if (lc_temp==nsnull) 
>                 xpLocale = lc_default;
>             else 
>                 xpLocale.AssignWithConversion(lc_temp);
245c267,269
<             result = NewLocale(loc, &mSystemLocale);
---
>             PRUnichar* cat = category.ToNewUnicode();
>             resultLocale->AddCategory(cat, loc);
>             nsCRT::free(cat);
247,280d270
<             if (NS_FAILED(result)) { posixConverter->Release(); return; }
<             mApplicationLocale=mSystemLocale;
<             mApplicationLocale->AddRef();
<             posixConverter->Release();
<         } else {
<             if (lang==nsnull) {
<                 xpLocale.AssignWithConversion("en-US");
<                 PRUnichar* loc = xpLocale.ToNewUnicode();
<                 result = NewLocale(loc, &mSystemLocale);
<                 nsCRT::free(loc);
<                 if (NS_FAILED(result)) { posixConverter->Release(); return; }
<                 mApplicationLocale = mSystemLocale;
<                 mApplicationLocale->AddRef();
<                 posixConverter->Release();
<             } else {
<                 int i;
<                 nsString category;
<                 nsLocale* resultLocale = new nsLocale();
< 				        if (resultLocale==NULL) { 
posixConverter->Release(); return; }
<                 for(i=0;i<LocaleListLength;i++) {
<                     char* lc_temp = setlocale(posix_locale_category[i],"");
<                     category.AssignWithConversion(LocaleList[i]);
<                     if (lc_temp==nsnull) 
xpLocale.AssignWithConversion("en-US");
<                     else xpLocale.AssignWithConversion(lc_temp);
<                     PRUnichar* loc = xpLocale.ToNewUnicode();
<                     PRUnichar* cat = category.ToNewUnicode();
<                     resultLocale->AddCategory(cat, loc);
<                     nsCRT::free(cat);
<                     nsCRT::free(loc);                    
<                 }
<                 
(void)resultLocale->QueryInterface(kILocaleIID,(void**)&mSystemLocale);
<                 
(void)resultLocale->QueryInterface(kILocaleIID,(void**)&mApplicationLocale);
<                 posixConverter->Release();
<             }
282c272,276
<     }
---
>         
(void)resultLocale->QueryInterface(kILocaleIID,(void**)&mSystemLocale);
>         
(void)resultLocale->QueryInterface(kILocaleIID,(void**)&mApplicationLocale);
>         posixConverter->Release();
>     }  // if ( NS_SUCCEEDED )...
>        

Comment 25

[John Gaunt (redfive)]

Attachment: nsLocaleService.cpp with modifications

Comment 26

[John Gaunt (redfive)]

nominating nsbeta3 - this is currently preventing population of the sidebar 
customization box with additional sidebars on other flavors of Unix ( hp,aix ). 
Locale variables are being incorrectly set with possibly far greater side 
effects than this. currently testing the fix.

Comment 27

[John Gaunt (redfive)]

adding tao to CC list

after some investigation my initial fix needs some refinement. cleaning it up a 
little. 

looked into the posix_locale_categor v. Locale_List thing. they definitely don't 
match up. It almost looks like it was purposeful. If someone know something abou 
this portion of code, please speak up. The order of the LC variables in these 2 
arrays are different ( line 91-126 ), although they are used in combination in a 
for loop that pulls elements based on the loop variable (lines 254-266 - new 
file referenced in the bug). We then store the local variable and it's value in 
a hash. The locale is set properly in the environment, but if we ever pull the 
value out of the hash, it will be for the wrong locale varialbe. :-( 

Comment 28

[tao]

Hi, Naoki:


Comments?

Comment 29

[John Gaunt (redfive)]

here is the meat of the revised changes to nsLocaleService.cpp

Lines 86-116 of my changes - the posix_locale_category used to be in a different 
order ( time,collate,ctype,monetary,numeric,messages(or ctype) ), now it matches 
LocaleList.

//
// implementation constants
const int LocaleListLength = 6;
const char* LocaleList[LocaleListLength] =
{
     NSILOCALE_COLLATE,
    NSILOCALE_CTYPE,
    NSILOCALE_MONETARY,
    NSILOCALE_NUMERIC,
    NSILOCALE_TIME,
    NSILOCALE_MESSAGE
};
#define NSILOCALE_MAX_ACCEPT_LANGUAGE    16
#define NSILOCALE_MAX_ACCEPT_LENGTH        18
#if defined(XP_UNIX) || defined(XP_BEOS)
static int posix_locale_category[LocaleListLength] =
{
  LC_COLLATE,
  LC_CTYPE,
  LC_MONETARY,
  LC_NUMERIC,
  LC_TIME,
#ifdef HAVE_I18N_LC_MESSAGES
  LC_MESSAGES
#else
  LC_CTYPE
#endif
};
#endif


________________________________________________________
Lines 232-276 most importantly, the removal of the call to setlocale(LC_ALL, 
""). Since we are polling all the individual LC variables in the for loop the 
call was unneccessary.



#if defined(XP_UNIX) || defined(XP_BEOS)
    nsIPosixLocale* posixConverter;
    nsString xpLocale;
    nsresult result = nsComponentManager::CreateInstance(kPosixLocaleFactoryCID,
               NULL,kIPosixLocaleIID,(void**)&posixConverter);
    if (NS_SUCCEEDED(result) && posixConverter!=nsnull) {
    char* lang = getenv("LANG");
    nsString lc_default;
    nsString category;
    nsLocale* resultLocale;
    int i;

    // rewrite for bug #35480(locale handling problem) - jgaunt
    // in case of illegal/unsupported types, set a default locale
    if ( lang != nsnull )
        lc_default.AssignWithConversion(lang);
    else
            lc_default.AssignWithConversion("en-US");
    resultLocale = new nsLocale();
    if (resultLocale==NULL) {
            posixConverter->Release();
            return;
    }
    //  loop through the 6 locales and set the program environment to
    //   match the user environment. AddCategory stores the attribute-value
    //   pair in a hash for later reference. ?needed?
    for( i = 0; i < LocaleListLength; i++ ) {
        char* lc_temp = setlocale(posix_locale_category[i],"");
        category.AssignWithConversion(LocaleList[i]);
        if (lc_temp==nsnull)
        xpLocale = lc_default;
            else
        xpLocale.AssignWithConversion(lc_temp);
        PRUnichar* loc = xpLocale.ToNewUnicode();
        PRUnichar* cat = category.ToNewUnicode();
        resultLocale->AddCategory(cat, loc);
        nsCRT::free(cat);
        nsCRT::free(loc);
    }
    (void)resultLocale->QueryInterface(kILocaleIID,(void**)&mSystemLocale);
    (void)resultLocale->QueryInterface(kILocaleIID,(void**)&mApplicationLocale);
    posixConverter->Release();
    }  // if ( NS_SUCCEEDED )...
#endif // XP_UNIX || XP_BEOS

Comment 30

[John Gaunt (redfive)]

Here are some comments from a guy at sun. I've taken out the setlocale(LC_ALL..) 
call, but the individual calls are still there. However, we assign them 2 or 3 
times before we even leave this function, so I am not sure if these comments 
apply. 

>Sorry, I don't have enough time to review entire his change, but I have
>one comment.
>
> There are 4 setlocale() calls in the file, but it must be changed like:
> 
>   FROM:
>       char *lc_all = setlocale(LC_ALL, "");
>   TO:
>       char *lc_all = strdup(setlocale(LC_ALL, ""));   // copy to a new string
>       ...
>       nsCRT::free(lc_all);                            // free the string
>
>The return value of setlocale() may be volatile. Actually, in Solaris, it is,
>so the caller has to make own copy of it if the value is referred to in the
>later codes.
>

Comment 31

[John Gaunt (redfive)]

Attachment: latest and greatest changes, using the getXPLocale call is neccessary

Comment 32

[John Gaunt (redfive)]

Attachment: diff for final fix

Comment 33

[John Gaunt (redfive)]

Attachment: final bug changes as gone over with Tao.

Comment 34

[tao]

Hi, ji/nhotta:


I had applied the proposed patch to my linux debug tree and make it installable
tar ball accessible from the URL field. 

Please give it a test to see if it introduces any regresion or breakage.

thx

Comment 35

[ji]

I'll check it out on Japanese RH6.2.

Comment 36

[Frank Tang]

John, could you please include a cvs -u or -c version here ? The diff is 
difficult to review without -c or -u. Thanks

Comment 37

[John Gaunt (redfive)]

Attachment: previous diff, this time with context! :-)

Comment 38

[ji]

I tried out tao's personal debug build on my Japanese linux box in the following 
Japanese locale names: ja_JP, ja_JP.ujis and ja_JP.eucJP.
In all the japanese locales, IME can be invoked correctly. Mail date shows 
Japanese format. But mail sorting problem is still there. (Bug 18338).
The sidebar is still US contents even I started mozilla in Japanese locale.
Tao, with the fix, should I get Japanese sidebar if I start the browser with 
en_US lang pack but in Japanese locale?

Comment 39

[tao]

John's patch is not to fix bug 18338, so don't worry about that.

For the Sidebar problem, were you able to get Japanese sidebar in build w/o
John's patch?

Comment 40

[tao]

BTW, the remote sidebar panel is determined by the system locale.

Comment 41

[ji]

No, I don't get Japanese sidebar in Japanese locale with or w/o John's fix.

Comment 42

[tao]

The default channels in the Sidebar panels are sticky to the user profile, while
the remote channels appear in the left list of the Customize dialog is 
determined by system locale.

Which one you were referring to ?

Comment 43

[ji]

Checked it again. With John's fix, the remote channels on the customize sidebar 
dialog do have Japanese stuff. 

Comment 44

[tao]

Hi, John:

Looks like we are cleared on Linux. 

Could shanjian help w/ intl Hpux setup/testing?

How about SunOS and AIX?

Comment 45

[John Gaunt (redfive)]

The SunOS build worked fine. Haven't heard from Jim about the AIX build.

Have you tried trieste as an intl hp machine? It should be set up for it.

Comment 46

[tao]

Any idea if the Sun folks purified the patch since you mentioned that some of
the code segment never got executed during your testing?

Also, where this trieste? IQA might need to sit in front of the machine to test
IME.

Comment 47

[John Gaunt (redfive)]

trieste is down the hall from me, i can show someone the way if they need.

no purify has not been run. I don't think she is even set up to run it.

Comment 48

[tao]

Hi, Ji:


Will you be able to rerun your test suite on "trieste"?  It appears to have
ja support.

thx

Comment 49

[ji]

Is trieste a Solaris machine? If it is, do you know where I can find a testable 
build?

Comment 50

[tao]

It's a Hpux11.0 with intl setup. I was thinking about applying your Linux test
cases to it. Is there anything you foresee won't apply?

Comment 51

[ji]

I think all the testcases on linux can be run on HPUX. If the build is ready, I 
can go over to test the IME and the other stuff.

Comment 52

[Jim Dunn]

This seems to do the trick on AIX!

NOTE: you could clean this up a bit by using only one free instead of 2
(but I don't really care)
--old--
if (NS_FAILED(result)) {
    posixConverter->Release();
    nsCRT::free(lc_temp);
    return;
}
              	        
resultLocale->AddCategory(category.GetUnicode(),xpLocale.GetUnicode());
nsCRT::free(lc_temp);
--new--
nsCRT::free(lc_temp);
if (NS_FAILED(result)) {
   posixConverter->Release();
   return;
}
             
resultLocale->AddCategory(category.GetUnicode(),xpLocale.GetUnicode());
--end--

Comment 53

[John Gaunt (redfive)]

I just finished a build of mozilla with my change in it, if someone from iqa 
wants to check it out on trieste. It's working fine with en-us. :-)

Comment 54

[ji]

Checked John's build on triest. Got bus error in ja_JP.SJIS and ja_JP.eucJP
locale. But it looks good in fr_FR.iso-8859-1 locale.

Comment 55

[John Gaunt (redfive)]

Hmm....
I just rebuilt and tested setting LANG and LC_ALL in the environment to 
ja_JP.SJIS and ja_JP.eucJP, but logging in as en-us to the system and it worked 
for me. I even forced the profile manager to come up and it worked too. I wonder 
if it has to do with something in your existing profiles that HP doesn't like ( 
it's happened to us before ), or maybe how HP is dealing with international 
logons. Hmm...

Shirley, if you have time this afternoon to stop by again, i'd like you to have 
another crack at it.

Comment 56

[ji]

I tried as root to launch the build in Japanese locale. 
Same problem as in my own environment, but this time a coredump generated in the
.../dist/bin directory, it might be useful to you.

Comment 57

[John Gaunt (redfive)]

fix has been checked in, the problem with the cores appears to be related to a 
different problem, as it appears with or without the fix in the build.

Comment 58

[John Gaunt (redfive)]

resolving as fixed

Comment 59

[Barrett Lindstrom]

I checked that the Customize Sidebar was populating, since that was a problem 
because of this bug.  I also checked that the sorting works in the mail window.
Tested on build #2000090621 on HPUX 11.00.