142623 - No reload of Language strings due to XUL FastLoad

Status: RESOLVED INACTIVE

(Core :: XUL, defect)

Description

[Robert Kaiser]

When I try to switch Langauge packs in recent builds (e.g. having installed
http://www.hirsch.sth.ac.at/mozilla/de-AT.xpi on a current trunk nightly), the
changes of XUL (.dtd) strings don't apply until I trash my profile's XUL.mfasl
by hand.

Shouldn't that somehow invalidate when switching, so that XUL.mfasl is re-cuilt
after the needed restart?

Comment 1

[Robert Kaiser]

Severity=major, as it breaks the langauge switching functionality. Sorry for the
spam.

Comment 2

[Håkan Waara]

This breaks language switching altogether (until you trash the fastload cache)?
Then this ought to be a blocker, IMHO.

CC'ing fastload people.

Comment 3

[Brendan Eich [:brendan]]

Used to work with just js-in-xul fastload, busted with xul element fastload.  Is
it possible the problem in bug 142165 is related?

/be

Comment 4

[Robert Kaiser]

brendan: as bug 142165 is fixed, and I still see it with today's build (and I
see ben' changes in the changes I pulled), this might not be related.
I'm sure it was caused to appear by the same checkin.

I'm seeing all strings from .properties (stringbundles) called correctly, those
are switched, while all entity-referenced strings (from .dtd files) get not
switched, we still call the cached copies from fastload and don't
invalidate/reload those "copies".

Comment 5

[Boris Zbarsky [:bzbarsky]]

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

Comment 6

[Robert Kaiser]

anyone working on that? I doubt that noone cares about that problem...

Comment 7

[timeless]

i might be stuck working on this. marking embed, wgate, asking for help

Comment 8

[timeless]

Attachment: make stringbundleservice listen for locale changes

this is probably not really related to the main problem, but with it and xul
cache disabled new windows give me correct text for
file(current-localization)>workoffline(current-localization). otherwise you get
file(current-localization)>workoffline(initial-localization).

Next step is of course playing with xul cache.

Comment 9

[timeless]

ok well with that patch (and xul cache enabled) you get 
file(initial-localization)>workoffline(current-localization). otherwise you get 
file(initial-localization)>workoffline(initial-localization).

I'm not quite sure how to do play with the xul cache service.
I suppose that it should be possible to ask the service if the dtd timestamp 
info matches...

Comment 10

[Alec Flett]

Comment on attachment 83591 [details] [diff] [review]
make stringbundleservice listen for locale changes

Hmm.. this is already happening in the chrome registry..supposedly..

http://lxr.mozilla.org/seamonkey/source/rdf/chrome/src/nsChromeRegistry.cpp#265
6

Comment 11

[timeless]

but nothing calls that :) 
http://lxr.mozilla.org/seamonkey/search?string=ReloadChrome

Comment 12

[Aleksander Adamowski]

Is it only on the trunk, or has it been seen on the 1.0 branch?

Comment 13

[John Morrison]

The code in question if definitely only on the trunk.

Comment 14

[Jan M]

i can see this on Win98 so Os should be changed from Linuxd to all...

Comment 15

[timeless]

growing the problem list, mailnews localized folders don't update, so i now have
<mailserver>
- Posteingang
...

 93 PRUnichar *nsMsgFolder::kLocalizedInboxName;
1. these lines should be s prefixed not k prefixed, they're statics not constants
2. they need to listen .. i'll fix that :)

Comment 16

[Ben Goodger (use ben at mozilla dot org for email)]

Well the problem here is that the entity lists are likely merged with the
document by the parser, which is of course not being done in the fast loaded
case, where the label attributes are just coming from the fasl file. An easy fix
for this would be to delete the fasl file when you switch languages. The next
start wouldn't be as fast as the previous one, but since that would be tied to
the act of language switching that's acceptable IMO. 

Comment 17

[timeless]

ben: fwiw that won't be acceptable for worldgate, because they (and I'm no 
longer in their employ -- but we did explain this to brendan, so it's not 
confidential) need to switch locales after mozilla has started, and it's likely 
that 1 in 3 sessions will require a locale switch at start. plus if they ever 
decide that mozilla doesn't leak, then they'd want to switch locales later when 
they recycle mozilla clients.

