54784 - Chrome registry should flush stringbundle and XUL caching after profile selection

Status: VERIFIED FIXED

(Core :: XUL, defect, P2)

Description

[tao]

During start up, Mozilla looks up bin/chrome/user-locales.rdf to determine
which language pack to load, say en-US.jar. However, after profile selection,
if there is a [profile_name]/chrome/user-locales.rdf, it switches to the
"profile" locale (load different language pack, say en-DE.jar). However, no
cache flushing is triggered during this transition. The result: localizable
resources, e.g., navigator.properties, loaded before profile selection are
cached even the user profile locale is different from the start up locale.

Please refer to http://bugscape.netscape.com/show_bug.cgi?id=2518 for details.

Comment 1

[tao]

This bug blocks http://bugscape.netscape.com/show_bug.cgi?id=2348 which is
nsbeta3++. THis results in wrong resources such as home page URL, user agent
string, etc.. in the browser and messenger.

Comment 2

[John Morrison]

Um,  http://bugscape.netscape.com/show_bug.cgi?id=2348 is CLOSED INVALID (Sep 14).

Perhaps, you meant something else :-]

Comment 3

[tao]

YOu're right; I was referring to 2518.

BTW, I noticed that we DO refresh skin in switching from global skin to user
profile skin but not doing the same for locale :-\ (see bug fix for 47081).

Reassign to hyatt.

Comment 4

[tao]

I tried to replace refreshSkins() with reloadChrome() and hit a core dump after
several failure of removing channeling from JAR.

Is this a known bug?

Comment 5

[tao]

replace nsbeta3 w/ rtm.

copy/paste my comments from buscape
-----------------------------------

Furhter findings:
1. In addition to navigator.properties, the following are loaded before profile
   selections:

   necko.properties
   profileManger.properties
   brand.properties

2. Flushing stringBundle after profile selection will fix the problem for
   "Home" and throbber URLs. In other words, clicking on the HOME
   toolbar/menuitem and throbber does load DE page (when en-DE is the profile
   locale) into browser.

3. However, w/ the patch, the locale info in the U-A string and the browser
   startup page are still from en-US.jar

I am attaching the patch below.


Index: nsChromeRegistry.cpp
===================================================================
RCS file: /cvsroot/mozilla/rdf/chrome/src/nsChromeRegistry.cpp,v
retrieving revision 1.159
diff -r1.159 nsChromeRegistry.cpp
469a470,481
> // 1. string bundle cache first.
> nsCOMPtr<nsIStringBundleService> bundleService =
> do_GetService(kStringBundleServiceCID, &rv);
> > if (NS_SUCCEEDED(rv)) {
> rv = bundleService->FlushBundles();
> if (NS_FAILED(rv)) return rv;
> }
> printf("\n--> nsChromeRegistry::ConvertChromeURL()--
bundleService->FlushBundles done\n");
> if (NS_FAILED(rv)) return rv;
> > // 2. skin, too.

Comment 6

[tao]

Add dependency. Fixes for 55155 and 55156 will not work unless this bug is fixed.

Comment 7

[Peter Trudelle]

rtm+ need info, needs super-reviewer, low risk, very high benefit.

Comment 8

[Mike Shaver (:shaver emeritus)]

I don't like the printf there, though I'm not expert on the subject area.  Can
we at least get rid of that, before we put it in the tree?

Comment 9

[tao]

Sure... That's an oversight..

thx

Comment 10

[Alec Flett]

I'm willing to sr=alecf on this one.

Comment 11

[tao]

Hi, Peter:



Is there anything else we can do to rtm+ this?



Thx

Comment 12

[tao]

Attachment: re-submitted patch w/o the printf.

Comment 13

[tao]

Attachment: moving the strbundle flush cache to profile

Comment 14

[tao]

Hi, Hyatt:


Please refer to my comment in bug 55155, it seems to me a better place to
flush stringbundle cache is in nsPorfile.cpp, nsProfile::LoadDefaultProfileDir(
 since that is when user profile selection (of chrome providers) finalizes.

Flushing stringbundle in convertChromeURL will cause the first chrome URL
loading to reuse the cached file if the hashkey is chrome URL.

Any thought?

Add alecf to re-super review and bhuvan for profile code review.


Comments?


thx

Comment 15

[Peter "jag" Annema]

I suspect this currently also causes the View > Text Size menu to come up with
items missing at first start up after a clean build (more specifically, if
user-locales.rdf and component.reg need to be generated by mozilla on start up).

Comment 16

[Alec Flett]

sr=alecf

Comment 17

[Jaime Rodriguez, Jr.]

FTang - Please mark as RTM+, so that PDT can ++ this afternoon.

Comment 18

[David Hyatt]

all set.  pdt, please accept.

Comment 19

[Phil Peterson]

rtm++

Comment 20

[tao]

Hi, Hyatt:


I have yet another better fix which moves the proposed patch in nsProfile.cpp
up to nsProfile::StartApprunner(). By doing this, we can have an easy fix for
bug 55156.

I am posting the new patch below. Need to get gagan to review/approval the
http handler part, though.

Comment 21

[David Hyatt]

I am only landing the chrome registry portion of this.  I will pass the bug off 
to you once I have done this.

Comment 22

[tao]

I had attached the patch to bug 55156. Please see the patch there.

thx

Comment 23

[David Hyatt]

Time out.  This patch isn't in the right place.  I assumed you were patching the 
chrome registry, and instead the flush is happening in the profile code.

This should be self-contained within the chrome registry (which is how I flush 
skin info).  

I will resubmit a new patch for this bug.

Comment 24

[David Hyatt]

Ok, I see the confusion.  YOu submitted a patch that did it in the chrome 
registry.  That's the one I signed off on, and it's the one I think is correct.  
I don't think the flush should happen over in the profile code.  The skin flush 
and the string bundle flush should be together in the chrome registry.

Comment 25

[David Hyatt]

Holding off on checking anything in until I understand why the flush was in the 
profile code.  Was the chrome registry patch insufficient?

Comment 26

[David Hyatt]

I guess I don't care that much.  It just seems like the skin flush and the 
string bundle flush would be together. :)  Is it that you need to flush bundles 
at some time other than a chrome registry locale switch?

Comment 27

[David Hyatt]

Never mind.  Your patch is fine.  I'm giving it back to you, tao, since it has 
nothing to do with the chrome registry now.

Comment 28

[tao]

patch checked in.

Comment 29

[Brian Ryner (not reading)]

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

Comment 30

[John Morrison]

verified fixed (traced through the code and picked up the flush and change of 
locale).