287687 - ChatZilla should not hardcode "Mozilla" as the suite name

Status: RESOLVED FIXED

(Other Applications :: ChatZilla, defect)

Description

[Robert Kaiser]

Instead of using the hardcoded word "Mozilla" as the brand name of the suite
ChatZilla can run in, it should use the &brandShortName; entity from brand.dtd
(or the brandShortName string from brand.poperties).
That way it's easier for the new suite project to rebrand the suite (see bug
285696) as well as for some distributor to change to his brand.
Where it can't be avoided, a hardcoded change or inclusion of the new suite name
(tba) might be a good idea (see "reasonable hardcoding" in list below).

(I'm marking this blocking suite-rebranding though it's no hard blocker on that
bug, esp. as calendar isn't part of the default build)

Here's a list of affected files/locale strings:
chatzilla.properties: msg.dcc.not.possible (reasonable hardcoding)
chatzilla.properties: msg.err.offline (reasonable hardcoding)
chatzilla.properties: cmd.cmd-mozilla-prefs.label
chatzilla.properties: cmd.exit-mozilla.help
chatzilla.properties: cmd.quit-mozilla.help

Comment 1

[:Gijs (he/him)]

Additionally, I guess that the following should also be adjusted?

http://lxr.mozilla.org/seamonkey/source/extensions/irc/xul/content/static.js#316

Is used internally to identify, for example, where the ChatZilla preferences go
(they are in Edit > Preferences on Seamonkey, and in Tools > Options on Firefox)
I haven't looked up all of those checks, but it shouldn't be too hard.

This is something internal to ChatZilla, which the average enduser won't notice.
More important is that it is also used here (see /ctcp <nickname> host):
http://lxr.mozilla.org/seamonkey/source/extensions/irc/js/lib/irc.js#2382
(http://lxr.mozilla.org/seamonkey/source/extensions/irc/xul/content/static.js#347)

I'm guessing we should change all the hardcoded "Mozilla" strings to the new
brand name used by the community (say, "Seamonkey"), and have the ctcp reply use
brand.dtd / brand.properties to get it right when people build their own
un-official builds?

Comment 2

[Robert Kaiser]

(In reply to comment #1)
> Is used internally to identify, for example, where the ChatZilla preferences go
> (they are in Edit > Preferences on Seamonkey, and in Tools > Options on Firefox)
> I haven't looked up all of those checks, but it shouldn't be too hard.

Well, for strings that aren't shown to the user anywhere, it doesn't matter too
much what you use. That's completely up to you...
If you show it, something like the generic "suite" or so might be best...

> I'm guessing we should change all the hardcoded "Mozilla" strings to the new
> brand name used by the community (say, "Seamonkey"), and have the ctcp reply use
> brand.dtd / brand.properties to get it right when people build their own
> un-official builds?

Actually, you should always use the brand bundle strings when showing to the
user, as someone like Beonex or IBM or whoever might use it in a redistributed
suite that has a different name, or soemone like Netscape or whoever might use
it in a redistributed Firefox with a different name.

Comment 3

[Ian Neal]

Hannibal, are you planning to create a patch for this? "Suite" sounds a good
substitution for "Mozilla" for internal use and as Robert says for the other stuff.

Comment 4

[:Gijs (he/him)]

Attachment: Preliminary patch

Fixes client.host

Ian, however confident you may be in my capabilities (not able to judge that,
but anyway), I'm as of yet still a Mozilla newbie. I'm probably overlooking
something, but I can't figure out how we should fix the help items.
Because that would require (as far as I can see) a stringbundle hack (hacking
brand names in afterwards) which is particularly messy code.
Either that or to have the entities entered inside the chatzilla.properties
file, and I don't know how to do that (it may not be possible, but how should I
know).
This is what I *can* fix, and I'm sure I could do the rest if someone gave me a
hand, but in that case you may just want to fix it yourself (I'm guessing we
need these bugs fixed asap?).

On a more professional note, do we also need to fix the "Firefox" strings for
client.host? If so, what should we replace those with?

Note to whomever makes the final patch: don't forget to start using pretty
brand names in the ctcp host reply!

Comment 5

[Ian Neal]

Attachment: Revised patch v0.1a

Changes from preliminary patch
* Changes var Mozilla as well
* Makes use of navigator.vendor for HOST_RPLY
* Revises how VERSION_RPLY/client.userAgent is arrived at

Comment 6

[:Gijs (he/him)]

Attachment: Patch v0.1b

Changes:
* Adding brand.properties to the list of stringbundles
* Made another small change in getting VERSION_RPLY, and used brand names for
HOST_RPLY
* Adding client.hostShortName, client.hostFullName, client.hostVendorName for
quick access to brand.properties stuff
* Fixing chatzilla.properties, with help of some js in commands.js and menus.js


Requesting review.

Ian, I didn't label your patch as obsolete because you assigned yourself to
this bug, and I didn't feel it was appropriate for me to just kick your patch
off. Sorry if this was the wrong thing to do.

Patch tested in Mozilla 1.8b2 (20050325) and Mozilla 1.0. Both seemed to work
fine.

Comment 7

[Ian Neal]

Comment on attachment 179024 [details] [diff] [review]
Patch v0.1b

Thanks for the updated patch - thinking about it in hindsight, still having
"Mozilla" hardcoded in, even if as a fallback, was wrong. Continuing on my
route, I would have probably used navigator.appCodeName as the fallback.

Thanks also for picking up the bits I missed in chatzilla.properties

Now to your patch:
>Index: mozilla/extensions/irc/xul/content/messages.js
>===================================================================
>+    client.hostShortName = brandBundle.GetStringFromName("brandShortName");
>+    client.hostVendorName = brandBundle.GetStringFromName("vendorShortName");
>+    try 
>+    {
>+        client.hostFullName = brandBundle.GetStringFromName("brandFullName");
>+    }
>+    catch(exception)
>+    {
>+        // Not all versions of Seamonkey have brandFullName defined.
>+        client.hostFullName = client.hostShortName;
>+    }
>+
As far as I can see you do not currently use client.hostVendorName and
client.hostFullName - are these for use a future patch?

If you do keep them I'd change the comment from Seamonkey to Suite.

Have you checked what this gives if you are running CZ under XULrunner - does
it have a brand.properties in that circumstance?

I'm happy for you to continue to develop the patch - the important thing is
that it is being worked on. You can also take ownership of the bug, if you want
:-)

Comment 8

[:Gijs (he/him)]

Attachment: Patch v0.2a

Changes:
* changed comment "Seamonkey" into "Suite" as per Ian's suggestion
* added brand.properties to the jar to get XULRunner to use that as the
branding file (this was the main goal of moving brand.properties from
chrome://global/ to chrome://branding/; now XULRunner apps can define their
own)
* removed the chrome registry check, instead I changed some code in
message-manager to cope with errors when adding stringbundles (and actually
removing them again, instead of leaving the junk behind)

Ian, the main reason to include the other strings from brand.properties was to
have easy access to them should we ever want to. If this is not necessary, we
could remove them. I'll leave that up to rginda.

rginda, it also seems client.hostPlatform is totally unused. Is there any
deeper point to it, or can we remove it just as well?

Something I did note is that ChatZilla's version reply gets a bit strange with
this patch, when used on XULRunner (with the separate brand.properties file).
It became something like:

ChatZilla 0.9.67 [ChatZilla 0.9.67/20050329]

I'm not saying there's anything *really* wrong with this, but it does look a
little bit odd.

I tested this patch on Moz 1.0, 6-day-old Mozilla nightly (20050325) and on
XULRunner (20050329). All worked fine, apart from bug 288410, which I doubt is
our fault (as it runs fine the first time).

Comment 9

[James Ross]

Comment on attachment 179155 [details] [diff] [review]
Patch v0.2a

>Index: mozilla/extensions/irc/jar.mn
>+	branding/locale/brand.properties             (xul/locale/en-US/brand.properties)

Evil evil evil. Never include global banding data inside an extension.


>Index: mozilla/extensions/irc/js/lib/message-manager.js
>-    this.importBundle(bundle, targetWindow, this.bundleList.length - 1);
>-
>+    // the bundle will load if the file doesn't exist. this will fail though.
>+    // We want to be clean and remove the bundle again.
>+    try
>+    {
>+        this.importBundle(bundle, targetWindow, this.bundleList.length - 1);
>+    }
>+    catch (exception)
>+    {
>+        // Clean up and return the exception.
>+        this.bundleList.pop();
>+        delete bundle;
>+        throw(exception);
>+    }

This seems reasonable...


>Index: mozilla/extensions/irc/xul/content/commands.js
>     for (var i in ary)
>     {
>         display (getMsg(MSG_FMT_USAGE, [ary[i].name, ary[i].usage]), MT_USAGE);
>-        display (ary[i].help, MT_HELP);
>+        // Replace brand names in help messages:
>+        if (ary[i].help.match(/%S/))
>+            display (ary[i].help.replace(/%S/, client.hostShortName), MT_HELP);
>+        else
>+            display (ary[i].help, MT_HELP);

Bad way to do it. I don't like it happening at all, though.

>-        if (client.host == "Mozilla") {
>+        if (client.host == "Suite") {

This, and all other changes to client.host should not happen.

If you must, add a new value, but I do not see this is in any way needed - it
is still the same codebase. client.host indicates what external resources and
functions it expects to exist, and has nothing to do with branding.


>Index: mozilla/extensions/irc/xul/content/menus.js
>+    var mozPrefs = getMsg("cmd.cmd-mozilla-prefs.label", client.hostShortName);
>+    client.commandManager.commands["cmd-mozilla-prefs"].label = mozPrefs;

*shudder*

Do not do this. See my suggestion further down.


>Index: mozilla/extensions/irc/xul/content/messages.js
>===================================================================
>RCS file: /cvsroot/mozilla/extensions/irc/xul/content/messages.js,v
>retrieving revision 1.6
>diff -p -u -8 -r1.6 messages.js
>--- mozilla/extensions/irc/xul/content/messages.js	21 Feb 2005 18:25:55 -0000	1.6
>+++ mozilla/extensions/irc/xul/content/messages.js	31 Mar 2005 08:45:57 -0000
>@@ -39,16 +39,42 @@
> 
> function initMessages()
> {
>     var path = "chrome://chatzilla/locale/chatzilla.properties";
>     
>     client.messageManager = new MessageManager();
>     client.defaultBundle = client.messageManager.addBundle(path);
> 
>+    // Use branding:
>+    var brandBundle;
>+    try 
>+    {
>+        path = "chrome://branding/locale/brand.properties";
>+        brandBundle = client.messageManager.addBundle(path);
>+    }
>+    catch (exception)
>+    {
>+        // The old location of the branding file.
>+        path = "chrome://global/locale/brand.properties";
>+        brandBundle = client.messageManager.addBundle(path);
>+    }
>+
>+    client.hostShortName = brandBundle.GetStringFromName("brandShortName");
>+    client.hostVendorName = brandBundle.GetStringFromName("vendorShortName");

Bad choice of names; they should, if stored like this at all should mention
that they are branding only.

>+    try 
>+    {
>+        client.hostFullName = brandBundle.GetStringFromName("brandFullName");
>+    }
>+    catch(exception)
>+    {
>+        // Not all versions of Suite have brandFullName defined.
>+        client.hostFullName = client.hostShortName;
>+    }
>+
>     client.viewName = client.unicodeName = MSG_CLIENT_NAME;
>     client.responseCodeMap =
>         {
>             "HELLO": MSG_RSP_HELLO,
>             "HELP" : MSG_RSP_HELP,
>             "USAGE": MSG_RSP_USAGE,
>             "ERROR": MSG_RSP_ERROR,
>             "WARNING": MSG_RSP_WARN,


>Index: mozilla/extensions/irc/xul/content/static.js
>     var ary = navigator.userAgent.match (/(rv:[^;)\s]+).*?Gecko\/(\d+)/);
>+    var ua = navigator.userAgent;
>     if (ary)
>     {
>         if (navigator.vendor)
>-        {
>-            client.userAgent = getMsg(MSG_VERSION_REPLY,
>-                                      [ver, navigator.vendor + " " + 
>-                                       navigator.vendorSub + "/" + ary[2]]);
>-        }
>+            ua = navigator.vendor + " " + navigator.vendorSub;
>         else
>-        {
>-            client.userAgent = getMsg(MSG_VERSION_REPLY,
>-                                      [ver, client.host + " " + 
>-                                       ary[1] + "/" + ary[2]]);
>-        }
>-    }
>-    else
>-    {
>-        client.userAgent = getMsg(MSG_VERSION_REPLY,
>-                                  [ver, navigator.userAgent]);
>+            ua = client.hostShortName + " " + ary[1];
>+        ua = ua + "/" + ary[2];
>     }
>+    client.userAgent = getMsg(MSG_VERSION_REPLY, [ver, ua]);

I can't see the wood from the trees with this lot. Assuming proper changes to
hostShortName  is should be ok.

>     CIRCServer.prototype.OS_RPLY = navigator.oscpu + " (" +
>                                    navigator.platform + ")";
>-    CIRCServer.prototype.HOST_RPLY = client.host + ", " + client.platform;
>+
>+    // use branding for ctcp host reply.
>+    CIRCServer.prototype.HOST_RPLY = client.hostShortName + ", "
>+                                     + client.platform;

Similar.


>Index: mozilla/extensions/irc/xul/locale/en-US/brand.properties
>+brandShortName = ChatZilla
>+brandFullName  = ChatZilla
>+vendorShortName = Mozilla

Evil evil evil.


>Index: mozilla/extensions/irc/xul/locale/en-US/chatzilla.properties
>-cmd.cmd-mozilla-prefs.label  = &Mozilla Preferences...
>+cmd.cmd-mozilla-prefs.label  = &%S Preferences...

>+cmd.exit-mozilla.help  = Exit %S.

>+cmd.quit-mozilla.help   = Quit %S.



Ok, so that lot sucks a fair bit.

The line in jar.mn is totally wrong and should never happen. You are trying to
override/replace a global file with one from ChatZilla alone! Bad!

The 'solution' for localised messages leaves something to be desired, too. I
suggest doing the following:

  - load some brand file from some place, if it exists, like the current code.
  - store the branding data in message-manager.js somewhere, not the client (it
couldn't give a damn what banding is being used). Perhaps load it in
initMessages (like now), but then give the data over to the created message
manager. May need to do it before adding the default bundle, not sure when it
imports the first lot.
  - adjust the Message Manager to replace "&shortBrandName;" and other
appropriate strings with the data loaded from the brand stringbundle. If you
can't load it, use client.host (assuming we set that up before messages, which
actually seems unlikely... have to think about this).
  - probably expose the branding via, say,
MessageManager.prototype.getBranding(name).
  - use the above to get the branding string for use in the HOST_RPLY.

Urgh.

End transmission.

Comment 10

[:Gijs (he/him)]

Comment on attachment 179155 [details] [diff] [review]
Patch v0.2a

Thank you Silver. This was sort of what I meant when being not so sure about
things. I will have a look at your suggestions.

Off-topic: welcome back :-).

Comment 11

[:Gijs (he/him)]

Attachment: Patch v0.3

New, very different patch:
* MessageManager.loadBrands() is called from initMessages. 
* MessageManager.loadBrands uses the interface XULRunner provides to the data
entered into application.ini to get its values if we're in XULRunner.
  If we're not in XULRunner, it will attempt getting brand.properties from the
two possible locations, and use values from there.
* MessageManager.brandShortName, MessageManager.brandFullName and
MessageManager.brandVendorName should now contain valid values. (if branding
was done properly by the vendor)
* &brandShortName;, &brandFullName; and &brandVendorName; get replaced whenever
someone gets a locale string. To do this, the code uses the
MessageManager.replaceBranding function as a lambda replace.
* This means the help items for exit-mozilla / quit-mozilla now read:
  Firefox:   Quit Firefox
  Suite:     Quit Mozilla
  XULRunner: Quit ChatZilla
  The commands are still called 'exit-mozilla' and 'quit-mozilla' though. 
  Maybe we should call them 'exit-host' or something?

Generated CTCP version and host replies on tested builds:
Firefox 1.0+:	 Chatzilla 0.9.67 [Firefox 1.0+/20050405] 
		 Firefox, Windows
Mozilla 1.0:	 Chatzilla 0.9.67 [Mozilla rv:1.0.0/20020530]
		 Mozilla, Windows
Seamonkey 1.8b2: Chatzilla 0.9.67 [Mozilla rv:1.8b2/20050407]
		 Mozilla, Windows
XULrunner:	 Chatzilla 0.9.67 [ChatZilla 0.9.67/20050329]
		 ChatZilla, Windows

Since you were nice enough to 'review' the previous patch, I'm requesting
review from you again, Silver. Hoping this is better, of course :-).

Comment 12

[Ian Neal]

Was this intended?
-cli_inamesMsg3=You must supply a channel name to use /names from this view.

Comment 13

[:Gijs (he/him)]

In reply to comment #12:
Sort of. Something was broken, I removed that, it turned out not to have caused
the breaking, but I kept it out anyway since that string is not used. 

Probably not removed in a previous patch while it should have. (aka might as
well clear up that mess while we're at it).

Comment 14

[Ian Neal]

(In reply to comment #11)
> Generated CTCP version and host replies on tested builds:
> Seamonkey 1.8b2: Chatzilla 0.9.67 [Mozilla rv:1.8b2/20050407]
> 		 Mozilla, Windows
I presume this should be Mozilla 1.8b2, otherwise surely it should be:
Seamonkey 1.8b2: Chatzilla 0.9.67 [Seamonkey rv:1.8b2/20050407]
 		 Seamonkey, Windows
or Allizom, etc

Comment 15

[James Ross]

Comment on attachment 180031 [details] [diff] [review]
Patch v0.3

>Index: mozilla/extensions/irc/js/lib/message-manager.js

>+MessageManager.prototype.loadBrands =
>+function mm_loadbrands()
>+{
>+    if (!("getBrowserURL" in window))

Hm, if app compat is already inited (is it?), you should use that. If not, may
be worth trying to make that init before messages.

>+    {
>+        // In XULRunner we have to use the application.ini stuff
>+        var app = getService("@mozilla.org/xre/app-info;1", "nsIXULAppInfo");
>+        this.brandShortName = app.name;
>+        this.brandFullName = app.name + " " + app.version;
>+        this.brandVendorName = app.vendor;
>+        return;
>+    }
>+
>+    var brandBundle;
>+    var path = "chrome://branding/locale/brand.properties";
>+    try
>+    {
>+        brandBundle = this.addBundle(path);
>+    }
>+    catch (exception)
>+    {
>+        // May be an older mozilla version, try another location.
>+        path = "chrome://global/locale/brand.properties";
>+        brandBundle = this.addBundle(path);

Don't you need to use a nested try/catch here? Or is at least one location
/gurenteed/ to exist?

>+    }
>+
>+    this.brandShortName = brandBundle.GetStringFromName("brandShortName");
>+    this.brandVendorName = brandBundle.GetStringFromName("vendorShortName");
>+    // Not all versions of Suite / Fx have this defined; Cope:
>+    try
>+    {
>+        this.brandFullName = brandBundle.GetStringFromName("brandFullName");
>+    }
>+    catch(exception)
>+    {
>+        this.brandFullName = this.brandShortName;
>+    }
>+
>+    // Remove all of this junk, or it will be the default bundle for getMsg...
>+    this.bundleList.pop();
>+    delete brandBundle;

This delete is not needed, as it is a local variable and vanishes when the
function returns.

I also notice that you always pop the bundle list, yet that also happens when
it fails to load one. You'll need to make sure you only get here if it has
loaded one and only one bundle. :)

>+}
>+
> MessageManager.prototype.addBundle = 
> function mm_addbundle(bundlePath, targetWindow)
> {
>     var bundle = srGetStrBundle(bundlePath);
>     this.bundleList.push(bundle);
> 
>-    this.importBundle(bundle, targetWindow, this.bundleList.length - 1);
>-
>+    // the bundle will load if the file doesn't exist. this will fail though.
>+    // We want to be clean and remove the bundle again.
>+    try
>+    {
>+        this.importBundle(bundle, targetWindow, this.bundleList.length - 1);
>+    }
>+    catch (exception)
>+    {
>+        // Clean up and return the exception.
>+        this.bundleList.pop();
>+        delete bundle;

Again, this is a local variable.

>+        throw(exception);

I'm not sure I like the brackets there... throw has traditionally always been
used without them.

>+            constValue.replace(/\&brand(ShortName|FullName|VendorName)\;/g,
>+                               this.replaceBranding);

I think it will be more flexible if you call the replace wrapper for all
entities, i.e. use the regexp:

    /&(\w+);/

This way, we can add more very easily.

>+        // Replace branding.
>+        rv = rv.replace(/\&brand(ShortName|FullName|VendorName)\;/g,
>+                        this.replaceBranding);

Obviously, same change to the regexp.

>+MessageManager.prototype.replaceBranding = 
>+function mm_replacebranding(matched, type)
>+{
>+    return client.messageManager["brand" + type];
>+}

And now here you can just pass type (or rename it to |entity|).


>Index: mozilla/extensions/irc/xul/content/messages.js

>     client.messageManager = new MessageManager();
>+    client.messageManager.loadBrands();

I wonder if it would not be better to make the MM do that all by itself when it
is created...


>Index: mozilla/extensions/irc/xul/content/static.js

>     CIRCServer.prototype.OS_RPLY = navigator.oscpu + " (" +
>                                    navigator.platform + ")";
>-    CIRCServer.prototype.HOST_RPLY = client.host + ", " + client.platform;
>+
>+    // use branding for ctcp host reply.
>+    CIRCServer.prototype.HOST_RPLY = client.messageManager.brandShortName
>+                                     + ", " + client.platform;


>Index: mozilla/extensions/irc/xul/locale/en-US/chatzilla.properties

>-cli_inamesMsg3=You must supply a channel name to use /names from this view.

Nice one. We don't use it, anyway, do we?

Comment 16

[:Gijs (he/him)]

(In reply to comment #15)
> (From update of attachment 180031 [details] [diff] [review] [edit])
> >Index: mozilla/extensions/irc/js/lib/message-manager.js
> 
> >+MessageManager.prototype.loadBrands =
> >+function mm_loadbrands()
> >+{
> >+    if (!("getBrowserURL" in window))
> 
> Hm, if app compat is already inited (is it?), you should use that. If not, may
> be worth trying to make that init before messages.

We discussed this already. I'll try to do whatever I can ;-).

> >+    catch (exception)
> >+    {
> >+        // May be an older mozilla version, try another location.
> >+        path = "chrome://global/locale/brand.properties";
> >+        brandBundle = this.addBundle(path);
> 
> Don't you need to use a nested try/catch here? Or is at least one location
> /guaranteed/ to exist?

Yes, either should exist.

> >+    this.bundleList.pop();
> >+    delete brandBundle;
> 
> This delete is not needed, as it is a local variable and vanishes when the
> function returns.
> 
> I also notice that you always pop the bundle list, yet that also happens when
> it fails to load one. You'll need to make sure you only get here if it has
> loaded one and only one bundle. :)

Right, will do that.

> >+            constValue.replace(/\&brand(ShortName|FullName|VendorName)\;/g,
> >+                               this.replaceBranding);
> 
> I think it will be more flexible if you call the replace wrapper for all
> entities, i.e. use the regexp:
> 
>     /&(\w+);/
> 
> This way, we can add more very easily.

> >+MessageManager.prototype.replaceBranding = 
> >+function mm_replacebranding(matched, type)
> >+{
> >+    return client.messageManager["brand" + type];
> >+}
> 
> And now here you can just pass type (or rename it to |entity|).

Right.
 
> 
> >Index: mozilla/extensions/irc/xul/content/messages.js
> 
> >     client.messageManager = new MessageManager();
> >+    client.messageManager.loadBrands();
> 
> I wonder if it would not be better to make the MM do that all by itself when it
> is created...

We discussed this too :-).


> >-cli_inamesMsg3=You must supply a channel name to use /names from this view.
> 
> Nice one. We don't use it, anyway, do we?

We don't, no.

Comment 17

[:Gijs (he/him)]

Attachment: Patch v0.4

New patch based on comments here and on IRC.

Comment 18

[James Ross]

Comment on attachment 181073 [details] [diff] [review]
Patch v0.4

r=[e-mail]

This looks good. We only took care of the 3 "unreasonable" hard codings, if
other entities are desired or needed, I think seperate bugs or (if it is for
Seamonkey branding) additional patches be done.

Comment 19

[Asa Dotzler [:asa]]

Comment on attachment 181073 [details] [diff] [review]
Patch v0.4

a=asa

Comment 20

[James Ross]

Checked in --> FIXED.

Comment 21

[Felix Miata]

Since this fix, trying to open CZ prefs fails with an unpopulated prefs window
and this in the JS console:

Error: entities has no properties
Source File: chrome://chatzilla/content/lib/js/message-manager.js Line: 80

Comment 22

[James Ross]

Probably the best solution is to bail out at the top of |loadBrands| if
(!this.entities). |replaceEntities| probably needs to check for |me.entities|
before doing the |in| operation (which is liable to fail with something like
'invalid in object' or whatever it is).

Comment 23

[Tobias Fischer]

(In reply to comment #21)
> Since this fix, trying to open CZ prefs fails with an unpopulated prefs window
> and this in the JS console:
> 
> Error: entities has no properties
> Source File: chrome://chatzilla/content/lib/js/message-manager.js Line: 80

Yes, I have noticed this Behaviour too. But upgrading ChatZilla from Suites
Build-In CZ 0.9.67 to CZ 0.9.68a available as extension from
http://ftp.mozilla.org/pub/mozilla.org/extensions/chatzilla/ solves this
Problem. With this ChatZilla Build installed there are no JS-Errors. 

Probably it was needed to upgrade the Suite-Build-In ChatZilla to 0.9.68a or higher.

Comment 24

[:Gijs (he/him)]

Attachment: Patch to fix pref window bustage

Patch to fix prefs

Comment 25

[Robert Ginda]

Comment on attachment 181893 [details] [diff] [review]
Patch to fix pref window bustage

r=rginda

Comment 26

[:Gijs (he/him)]

Comment on attachment 181893 [details] [diff] [review]
Patch to fix pref window bustage

Asking for approval to get this checked in, since a broken ChatZilla pref
window is very bad.

Comment 27

[Asa Dotzler [:asa]]

Comment on attachment 181893 [details] [diff] [review]
Patch to fix pref window bustage

a=asa

Comment 28

[Mike Kaply [:mkaply]]

I checked this fix in for you.

Comment 29

[Felix Miata]

Verified fixed in 2005042805, Linux & OS/2