342988 - Allow plugins to rebuild menu(s) after changing client.menuSpecs

Status: RESOLVED FIXED

(Other Applications :: ChatZilla, defect)

Description

[James Ross]

To allow for plugins to modify the menus when they load/unload, we need to allow them to trigger a rebuild of all the menus (on all windows once bug 76175 is done). I'm thinking:

  client.menuManager.updateMenus();

which can be called after any change to client.menuSpecs to sync things (only one way). We should get plugins into the habit of calling this as soon as possible, so that when we get to the new ChatZilla service code and multiple windows stuff, it actually works for most existing plugins.

Comment 1

[James Ross]

After some discussion in #chatzilla, it was decided that allowing an array of menuSpec keys as an optional parameter is wise. This would allow the plugin to only trigger the updating on menus it knew it had changed (including ones it has added and/or removed).

Comment 2

[:Gijs (he/him)]

Attachment: Patch

Taking this for a bit; I think this patch will do, it sufficed in my testing, but given that I wrote the 'fun' bits while in a conference (being mildly distracted at times) it might need some care.

Comment 3

[:Gijs (he/him)]

Argh. I want a 'keep assignee CC-ed' field. Seriously.

Comment 4

[:Gijs (he/him)]

Attachment: [checked in w/ nits] Better patch

Patch with a different API, in a different place.

Comment 5

[James Ross]

Comment on attachment 243909 [details] [diff] [review]
[checked in w/ nits] Better patch

>+    if (!menu.parentNode)
>+        parentNode.insertBefore(menu, beforeNode);
>+

I'm going to trust you that the code works, but can we at least have a comment for why this moved?

r=silver with that commented.

Comment 6

[:Gijs (he/him)]

Fixed with additional comment:

Checking in mozilla/extensions/irc/js/lib/menu-manager.js;
/cvsroot/mozilla/extensions/irc/js/lib/menu-manager.js,v  <--  menu-manager.js
new revision: 1.9; previous revision: 1.8
done
Checking in mozilla/extensions/irc/xul/content/static.js;
/cvsroot/mozilla/extensions/irc/xul/content/static.js,v  <--  static.js
new revision: 1.213; previous revision: 1.212
done

Comment 7

[:Gijs (he/him)]

So, there's a small problem which Toast and I stumbled upon when writing a new version of ChattyTunes:

1. Write a plugin that adds a new menuspec
2. run client.updateMenus("mainmenu:yourmenu") in your enable code.
3. Autoload your plugin
4. Your menu is duplicated.

This is, afaict, because right now createMenus doesn't really give a shit if your menu is there already, it'll just re-add all the items! Oops.

I'm curious whether it will live if one just replaces calls to createMenus with updateMenus, but I can try that later.

Comment 8

[James Ross]

The original plan was that calls to client.updateMenus would do nothing if they were called before initMenus() or whatever it is that creates the initial set. That's what needs to happen here.

Comment 9

[:Gijs (he/him)]

Attachment: [checked in] Patch to fix it not to do anything on startup

Well, yeah. That was fairly straightforward. Prevented any problems from showing on ChattyTunes on Fx 2.0 w/ CZ trunk + this patch.

Comment 10

[:Gijs (he/him)]

Checking in mozilla/extensions/irc/xul/content/static.js;
/cvsroot/mozilla/extensions/irc/xul/content/static.js,v  <--  static.js
new revision: 1.220; previous revision: 1.219
done