471508 - Make the toolboxChanged() callback return useful information.

Status: RESOLVED FIXED

(Toolkit :: Toolbars and Toolbar Customization, enhancement)

Description

[Philip Chee]

Attachment: Patch v1.0 enhanced toolboxChanged()

SeaMonkey customizable toolbars have attribute settings in additional to those supported by toolkit (e.g. "labelalign" and "moz-collapsed"). We need to reset these to default when the "reset to defaults" button in toolkit customize toolbar window is called.

This patch makes the toolboxChanged() callback return useful additional information that we and other applications can use:

function toolboxChanged(event, cargo)

event: is a string e.g. "reset" "itemadded" "itemmoved" "itemremoved" "newtoolbar"
cargo: the item associated with the event e.g. a toolbarbutton, or a new toolbar. Can be null.

Comment 1

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #0)
> We need to reset these to default when the "reset to defaults" button in
> toolkit customize toolbar window is called.

Do you have a use planned for the other notifications added in the patch?

Comment 2

[Philip Chee]

> Do you have a use planned for the other notifications added in the patch?

No, but I think if the callback returns information, it should always return consistent information. Do you think that this is over-engineering? If so I'll file an updated patch with only the reset notification returned.

I've also CCed philor to see if Thunderbird would like to use these callback notifications.

And there are certainly extensions that would like to be notified if their toolbarbuttons are inserted into a toolbar. Currently they just listen for the DOMNodeInserted event.

Comment 3

[Philip Chee]

Attachment: Patch v2.0 (Minimal Fix)

As discussed over IRC with gavin, this is a mimimal patch. Only restoreDefaultSet() passes a parameter "reset" to toolboxChanged() and thence to the callback.

Comment 4

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 356515 [details] [diff] [review]
Patch v2.0 (Minimal Fix)

I apparently missed the bugmail for you attaching this minimal patch... I know I've been slow to review patches in the past, but you shouldn't let that prevent you from poking me about it directly before two weeks have elapsed without a response, especially when the patch is this small...

Comment 5

[Philip Chee]

Thanks Gavin. Requesting for 1.9.1 as well since this is needed for SeaMonkey to reset our custom attributes.

Comment 6

[Stefan [:stefanh]]

You need to get it landed on trunk first, then request approval1.9.1? (on the attachment).

Comment 7

[Stefan [:stefanh]]

http://hg.mozilla.org/mozilla-central/rev/008a692ecbca

Comment 8

[Philip Chee]

Comment on attachment 356515 [details] [diff] [review]
Patch v2.0 (Minimal Fix)

Requesting approval for 1.9.1 because SeaMonkey needs this to restore our custom attributes on reset.

1. Has landed on trunk and baked for a few days.
2. Performance impact: Any potential impact will occur only if the reset toolbars to defaults button is pressed which should be minimal.
3. Risk assessment: This is a minimal change/enhancement. All existing consumers (except SeaMonkey) ignore the return value. Regressions are highly unlikely.

Comment 9

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 356515 [details] [diff] [review]
Patch v2.0 (Minimal Fix)

a191=beltzner

Comment 10

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 356515 [details] [diff] [review]
Patch v2.0 (Minimal Fix)

a191=beltzner

Comment 11

[Stefan [:stefanh]]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/39b6e0193053