Closed Bug 471508 Opened 11 years ago Closed 11 years ago

Make the toolboxChanged() callback return useful information.

Categories

(Toolkit :: Toolbars and Toolbar Customization, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla1.9.2a1

People

(Reporter: philip.chee, Assigned: philip.chee)

References

(Blocks 1 open bug)

Details

(Keywords: fixed1.9.1)

Attachments

(1 file, 1 obsolete file)

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.
Attachment #354796 - Flags: review?(gavin.sharp)
(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?
> 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.
Blocks: 471372
As discussed over IRC with gavin, this is a mimimal patch. Only restoreDefaultSet() passes a parameter "reset" to toolboxChanged() and thence to the callback.
Attachment #354796 - Attachment is obsolete: true
Attachment #356515 - Flags: review?(gavin.sharp)
Attachment #354796 - Flags: review?(gavin.sharp)
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...
Attachment #356515 - Flags: review?(gavin.sharp) → review+
Thanks Gavin. Requesting for 1.9.1 as well since this is needed for SeaMonkey to reset our custom attributes.
Status: NEW → ASSIGNED
Flags: wanted1.9.1?
Keywords: checkin-needed
Whiteboard: checkin comment #4
You need to get it landed on trunk first, then request approval1.9.1? (on the attachment).
http://hg.mozilla.org/mozilla-central/rev/008a692ecbca
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: checkin comment #4
Target Milestone: --- → mozilla1.9.2a1
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.
Attachment #356515 - Flags: approval1.9.1?
Attachment #356515 - Flags: approval1.9.1? → approval1.9.1+
Comment on attachment 356515 [details] [diff] [review]
Patch v2.0 (Minimal Fix)

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

a191=beltzner
Keywords: checkin-needed
Whiteboard: [has approval1.9.1][comment 9]
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/39b6e0193053
Flags: wanted1.9.1?
Whiteboard: [has approval1.9.1][comment 9]
You need to log in before you can comment on or make changes to this bug.