Bug 884402 - allow users to customize out items from Australis' overflow panel, r=mconley
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Wed, 12 Feb 2014 17:32:23 +0000
changeset 169394 97b384579f34a25a899f0b507b7ebea2ada85b75
parent 169393 55b19bbcbdc7c0ed592966620d021dcce3d1aac8
child 169395 15f403785e0160dc1d932f8a6a346b32f0bc8c13
push id270
push userpvanderbeken@mozilla.com
push dateThu, 06 Mar 2014 09:24:21 +0000
reviewersmconley
bugs884402
milestone30.0a1
Bug 884402 - allow users to customize out items from Australis' overflow panel, r=mconley
browser/base/content/browser.js
browser/base/content/browser.xul
browser/components/customizableui/content/panelUI.inc.xul
browser/components/customizableui/src/CustomizableUI.jsm
browser/components/customizableui/src/CustomizeMode.jsm
browser/components/customizableui/test/browser.ini
browser/components/customizableui/test/browser_880164_customization_context_menus.js
browser/components/customizableui/test/browser_884402_customize_from_overflow.js
browser/components/customizableui/test/head.js
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -4233,16 +4233,17 @@ function onViewToolbarsPopupShowing(aEve
   let toolbarItem = popup.triggerNode;
 
   if (toolbarItem && toolbarItem.localName == "toolbarpaletteitem") {
     toolbarItem = toolbarItem.firstChild;
   } else {
     while (toolbarItem && toolbarItem.parentNode) {
       let parent = toolbarItem.parentNode;
       if ((parent.classList && parent.classList.contains("customization-target")) ||
+          parent.getAttribute("overflowfortoolbar") || // Needs to work in the overflow list as well.
           parent.localName == "toolbarpaletteitem" ||
           parent.localName == "toolbar")
         break;
       toolbarItem = parent;
     }
   }
 
   // Right-clicking on an empty part of the tabstrip will exit
--- a/browser/base/content/browser.xul
+++ b/browser/base/content/browser.xul
@@ -264,17 +264,24 @@
                 label="&customizeMenu.moveToPanel.label;"
                 class="customize-context-moveToPanel"/>
       <menuitem oncommand="gCustomizeMode.removeFromArea(document.popupNode)"
                 accesskey="&customizeMenu.removeFromToolbar.accesskey;"
                 label="&customizeMenu.removeFromToolbar.label;"
                 class="customize-context-removeFromToolbar"/>
       <menuseparator/>
       <menuseparator id="viewToolbarsMenuSeparator"/>
-      <menuitem command="cmd_CustomizeToolbars"
+      <!-- XXXgijs: we're using oncommand handler here to avoid the event being
+                    redirected to the command element, thus preventing
+                    listeners on the menupopup or further up the tree from
+                    seeing the command event pass by. The observes attribute is
+                    here so that the menuitem is still disabled and re-enabled
+                    correctly. -->
+      <menuitem oncommand="BrowserCustomizeToolbar()"
+                observes="cmd_CustomizeToolbars"
                 class="viewCustomizeToolbar"
                 label="&viewCustomizeToolbar.label;"
                 accesskey="&viewCustomizeToolbar.accesskey;"/>
     </menupopup>
 
     <menupopup id="blockedPopupOptions"
                onpopupshowing="gPopupBlockerObserver.fillPopupList(event);"
                onpopuphiding="gPopupBlockerObserver.onPopupHiding(event);">
--- a/browser/components/customizableui/content/panelUI.inc.xul
+++ b/browser/components/customizableui/content/panelUI.inc.xul
@@ -217,13 +217,15 @@
     </svg:defs>
   </svg:svg>
 </panel>
 
 <panel id="widget-overflow"
        role="group"
        type="arrow"
        level="top"
+       context="toolbar-context-menu"
        hidden="true">
   <vbox id="widget-overflow-scroller">
-    <vbox id="widget-overflow-list" class="widget-overflow-list"/>
+    <vbox id="widget-overflow-list" class="widget-overflow-list"
+          overflowfortoolbar="nav-bar"/>
   </vbox>
 </panel>
--- a/browser/components/customizableui/src/CustomizableUI.jsm
+++ b/browser/components/customizableui/src/CustomizableUI.jsm
@@ -3417,17 +3417,21 @@ OverflowableToolbar.prototype = {
   },
 
   handleEvent: function(aEvent) {
     switch(aEvent.type) {
       case "resize":
         this._onResize(aEvent);
         break;
       case "command":
-        this._onClickChevron(aEvent);
+        if (aEvent.target == this._chevron) {
+          this._onClickChevron(aEvent);
+        } else {
+          this._panel.hidePopup();
+        }
         break;
       case "popuphiding":
         this._onPanelHiding(aEvent);
         break;
       case "customizationstarting":
         this._disable();
         break;
       case "aftercustomization":
@@ -3452,29 +3456,34 @@ OverflowableToolbar.prototype = {
       this.removeEventListener("popupshown", onPopupShown);
       deferred.resolve();
     });
 
     return deferred.promise;
   },
 
   _onClickChevron: function(aEvent) {
-    if (this._chevron.open)
+    if (this._chevron.open) {
       this._panel.hidePopup();
-    else {
+    } else {
       let doc = aEvent.target.ownerDocument;
       this._panel.hidden = false;
+      let contextMenu = doc.getElementById(this._panel.getAttribute("context"));
+      gELS.addSystemEventListener(contextMenu, 'command', this, true);
       let anchor = doc.getAnonymousElementByAttribute(this._chevron, "class", "toolbarbutton-icon");
       this._panel.openPopup(anchor || this._chevron, "bottomcenter topright");
     }
     this._chevron.open = !this._chevron.open;
   },
 
   _onPanelHiding: function(aEvent) {
     this._chevron.open = false;
+    let doc = aEvent.target.ownerDocument;
+    let contextMenu = doc.getElementById(this._panel.getAttribute("context"));
+    gELS.removeSystemEventListener(contextMenu, 'command', this, true);
   },
 
   onOverflow: function(aEvent) {
     if (!this._enabled ||
         (aEvent && aEvent.target != this._toolbar.customizationTarget))
       return;
 
     let child = this._target.lastChild;
--- a/browser/components/customizableui/src/CustomizeMode.jsm
+++ b/browser/components/customizableui/src/CustomizeMode.jsm
@@ -492,22 +492,44 @@ CustomizeMode.prototype = {
 
   dispatchToolboxEvent: function(aEventType, aDetails={}) {
     let evt = this.document.createEvent("CustomEvent");
     evt.initCustomEvent(aEventType, true, true, {changed: this._changed});
     let result = this.window.gNavToolbox.dispatchEvent(evt);
   },
 
   _getCustomizableChildForNode: function(aNode) {
-    let area = this._getCustomizableParent(aNode);
-    area = area.customizationTarget || area;
-    while (aNode && aNode.parentNode != area) {
-      aNode = aNode.parentNode;
+    // NB: adjusted from _getCustomizableParent to keep that method fast
+    // (it's used during drags), and avoid multiple DOM loops
+    let areas = CustomizableUI.areas;
+    // Caching this length is important because otherwise we'll also iterate
+    // over items we add to the end from within the loop.
+    let numberOfAreas = areas.length;
+    for (let i = 0; i < numberOfAreas; i++) {
+      let area = areas[i];
+      let areaNode = aNode.ownerDocument.getElementById(area);
+      let customizationTarget = areaNode && areaNode.customizationTarget;
+      if (customizationTarget && customizationTarget != areaNode) {
+        areas.push(customizationTarget.id);
+      }
+      let overflowTarget = areaNode.getAttribute("overflowtarget");
+      if (overflowTarget) {
+        areas.push(overflowTarget);
+      }
     }
-    return aNode;
+    areas.push(kPaletteId);
+
+    while (aNode && aNode.parentNode) {
+      let parent = aNode.parentNode;
+      if (areas.indexOf(parent.id) != -1) {
+        return aNode;
+      }
+      aNode = parent;
+    }
+    return null;
   },
 
   addToToolbar: function(aNode) {
     aNode = this._getCustomizableChildForNode(aNode);
     if (aNode.localName == "toolbarpaletteitem" && aNode.firstChild) {
       aNode = aNode.firstChild;
     }
     CustomizableUI.addWidgetToArea(aNode.id, CustomizableUI.AREA_NAVBAR);
--- a/browser/components/customizableui/test/browser.ini
+++ b/browser/components/customizableui/test/browser.ini
@@ -6,16 +6,18 @@ support-files =
 [browser_876926_customize_mode_wrapping.js]
 [browser_876944_customize_mode_create_destroy.js]
 [browser_877006_missing_view.js]
 [browser_877178_unregisterArea.js]
 [browser_877447_skip_missing_ids.js]
 [browser_878452_drag_to_panel.js]
 [browser_880164_customization_context_menus.js]
 [browser_880382_drag_wide_widgets_in_panel.js]
+[browser_884402_customize_from_overflow.js]
+skip-if = os == "linux"
 [browser_885052_customize_mode_observers_disabed.js]
 # Bug 951403 - Disabled on OSX for frequent failures
 skip-if = os == "mac"
 
 [browser_885530_showInPrivateBrowsing.js]
 [browser_886323_buildArea_removable_nodes.js]
 [browser_887438_currentset_shim.js]
 [browser_888817_currentset_updating.js]
--- a/browser/components/customizableui/test/browser_880164_customization_context_menus.js
+++ b/browser/components/customizableui/test/browser_880164_customization_context_menus.js
@@ -300,63 +300,8 @@ add_task(function() {
   contextMenu.hidePopup();
   yield hiddenContextPromise;
 
   let hiddenPromise = promisePanelHidden(window);
   PanelUI.hide();
   yield hiddenPromise;
 });
 
-function contextMenuShown(aContextMenu) {
-  let deferred = Promise.defer();
-  let win = aContextMenu.ownerDocument.defaultView;
-  let timeoutId = win.setTimeout(() => {
-    deferred.reject("Context menu (" + aContextMenu.id + ") did not show within 20 seconds.");
-  }, 20000);
-  function onPopupShown(e) {
-    aContextMenu.removeEventListener("popupshown", onPopupShown);
-    win.clearTimeout(timeoutId);
-    deferred.resolve();
-  };
-  aContextMenu.addEventListener("popupshown", onPopupShown);
-  return deferred.promise;
-}
-
-function contextMenuHidden(aContextMenu) {
-  let deferred = Promise.defer();
-  let win = aContextMenu.ownerDocument.defaultView;
-  let timeoutId = win.setTimeout(() => {
-    deferred.reject("Context menu (" + aContextMenu.id + ") did not hide within 20 seconds.");
-  }, 20000);
-  function onPopupHidden(e) {
-    win.clearTimeout(timeoutId);
-    aContextMenu.removeEventListener("popuphidden", onPopupHidden);
-    deferred.resolve();
-  };
-  aContextMenu.addEventListener("popuphidden", onPopupHidden);
-  return deferred.promise;
-}
-
-// This is a simpler version of the context menu check that
-// exists in contextmenu_common.js.
-function checkContextMenu(aContextMenu, aExpectedEntries, aWindow=window) {
-  let childNodes = aContextMenu.childNodes;
-  for (let i = 0; i < childNodes.length; i++) {
-    let menuitem = childNodes[i];
-    try {
-      if (aExpectedEntries[i][0] == "---") {
-        is(menuitem.localName, "menuseparator", "menuseparator expected");
-        continue;
-      }
-
-      let selector = aExpectedEntries[i][0];
-      ok(menuitem.mozMatchesSelector(selector), "menuitem should match " + selector + " selector");
-      let commandValue = menuitem.getAttribute("command");
-      let relatedCommand = commandValue ? aWindow.document.getElementById(commandValue) : null;
-      let menuItemDisabled = relatedCommand ?
-                               relatedCommand.getAttribute("disabled") == "true" :
-                               menuitem.getAttribute("disabled") == "true";
-      is(menuItemDisabled, !aExpectedEntries[i][1], "disabled state for " + selector);
-    } catch (e) {
-      ok(false, "Exception when checking context menu: " + e);
-    }
-  }
-}
new file mode 100644
--- /dev/null
+++ b/browser/components/customizableui/test/browser_884402_customize_from_overflow.js
@@ -0,0 +1,78 @@
+"use strict";
+
+let overflowPanel = document.getElementById("widget-overflow");
+
+const isOSX = (Services.appinfo.OS === "Darwin");
+
+let originalWindowWidth;
+registerCleanupFunction(function() {
+  window.resizeTo(originalWindowWidth, window.outerHeight);
+});
+
+// Right-click on an item within the overflow panel should
+// show a context menu with options to move it.
+add_task(function() {
+  originalWindowWidth = window.outerWidth;
+  let navbar = document.getElementById(CustomizableUI.AREA_NAVBAR);
+  ok(!navbar.hasAttribute("overflowing"), "Should start with a non-overflowing toolbar.");
+  let oldChildCount = navbar.customizationTarget.childElementCount;
+  window.resizeTo(400, window.outerHeight);
+
+  yield waitForCondition(() => navbar.hasAttribute("overflowing"));
+  ok(navbar.hasAttribute("overflowing"), "Should have an overflowing toolbar.");
+
+  let chevron = document.getElementById("nav-bar-overflow-button");
+  let shownPanelPromise = promisePanelElementShown(window, overflowPanel);
+  chevron.click();
+  yield shownPanelPromise;
+
+  let contextMenu = document.getElementById("toolbar-context-menu");
+  let shownContextPromise = contextMenuShown(contextMenu);
+  let homeButton = document.getElementById("home-button");
+  ok(homeButton, "home-button was found");
+  ok(homeButton.classList.contains("overflowedItem"), "Home button is overflowing");
+  EventUtils.synthesizeMouse(homeButton, 2, 2, {type: "contextmenu", button: 2});
+  yield shownContextPromise;
+
+  is(overflowPanel.state, "open", "The widget overflow panel should still be open.");
+
+  let expectedEntries = [
+    [".customize-context-moveToPanel", true],
+    [".customize-context-removeFromToolbar", true],
+    ["---"]
+  ];
+  if (!isOSX) {
+    expectedEntries.push(["#toggle_toolbar-menubar", true]);
+  }
+  expectedEntries.push(
+    ["#toggle_PersonalToolbar", true],
+    ["---"],
+    [".viewCustomizeToolbar", true]
+  );
+  checkContextMenu(contextMenu, expectedEntries);
+
+  let hiddenContextPromise = contextMenuHidden(contextMenu);
+  let hiddenPromise = promisePanelElementHidden(window, overflowPanel);
+  let moveToPanel = contextMenu.querySelector(".customize-context-moveToPanel");
+  if (moveToPanel) {
+    moveToPanel.click();
+  }
+  contextMenu.hidePopup();
+  yield hiddenContextPromise;
+  yield hiddenPromise;
+
+  let homeButtonPlacement = CustomizableUI.getPlacementOfWidget("home-button");
+  ok(homeButtonPlacement, "Home button should still have a placement");
+  is(homeButtonPlacement && homeButtonPlacement.area, "PanelUI-contents", "Home button should be in the panel now");
+  CustomizableUI.reset();
+
+  // In some cases, it can take a tick for the navbar to overflow again. Wait for it:
+  yield waitForCondition(() => navbar.hasAttribute("overflowing"));
+  ok(navbar.hasAttribute("overflowing"), "Should have an overflowing toolbar.");
+
+  let homeButtonPlacement = CustomizableUI.getPlacementOfWidget("home-button");
+  ok(homeButtonPlacement, "Home button should still have a placement");
+  is(homeButtonPlacement && homeButtonPlacement.area, "nav-bar", "Home button should be back in the navbar now");
+
+  ok(homeButton.classList.contains("overflowedItem"), "Home button should still be overflowed");
+});
--- a/browser/components/customizableui/test/head.js
+++ b/browser/components/customizableui/test/head.js
@@ -371,8 +371,65 @@ function promiseTabHistoryNavigation(aDi
     }
   }
   gBrowser.addEventListener("pageshow", listener, true);
 
   content.history.go(aDirection);
 
   return deferred.promise;
 }
+
+function contextMenuShown(aContextMenu) {
+  let deferred = Promise.defer();
+  let win = aContextMenu.ownerDocument.defaultView;
+  let timeoutId = win.setTimeout(() => {
+    deferred.reject("Context menu (" + aContextMenu.id + ") did not show within 20 seconds.");
+  }, 20000);
+  function onPopupShown(e) {
+    aContextMenu.removeEventListener("popupshown", onPopupShown);
+    win.clearTimeout(timeoutId);
+    deferred.resolve();
+  };
+  aContextMenu.addEventListener("popupshown", onPopupShown);
+  return deferred.promise;
+}
+
+function contextMenuHidden(aContextMenu) {
+  let deferred = Promise.defer();
+  let win = aContextMenu.ownerDocument.defaultView;
+  let timeoutId = win.setTimeout(() => {
+    deferred.reject("Context menu (" + aContextMenu.id + ") did not hide within 20 seconds.");
+  }, 20000);
+  function onPopupHidden(e) {
+    win.clearTimeout(timeoutId);
+    aContextMenu.removeEventListener("popuphidden", onPopupHidden);
+    deferred.resolve();
+  };
+  aContextMenu.addEventListener("popuphidden", onPopupHidden);
+  return deferred.promise;
+}
+
+
+// This is a simpler version of the context menu check that
+// exists in contextmenu_common.js.
+function checkContextMenu(aContextMenu, aExpectedEntries, aWindow=window) {
+  let childNodes = aContextMenu.childNodes;
+  for (let i = 0; i < childNodes.length; i++) {
+    let menuitem = childNodes[i];
+    try {
+      if (aExpectedEntries[i][0] == "---") {
+        is(menuitem.localName, "menuseparator", "menuseparator expected");
+        continue;
+      }
+
+      let selector = aExpectedEntries[i][0];
+      ok(menuitem.mozMatchesSelector(selector), "menuitem should match " + selector + " selector");
+      let commandValue = menuitem.getAttribute("command");
+      let relatedCommand = commandValue ? aWindow.document.getElementById(commandValue) : null;
+      let menuItemDisabled = relatedCommand ?
+                               relatedCommand.getAttribute("disabled") == "true" :
+                               menuitem.getAttribute("disabled") == "true";
+      is(menuItemDisabled, !aExpectedEntries[i][1], "disabled state for " + selector);
+    } catch (e) {
+      ok(false, "Exception when checking context menu: " + e);
+    }
+  }
+}