71387 - Safe check usage of GetSelectedLocale()

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect)

Description

[tao]

Hi, Jud:

As you might have known that I recently landed a change to separate 
region-specific resources from UI language resources. As a result, the 
directory names in bin/defaults/* are also changed. In US build, they become
"bin/defaults/profile/US/*" instead of "bin/defaults/profile/en-US/*".

Looking at the usage of GetSelectedLocale() in the following code, I believe
that the package name, "navigator", should be changed to "global-region".
Otherwise, those default files will not be copied as expected.

  mozilla/embedding/browser/powerplant/source/CAppFileLocationProvider.cpp, 
  line 285 -- 
     rv = chromeRegistry->GetSelectedLocale(tmpstr.GetUnicode(), localeName);

  mozilla/embedding/tests/mfcembed/winEmbedFileLocProvider.cpp, line 302 -- 
     rv = chromeRegistry->GetSelectedLocale(tmpstr.GetUnicode(), localeName);

Comment 1

[Judson Valeski]

-> conrad, then maybe chak?

Comment 2

[Chak Nanga]

Tao:
Is the fix you're suggesting is to just change from the string "navigator" to 
the string "global-region" in the places you've mentioned?

Cc:ing Conrad as well...

Comment 3

[Conrad Carlen (not reading bugmail)]

Actually, the GetChromeLocale() routine should just be removed from these files
- it's no longer needed. See
http://lxr.mozilla.org/seamonkey/source/xpcom/io/nsAppFileLocationProvider.cpp#155

Comment 4

[tao]

>Tao:
>Is the fix you're suggesting is to just change from the string "navigator" to 
>the string "global-region" in the places you've mentioned?
yes.

Comment 5

[Judson Valeski]

-> chak, then to conrad for removal/replacement.

Comment 6

[Conrad Carlen (not reading bugmail)]

Attachment: patch to remove this business

Comment 7

[Conrad Carlen (not reading bugmail)]

Here's a patch to remove the locale issue from these files - it's now dealt with
elsewhere. Chak, you can assign this to me if you want.

Comment 8

[Chak Nanga]

Re-Assigning to ccarlen

Comment 9

[Conrad Carlen (not reading bugmail)]

Accepting - tao, can you review? This is the same as what was done for bug
65907. I should have done the same for these two files but didn't :-/

Comment 10

[tao]

Hi, Conrad:

Would you mind briefly desribe what you wanna achieve in the patch? thx

Comment 11

[Conrad Carlen (not reading bugmail)]

tao, did you go back and look at bug 65907? This does the same thing as was done
there: the application-level directory service provider is not responsible for
returning the locale-specific subdir of the profile defaults - only the root
profile defaults. The profile mgr, when it activates a profile, gets the root
profile defaults dir from directory service, gets the locale from chrome, and
then appends that as a sub dir. So, what this patch does is just remove that
unnescesary reference to chrome for the locale. Since that change was made to
profile mgr, this code I want to remove has not been used and this patch just
gets rid of it (plain cleanup).

Comment 12

[tao]

In bug 65907, the locale tag is appended to the path in ProfileManager code.
How about in this bug, i.e., in "embedding"? Does embedding have a 
ProfileManger or the "embedding" does not care and leave it to the application?

Comment 13

[Conrad Carlen (not reading bugmail)]

Both of the embedding samples to which these directory service providers belong
use profile mgr so it's the same as bug 65907.

Comment 14

[tao]

r=tao. 

Comment 15

[Conrad Carlen (not reading bugmail)]

Fix checked in.