From a mail conversation with Hyatt: --- markh --- The current way the world works is this: * People create commands and place them in commandsets. These commandsets nominate "events" that may change their state. * People define nsIControllers to update the comments. * At certain times, these "events" are triggered - either automatically (like "focus") or manually by the user [using window.updateCommands('event')] * On these events we find all commandsets that nominated this event, for each command find its controller, then update the command. The problem Komodo has is that we have around 150 different commands. Many of these are basic editor commands. We have optimized the commandsets heavily, but often find outselves updating 50 or so commands for simple focus switching, for example. All 150 are updated as the product starts. When commandsets are implemented in javascript, the penalty for updating commandsets is significant. I'm wondering if somehow we could move to a model that only updates each command as its state is actually needed. Something like: * Still define commands and commandsets that nominate events. * Still define nsIControllers in the same way. * As each event is triggered, find each command that could be updated, and set it's enabled state to "unknown". * Whenever UI code needs the "enabled" state, instead of looking at the "enabled" tag, it calls a function to determine the state. If the state is "unknown", we then find the controller and update only that command. * We cache the state until it is next set to "unknown". Thus only the first UI element to need the state takes the penalty. It seems to me that this could offer significant performance benefits. -- From Dave -- It seems like what you really want to know is whether or not there's a visible UI widget that is currently observing a command. If there is, your command updater must deal with that command, but if not, that command need not be updated during the command updating phase and could simply be removed from any command updating routines. So a command should automatically know if it has visible UI on it without you havingto specify anything. That would just involve caching a little bit of statewith the command. Then the only remaining step is to augment the key and menu C++ code to call some new routine like UpdateCommand on the controller if it sees this attribute set. File a bug on me. dave (hyatt@netscape.com)

hyatt: any way we can help w/ this patch?

I've been playing with this a little. I have a patch that adds a "hasBroadcastListener()" method to a XULElement. This allows the commandset updater to skip any commands without visible elements. What this means however is that the menu and key handlers must explicitly handle this command. For example, the menu must force a check of the enabled state as the frame is created as menu items can be skipped with the above check. However, I am stuck working out how to make only _one_ command get updated. I can send a NS_XUL_COMMAND_UPDATE event, but this still causes the entire commandset to be refreshed rather than the specific command. Indeed, there appears to be no mechanism for updating a single command at all - ie, will we need to allow <command oncommand="whatever()" onupdatecommand="be_smart()"/> ? So now I must patiently wait for (or politely harass :) Hyatt to offer some more guidance.

Attaching a possible patch for this. Notes: * The menu handling code attempts to find the controller, then sets the "disabled" attribute on the command depending on the result of the controller call. The existing code remains unchanged, which will re-read the "disabled" attribute. It would be faster to ignore the controller's "disabled" attribute alltogether, but I thought I would get some comments first. * The key handling code does _not_ do this - if a controller is found, the "disabled" attribute is ignored. * A new |nsIDOMXULElement| method |boolean hasBroadcastListener(in DOMString attr);| has been added. '*' is supported for the attribute name, which is the only semantic required for this particular optimization. This, the 'attr' param could be dropped all-together - but this seemed a better fit with the existing interface.

->moz1.0

same here... hyatt, how much will this help window open and what's the scope for fixing this? -- thanks! :-)

The patch I submittted seem to work fine - however, it does not really play well with "checked" items. nsIController does not attempt to work with "checked" items, but even still, for ActiveState's Komodo we find that it does not work as well in practice as it should. I would really like to start a dialog with Hyatt on this, but getting his attention is pretty hard at the moment :)

Hyatt, I was looking at the command updater code recently and noticed a targets attribute that isn't currently used anywhere. If I understand correctly, this is a filter to specify that the commands are only updated when certain elements receieve the |events|, as opposed to *all* the elements. This could be a great help in places like msg compose, where I found that we update tons of commands when switching focus between the address and subject fields, even though the body isn't even involved. The other topic worth noting is updating commands that are in disabled toplevel menus. In bug 89624 I have a prototype that makes focusing the msg compose body instanteous after the first time. It reduces the number of commands we have to update when focusing the body from like 50 to 6. We're able to do this in part because we don't have to update commands that have no visible toolbar equivalent and are in top level menus that are disabled. (Of course, this is an ideal case because the msg compose window has few accelerators, and the only condition for their enabled-ness is whether the body is focused).