Closed Bug 364201 Opened 13 years ago Closed 10 years ago

Hidden toolbars should be visible while customizing

Categories

(Toolkit :: Toolbars and Toolbar Customization, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla1.9.3a5

People

(Reporter: philor, Assigned: dao)

References

Details

Attachments

(1 file, 1 obsolete file)

I'm still not quite absolutely convinced this is right, but at least non-empty toolbars should probably be visible while customizing: if you've emptied out the bookmarks toolbar and hidden it, you probably never want to see it again, but if you've created a custom toolbar, put the bookmarks and history sidebar buttons on it, and then hidden it, you won't be able to find them anywhere to drag elsewhere until you remember that hidden toolbar, and unhide it while not customizing.

In case I forget, mconnor's vote was "I think so"
Assignee: philringnalda → nobody
Another possible solution is to show items on hidden toolbars in the palette.  IOW, if your Home button is on the bookmarks toolbar but the bookmarks toolbar is hidden, the Home button should appear in the palette.
Duplicate of this bug: 445069
Duplicate of this bug: 566495
Attached patch Unhide toolbars patch v0.1 (obsolete) — Splinter Review
This patch:
* Preserves hidden=true status in an attribute barhidden and unhides any hidden
toolbars when showing customization dialog;
* Restores relevant hidden status to those toolbars that were unhidden when the
customization dialog is closed.
Assignee: nobody → iann_bugzilla
Status: NEW → ASSIGNED
Attachment #445855 - Flags: review?(philringnalda)
Attachment #445855 - Flags: review?(philringnalda) → review?(gavin.sharp)
Comment on attachment 445855 [details] [diff] [review]
Unhide toolbars patch v0.1

>diff --git a/toolkit/content/customizeToolbar.js b/toolkit/content/customizeToolbar.js
>--- a/toolkit/content/customizeToolbar.js
>+++ b/toolkit/content/customizeToolbar.js
>@@ -88,37 +88,68 @@ function onUnload()
>     finishToolbarCustomization();
> }
> 
> function finishToolbarCustomization()
> {
>   removeToolboxListeners();
>   unwrapToolbarItems();
>   persistCurrentSets();
>+  toggleToolbars(gToolbox, false);
>   gToolbox.customizing = false;
> 
>   notifyParentComplete();
> }
> 
> function initDialog()
> {
>   var mode = gToolbox.getAttribute("mode");
>   document.getElementById("modelist").value = mode;
>   var smallIconsCheckbox = document.getElementById("smallicons");
>   smallIconsCheckbox.checked = gToolbox.getAttribute("iconsize") == "small";
>   if (mode == "text")
>     smallIconsCheckbox.disabled = true;
> 
>+  // Show any hidden toolbars.
>+  toggleToolbars(gToolbox, true);
>+
>   // Build up the palette of other items.
>   buildPalette();
> 
>   // Wrap all the items on the toolbar in toolbarpaletteitems.
>   wrapToolbarItems();
> }
> 
>+function toggleToolbars(aContainer, aShow)
>+{
>+  // Find children of toolbox or toolbarset.
>+  var children = aContainer.children;
>+  for (var i = 0; i < children.length; ++i) {
>+    var child = children[i];
>+    if (child.localName == "toolbar") {

Omit aContainer, use forEachCustomizableToolbar.
Comment on attachment 445855 [details] [diff] [review]
Unhide toolbars patch v0.1

Actually this doesn't work, because we use 'collapsed', not 'hidden'. This also doesn't work for menu toolbars using 'autohide'.
Attachment #445855 - Flags: review?(gavin.sharp) → review-
Attached patch patchSplinter Review
This solves the problems mentioned in the previous comment. If there's no hope that the way toolbars are hidden will be unified, we could also move the app-specific stuff to browser.css and navigator.css respectively.
Attachment #445855 - Attachment is obsolete: true
Attachment #447502 - Flags: review?(enndeakin)
Attachment #447502 - Flags: review?(enndeakin) → review+
Assignee: iann_bugzilla → dao
http://hg.mozilla.org/mozilla-central/rev/a1f383dffcf4
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a5
Blocks: 612588
You need to log in before you can comment on or make changes to this bug.