Comment 18

[Robert Kaiser]

> An easy fix for this would be to delete the fasl file when you switch languages.

I don't think of that as a fix but a workaround - we don't need to rebuild all
of fasl to update this, right (would need deeper look and more coding of course)?

I also think this workaround should be applied to the trunk until we get a real
fix though. If it's that easy, it helps a lot for common cases (just installing
and applying a language pack), and can sit around there until a real fix is found.
The only risk is that perhaps noone will look for the real fix then :(

Comment 19

[Ben Goodger (use ben at mozilla dot org for email)]

Ok. I'm going to post a patch to provide this workaround which is acceptable for
present trunk usages. 

Comment 20

[Robert Kaiser]

Ben: Do you have a patch ready? We won't be able to provide localized versions
of 1.1alpha if this bug still lives in current form.

Comment 21

[Randell Jesup [:jesup] (needinfo me)]

For a more complete solution, should we try loading XUL.<lang>.mfasl first, and
save under XUL.<lang>.mfasl?  That would cost a .mfsal file per lang selected,
but would make switching fast(er).  Certainly that would help us, and might be
of use to others.  (We also need to be able to switch on-the-fly, of course.)

Ben, should you take this bug?

Comment 22

[timeless]

Attachment: make menuitems listen to locale changes

Comment 23

[timeless]

Attachment: make mailfolders listen to locale changes

Comment 24

[timeless]

Attachment: enable live locale switching

Ok, with the three patches above (and perhaps some other local tree changes)

Most things switch, not yet switching:
Window titles.

Things that break:
1. I'm using reload chrome which is a nice thing, except that it destroys the
DOM, so document.content is totally lost (say goodbye to any browser tabs, any
mail compositions, any editor pages, whatever you were inspecting w/
dominspector)
2. For some reason (and this may be unrelated to my changes), JavaScript
console isn't in my window list.

-- Note that at this point the first three patches could be checked in, but
this patch to enable live locale switching is clearly not ready, I'm just
making it available for people (esp: Localizers) who want to try it out.

The first three patches are of course c++ and require rebuilds, this one is
xul/js and you can apply it to any recent mozilla binary.

Comment 25

[timeless]

3. mail really breaks, you'll need to create a new mail window to do anything
Error: uncaught exception: [Exception... "Component returned failure code: 
0x80004005 (NS_ERROR_FAILURE) [nsIObserverService.removeObserver]"  nsresult: 
"0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: 
chrome://messenger/content/commandglue.js :: RemoveMailOfflineObserver :: line 
828"  data: no]

4. Some people have tested live locale/skin-switching and have filed some 
crasher bugs (layout xul/style), and I recently filed a bug aobut an infinite 
loop against venkman.  The venkman bug means that it's very hard for me to 
debug 3.

I'll look into 1 and 3 shortly.

Other things on my plate are skin and profile switching, I have a tendency to 
juggle bugs so I can't guarantee which one I will get to next.

Funny thing, both the menuitem and mailfolder patches were 90% done in my tree, 
mailfolder had a mismatched observer and menuitem just didn't have an observer 
because i couldn't think of where to stick it.

Anyway, could people please test and find reviewers for the first 3 patches 
while I try to figure out the next few rounds?

Comment 26

[Ben Bucksch (:BenB)]

I applied timeless' patches to a Jun 21 trunk source and used a current de-at
nightly XPI. Live switching worked fine, for everything, as far as I could see.
But only if I disabled XUL cache (disabled other caches, too) - otherwise, I had
mixed translations (half german, half english).

Comment 27

[Ben Bucksch (:BenB)]

oh, and Mailnews completely breaks, if you switch locales. no content at all
anymore in the main win. didn't try other apps or tab.

Comment 28

[Robert Kaiser]

I applied patches 1-3 but the original XUL fastload problem is still here...

Comment 29

[Alec Flett]

Comment on attachment 83591 [details] [diff] [review]
make stringbundleservice listen for locale changes

what happens in Observe()? do we just always  just flush the bundles?

Comment 30

[Alec Flett]

Comment on attachment 90312 [details] [diff] [review]
make menuitems listen to locale changes

man someone really needs to write getter_Copies for nsString.

it seems odd to have all these globals that functionally now can hang off the
nsAcceleratorTextService - how about you make the strings member variables of
the service, and then we could make them all nsXPIDLStrings, and avoid all
these extra "new nsString"'s

I guess I prefer to have one root object rather than a whole bunch of little
objectects floating around.

Comment 31

[Alec Flett]

Comment on attachment 90313 [details] [diff] [review]
make mailfolders listen to locale changes

I like the global nsMsgFolderStrings approach. .. however, it scares me a bit
to have a manual call to delete - how about NS_RELEASE() and let the object go
away. You never know what the observer service has done with the object.

Comment 32

[Alec Flett]

Comment on attachment 90315 [details] [diff] [review]
enable live locale switching

sr=alecf

Comment 33

[Alec Flett]

Comment on attachment 83591 [details] [diff] [review]
make stringbundleservice listen for locale changes

oh duh. the 2nd one is a strcmp(). Ignore me. 
sr=alecf

Comment 34

[Robert Kaiser]

timeless: none fo your patches fixes the original problem this bug was reported
against, so your patches are simply misplaced and gainig attracting where the
focus should be on the real L10n blocker here.

Don't get me wrong, I really like your patches, but they should never have been
attached to this bug report, as you don't even try to fix the fastload problem
but steal attention from it.

If I wouldn't like your patches, I'd simply mark them obsolete or needs-work
because they don't fix or work around the issue that has blocked a big bunch of
1.1a localizations, and is about to block another round of 1.1b or even 1.1final
localizations. Thank you.

Who can provide a workaround or fix to at least band aid the fastload problem?

Comment 35

[Taco Witte]

Wouldn't the ugly but easy workaround be a good thing at this moment? It was
already suggested before, but this bug exists for more than _two months_, and
there's still no real solution. I think an ugly but working temporary solution
is better than no solution at all. With the current situation, there will
probably be no (or only some) localizations for Mozilla 1.1(x), and the possibly
existing localizations would be very non-user friendly to use, which I think is
really not nice.
With regards,
Taco (maintainer of Dutch localization)

Comment 36

[Asa Dotzler [:asa]]

Can't ship 1.1final (and I'd hate to even ship beta) with this broken. Who can help?

Comment 37

[Robert Kaiser]

Asa:
In Comment #19, Ben Goodger stated he could write up at least a workaround. I
think this could be enough for beta. If he or anyone else can do that or provide
 even a real fix, this would help a lot...

Comment 38

[John Morrison]

Attachment: patch to pref-contentpacks.xul to blow away XUL.mfasl on a lang or region switch

Comment 39

[John Morrison]

This is a (possible) short-term fix for 1.1b. It just deletes the fastload
file if the user changes region or locale in pref-contentpacks.xul. As far as
I can see, it works for the general case. (There's a small problem if the
user, after changing lang packs, opens a dialog that was not yet in the XUL
memory cache, and then tries to reverse their lang pack switch. In this case,
the fastload file will be open, and on win32 (and mac 9?) we can't remove the
file. Hopefully, that's an edge case).

I can't really test this well, since I don't have other current langpacks handy
(if someone can point me to some, then please do). I hacked up regus.xpi to 
claim to be regnl.xpi (Dutch) and it worked well enough, but I couldn't make
a langnlnl.xpi that would be installed (rush testing job).

It would be better if I could flush the XUL cache, but the only scriptable way
I can do that is through the 'reloadChrome' method on nsIChromeRegistry, but
(a) that's untested code, (b) there's a comment in xpinstall code that says it
crashes, and (c) it will trash mailnews and composer sessions if they are open
at the time of the langpack switch.

The real fix belongs in the c++ chrome registry code, but there's no way that
I'm going to change that code just before shoving 1.1b out the door.

So this is a possibility for 1.1b. It's contained to only those users who ever
do a language/region switch, and only when they do that action. But it's not
perfect, and may fail for some users. Better than nothing? You decide.

Comment 40

[Brendan Eich [:brendan]]

Comment on attachment 91603 [details] [diff] [review]
patch to pref-contentpacks.xul to blow away XUL.mfasl on a lang or region switch

Looks good, thanks for doing this.  Nits below.

sr=[e-mail]

>@@ -67,6 +67,7 @@
>       //Switch the Content pack if necessary
>       var oldContentPack = parent.hPrefWindow.getPref( "localizedstring", "general.useragent.contentlocale");
>       var newContentPack;
>+      var shouldRemoveFaslFile = false;

No need to init to false, undefined converts to false, and
default-init-to-undefined is cheaper than explicit init.

> 
>       try {
>         var listbox = document.getElementById("contentPackList");
>@@ -86,6 +87,7 @@
>         try {
>           var chromeRegistry = Components.classes["@mozilla.org/chrome/chrome-registry;1"].getService(Components.interfaces.nsIXULChromeRegistry);
>           chromeRegistry.selectLocale(newContentPack, true);
>+          shouldRemoveFaslFile = true;
>           var observerService = Components.classes["@mozilla.org/observer-service;1"].getService(Components.interfaces.nsIObserverService);
>           observerService.notifyObservers(null, "locale-selected", null);
>         }
>@@ -115,6 +117,7 @@
>         try {
>           var chromeRegistry = Components.classes["@mozilla.org/chrome/chrome-registry;1"].getService(Components.interfaces.nsIXULChromeRegistry);
>           chromeRegistry.selectLocale(newLanguagePack, true);
>+          shouldRemoveFaslFile = true;
>           var observerService = Components.classes["@mozilla.org/observer-service;1"].getService(Components.interfaces.nsIObserverService);
>           observerService.notifyObservers(null, "locale-selected", null);
>         }
>@@ -122,6 +125,28 @@
>           return false;
>         }
>       }
>+
>+      // If we changed locale, we need to destroy the fastload file so that it
>+      // will load the language strings from the new locale jars. No one should
>+      // have the fastload file open at this moment, so the remove should succeed.
>+      // (XXX actually there is a small window where this is possible, in which 
>+      // case we're screwed).
>+      // XXX This should really be done in the chrome registry itself, not be in
>+      // front-end code, but this patch is only to get this mostly working for 1.1b
>+      // The code below must die before 1.1final!!
>+      if (shouldRemoveFaslFile) {
>+        var faslService, faslFile;

Declare these on first assignment, as is done elsewhere?

>+        try {
>+          const XUL_FASTLOAD_FILE_BASENAME = "XUL";
>+          faslService = Components.classes['@mozilla.org/fast-load-service;1']
>+            .getService(Components.interfaces.nsIFastLoadService);
>+          faslFile = faslService.newFastLoadFile(XUL_FASTLOAD_FILE_BASENAME);
>+          faslFile.remove(false);
>+        } catch(e) {
>+          dump("remove of fastload file failed: " + e);
>+        }
>+      }
>+
>     }
> 
>     function SelectContentPack()

