364201 - Hidden toolbars should be visible while customizing

Status: RESOLVED FIXED

(Toolkit :: Toolbars and Toolbar Customization, defect)

Description

[Phil Ringnalda (:philor)]

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"

Comment 1

[Jesse Ruderman]

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.

Comment 4

[Ian Neal]

Attachment: Unhide toolbars patch v0.1

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.

Comment 5

[Dão Gottwald [:dao]]

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 6

[Dão Gottwald [:dao]]

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'.

Comment 7

[Dão Gottwald [:dao]]

Attachment: patch

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.

Comment 8

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/a1f383dffcf4