Make it easier for applications and extensions to overlay the customize toolbar window.

RESOLVED FIXED in mozilla14

Status

()

Toolkit
Toolbars and Toolbar Customization
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Philip Chee, Assigned: Philip Chee)

Tracking

Trunk
mozilla14
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

5 years ago
Thunderbird overlays the customizeToolbar window then uses some ugly javascript to shuffle the UI around. This patch adds some IDs to various elements so that Thunderbird can then use static XUL markup in the overlay to position their UI.

Secondly Thunderbird and Lightning have what they call "inline toolbars/toolboxes". The UX would really suck if users were allowed to add custom toolbars so Thunderbird uses some javascript to hide the Add New Toolbar button and other bits of the UI. They could stop users from adding custom toolbars by not having a toolbarset. However the customize code in a couple of places implicitly depends on the toolbarset existing. We can help Thunderbird and Lighting hide the Add New Toolbar button automatically if the toolbox doesn't have a toolbarset.
(Assignee)

Comment 1

5 years ago
Created attachment 606875 [details] [diff] [review]
Patch v1.0 Proposed Fix.
Attachment #606875 - Flags: review?(dao)
Comment on attachment 606875 [details] [diff] [review]
Patch v1.0 Proposed Fix.

You should probably also add a sanity check for gToolbox.toolbarset in the persistCurrentSets's forEachCustomizableToolbar loop, just in case someone somehow manages to have toolbars with "customindex" set, but no toolbarset.

Making toolbox.xml's appendCustomToolbar fail a little more gracefully when this.toolbarset is null would also be good.
Attachment #606875 - Flags: review?(dao) → review+
(Assignee)

Comment 3

5 years ago
Created attachment 610561 [details] [diff] [review]
Patch v1.1 fix nits. r=gavin

Carrying forward r=gavin

> You should probably also add a sanity check for gToolbox.toolbarset in the
> persistCurrentSets's forEachCustomizableToolbar loop, just in case someone
> somehow manages to have toolbars with "customindex" set, but no toolbarset.
Fixed on checkin.

> Making toolbox.xml's appendCustomToolbar fail a little more gracefully when
> this.toolbarset is null would also be good.
Fixed on checkin.

Pushed to inbound:
http://hg.mozilla.org/integration/mozilla-inbound/rev/a9dfce846606
Attachment #606875 - Attachment is obsolete: true
Attachment #610561 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/a9dfce846606
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla14
You need to log in before you can comment on or make changes to this bug.