/be

Comment 41

[Robert Kaiser]

John: This approach sounds like a valuable workaround, thanks.
It would get r= from me if I'd know that FastLoadService code you added a bit
better. The patch looks good for me from the view of pref-contentpacks.xul (and
I know that file quite well now).

I still hope someone will write up a real fix for this soon...

Comment 42

[John Morrison]

Attachment: denitted patch to pref-contentpacks.xul to blow away XUL.mfasl on a lang or region switch

Patch updated with brendan's comments. Looking for an r=ben.

(I'll note that this is a workaround because (a) it doesn't belong in the front

end, and (b) it needs to flush the xul memory cache at the same time. But, I 
consider removing the fastload file to be the core of a complete fix. It has 
the attractive characteristic of simplicity.

So, I'm curious as to whether there is a typical end-user use case that would 
warrant "live locale switching". I suppose one example might be a public kiosk,
but I'm wondering if there are others.)

Comment 43

[John Morrison]

Comment on attachment 91704 [details] [diff] [review]
denitted patch to pref-contentpacks.xul to blow away XUL.mfasl on a lang or region switch

carry over sr=

Comment 44

[Ben Goodger (use ben at mozilla dot org for email)]

Comment on attachment 91704 [details] [diff] [review]
denitted patch to pref-contentpacks.xul to blow away XUL.mfasl on a lang or region switch

r=ben

Comment 45

[chris hofmann]

Comment on attachment 91704 [details] [diff] [review]
denitted patch to pref-contentpacks.xul to blow away XUL.mfasl on a lang or region switch

a=chofmann for 1.1b

Comment 46

[John Morrison]

attachment 91704 [details] [diff] [review] checked in by jag on 1.1b trunk, 07/17/2002 21:00

Leaving bug open for a more comprehensive fix.

Comment 47

[Robert Kaiser]

John, I can verify that the workaround does what it is supposed to. I tried with
my XPI Nightly Language Pack from http://www.kairo.at/mozilla/ (German L10n),
and switching between English and German works quite good here with a current
cvs build on Linux. Thanks!

Comment 48

[John Morrison]

For the record, I'm punting on a more comprehensive solution for 1.1final. 

Comment 49

[Jan M]

is still someone working on this?
just want to remind anyone that bug is nit solved yet, only worked around...

Comment 50

[Jo Hermans]

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

Comment 51

[Jo Hermans]

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

Comment 52

[Randell Jesup [:jesup] (needinfo me)]

jrgm wrote:
>So, I'm curious as to whether there is a typical end-user use case that would 
>warrant "live locale switching". I suppose one example might be a public kiosk,
>but I'm wondering if there are others.)

We (wgate) need it definitely.  We support (in our old browser) multiple users
in a household who use different languages, as well as multiple languages within
a cable system.  When you enter the system, you're in the system's (or the
account's) default language.  When you log in as a specific user
(accounts/households can have several users - bobby/suzy/dad/mom/grandpa, for
example), we need to switch the locale to the correct locale for that user.

Comment 53

[Aleksander Adamowski]

jrgm wrote:
>So, I'm curious as to whether there is a typical end-user use case that would 
>warrant "live locale switching". I suppose one example might be a public kiosk,
>but I'm wondering if there are others.)

Another scenario:
A user downloads a standard build from mozilla.org, containing only the English
locale. He then downloads his native language and content pack, installs it and
switches to it in Edit->Preferences->Appearance->Languages/Content.

If he's not a hardcore Mozilla developer/tester (not aware of this bug), he
expects to see the UI switch to his native language.

Comment 54

[Luca Olivetti]

I don't know if it's related to XUL fastload but I'm seeing this problem with
the 1.1 release under windows (didn't test yet under linux), so if the
workaround is included it isn't working. The strange thing is that on my machine
(where the profile probably dates back from 0.9.something) I can switch
languages ok (that is, the change takes effect restarting mozilla), so I made a
fresh install of mozilla 1.1 in a colleague's computer who doesn't understand
english, and there it doesn't change language (minus some menu items and
dialogs). I've tried with spanish (Argentina), galician and italian, but the
result is the same: the main menu bar stays in english.

Comment 55

[[e-mail] (away - not reading bugmail)]

Luca,

try creating a new profile. I believe this might be related to chrome.rdf in
local user profiles. Try deleting or at least renaming all instances of
"chrome.rdf".

Comment 56

[Luca Olivetti]

I'll try on monday at work when I see my colleague and I'll report the result.
Now that you mention profiles, the "failing" one was migrated from netscape 4.

Comment 57

[Luca Olivetti]

Ok, I deleted XUL.mfl from the existing profile and now everything is in spanish.
No need to create a new profile or delete chrome.rdf. Maybe the workaround isn't
working in all situations.

Comment 58

[Robert Kaiser]

Comment on attachment 91704 [details] [diff] [review]
denitted patch to pref-contentpacks.xul to blow away XUL.mfasl on a lang or region switch

>+      // (XXX actually there is a small window where this is possible, in which 
>+      // case we're screwed).

This "small window" has been seen by quite a bunch of Mozilla 1.1 German
users...

>+      // XXX This should really be done in the chrome registry itself, not be in
>+      // front-end code, but this patch is only to get this mostly working for 1.1b
>+      // The code below must die before 1.1final!!

What about this last line? It hasn't died before 1.1final, neither before
1.2alpha, will this still live in 1.2beta and 1.2 final ?????

Comment 59

[Croco Dil]

at 1.2a+ (2002092108)under win 98 se and 2k the language settings say
english(US) (needs update) when I load the de-AT xpi it says the same with the
german one and when you try to set something up the window just jitters and you
can't click on anything.
trashing the xul.mfl and the chrome.rdf didn't bring out any results

Comment 60

[Christian Stadler]

This is not a bug, it's a feature ;) The language-pack is for 20020918 and your
Version is 20020921, which means, that the language-pack is too old for your
version.

HTH

Christian Stadler

Comment 61

[Robert Kaiser]

Croco, Christian: This was a different thing, bug 168923, just resolved a minute
ago. Should be working again with upcoming nightlies...

Comment 62

[Croco Dil]

christian:
well at least I'm not yet deaf :-)
kairo:
thanks for the info!

Comment 63

[Christian Stadler]

Croco:
Me neither. I haven't seen, that you've meant the english language pack, too and
I haven't thought, that this was a bug :)

Robert: 
Thanks from me, too :)

Comment 64

[Joerg Starkmuth]

If this is the bug causing only a part of the menu items to be translated after
switching language packs, it is still present in Moz 1.3 (Windows XP). I first
saw it in Moz 1.2.1 trying to use the German (Austria) language pack. Manually
deleting the XUL file before switching removes the problem.

Joerg

Comment 65

[Samir Gehani]

adt: nsbeta1-

Comment 66

[:aceman]

The deleting of Xul.mfl after language switch worked in 1.3.

Now in 1.5, I think it was not deleted so I deleted it manually. Now, the
XUL.mfl is recreated on every Mozilla start. Did the format of the file change?
Now I see the language pack name written in the beginning of it. In 1.3 I didn't
see it.

Comment 67

[Robert Kaiser]

hmm, probably timeless does know something here...

Comment 68

[Simon Paquet [:sipaq]]

Does not qualify as a blocker.

Comment 69

[Robert Kaiser]

Simon:
What reasons do you have to tell that?
Did you read Comment #1, Comment #2, Comment #36?
Did you realize that the temporary workaround that is still in place was said to
be just enough to ship a beta, but too weak to ship a final (1.1 then)?
Did you know that lots of people have strange switching problems and still have
to manually kill their FastLoad file because of this (and maybe other problems)
still unresolved?
Are you either the owner of the bug or owner/peer of its module?
Still, if in one of those groups, how can your first post to the bug just state
"it's no blocker" without telling reasons why it shouldn't be?

As a major L10n contributor, I feel very very uncomfortable with having such a
major bug still fixed by a semi-working temporary beta workaround, and I'm
readin quite a bunch of people's e-mail and newsgroup posting who would have
less problems and be more satisfied if this problem could be solved correctly.
The bad thing is, that 1) I don't know the code that needs the fix, 2) I don't
know Mozilla's C/C++ too well at all, and 3) I'm distracted from the real L10n
work by having to answer people's mail about having a problem caused by this bug.

Comment 70

[Giacomo Magnini]

I don't know if this is of any help, but while updating my translation pack, I
saw this strange behaviour:
1) Switched back to English and close mozilla
2) Opened mozilla again and opened the updated translation .xpi
3) Mozilla showed the filename with escaped spaces (%20) and installed the thing
with success
4) Switched to Italian and closed mozilla
5) Opened mozilla again and everything was still in english, so back to the
usual workaround (ie, delete XUL.mfl)
Nothing strange upto here, but after some other changes to the translation, I
have performed steps 1 and 2 above again. This time mozilla showed the file name
*without* the escaped spaces (c:\Documents and Settings\etc.) and installed the
translation successfully. After repeating point 4, on the new launch of mozilla,
it showed itself fully translated and with no need of the workaround!
I don't know if this is reproducible, but I hope it givs some new idea about the
source of the problem.

Comment 71

[Michael Stadlbauer]

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

Comment 73

[BugBot [:suhaib / :marco/ :calixte]]

In the process of migrating remaining bugs to the new severity system, the severity for this bug cannot be automatically determined. Please retriage this bug using the new severity system.

Comment 74

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:enndeakin, could you have a look please?

For more information, please visit BugBot documentation.

Comment 75

[Neil Deakin]

Probably no longer relevant.