Bug 940307 - improve Australis' click-detection code so we close the panel at the right time, r=MattN

browser/components/customizableui/src/CustomizableUI.jsm

@@ -1192,19 +1192,92 @@ let CustomizableUIInternal = {
    * part of the menu.
    */
   _isOnInteractiveElement: function(aEvent) {
+    function getMenuPopupForDescendant(aNode) {
+      let lastPopup = null;
+      while (aNode && aNode.parentNode &&
+             aNode.parentNode.localName.startsWith("menu")) {
+        lastPopup = aNode.localName == "menupopup" ? aNode : lastPopup;
+        aNode = aNode.parentNode;
+      }
+      return lastPopup;
+    }
+
     let target = aEvent.originalTarget;
     let panel = this._getPanelForNode(aEvent.currentTarget);
+    // We keep track of:
+    // whether we're in an input container (text field)
     let inInput = false;
+    // whether we're in a popup/context menu
     let inMenu = false;
+    // whether we're in a toolbarbutton/toolbaritem
     let inItem = false;
-    while (!inInput && !inMenu && !inItem && target != panel) {
+    // whether the current menuitem has a valid closemenu attribute
+    let menuitemCloseMenu = "auto";
+    // whether the toolbarbutton/item has a valid closemenu attribute.
+    let closemenu = "auto";
+
+    // While keeping track of that, we go from the original target back up,
+    // to the panel if we have to. We bail as soon as we find an input,
+    // a toolbarbutton/item, or the panel:
+    while (true) {
       let tagName = target.localName;
-      inInput = tagName == "input";
-      inMenu = target.type == "menu";
+      inInput = tagName == "input" || tagName == "textbox";
       inItem = tagName == "toolbaritem" || tagName == "toolbarbutton";
-      target = target.parentNode;
+      let isMenuItem = tagName == "menuitem";
+      inMenu = inMenu || isMenuItem;
+      if (inItem && target.hasAttribute("closemenu")) {
+        let closemenuVal = target.getAttribute("closemenu");
+        closemenu = (closemenuVal == "single" || closemenuVal == "none") ?
+                    closemenuVal : "auto";
+      }
+
+      if (isMenuItem && target.hasAttribute("closemenu")) {
+        let closemenuVal = target.getAttribute("closemenu");
+        menuitemCloseMenu = (closemenuVal == "single" || closemenuVal == "none") ?
+                            closemenuVal : "auto";
+      }
+      // This isn't in the loop condition because we want to break before
+      // changing |target| if any of these conditions are true
+      if (inInput || inItem || target == panel) {
+        break;
+      }
+      // We need specific code for popups: the item on which they were invoked
+      // isn't necessarily in their parentNode chain:
+      if (isMenuItem) {
+        let topmostMenuPopup = getMenuPopupForDescendant(target);
+        target = (topmostMenuPopup && topmostMenuPopup.triggerNode) ||
+                 target.parentNode;
+      } else {
+        target = target.parentNode;
+      }
     }
-    return inMenu || inInput || !inItem;
+    // If the user clicked a menu item...
+    if (inMenu) {
+      // We care if we're in an input also,
+      // or if the user specified closemenu!="auto":
+      if (inInput || menuitemCloseMenu != "auto") {
+        return true;
+      }
+      // Otherwise, we're probably fine to close the panel
+      return false;
+    }
+    // If we're not in a menu, and we *are* in a type="menu" toolbarbutton,
+    // we'll now interact with the menu
+    if (inItem && target.getAttribute("type") == "menu") {
+      return true;
+    }
+    // If we're not in a menu, and we *are* in a type="menu-button" toolbarbutton,
+    // it depends whether we're in the dropmarker or the 'real' button:
+    if (inItem && target.getAttribute("type") == "menu-button") {
+      // 'real' button (which has a single action):
+      if (target.getAttribute("anonid") == "button") {
+        return closemenu != "none";
+      }
+      // otherwise, this is the outer button, and the user will now
+      // interact with the menu:
+      return true;
+    }
+    return inInput || !inItem;
   },
 
   hidePanelForNode: function(aNode) {

browser/components/customizableui/test/browser.ini

@@ -51,6 +51,7 @@ skip-if = os == "linux"
 [browser_938995_indefaultstate_nonremovable.js]
 [browser_940013_registerToolbarNode_calls_registerArea.js]
 [browser_940107_home_button_in_bookmarks_toolbar.js]
+[browser_940307_panel_click_closure_handling.js]
 [browser_940946_removable_from_navbar_customizemode.js]
 [browser_941083_invalidate_wrapper_cache_createWidget.js]
 [browser_942581_unregisterArea_keeps_placements.js]

browser/components/customizableui/test/browser_940307_panel_click_closure_handling.js

@@ -0,0 +1,108 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+"use strict";
+
+let button, menuButton;
+/* Clicking a button should close the panel */
+add_task(function() {
+  button = document.createElement("toolbarbutton");
+  button.id = "browser_940307_button";
+  button.setAttribute("label", "Button");
+  PanelUI.contents.appendChild(button);
+  yield PanelUI.show();
+  let hiddenAgain = promisePanelHidden(window);
+  EventUtils.synthesizeMouseAtCenter(button, {});
+  yield hiddenAgain;
+  button.remove();
+});
+
+/* Clicking a menu button should close the panel, opening the popup shouldn't.  */
+add_task(function() {
+  menuButton = document.createElement("toolbarbutton");
+  menuButton.setAttribute("type", "menu-button");
+  menuButton.id = "browser_940307_menubutton";
+  menuButton.setAttribute("label", "Menu button");
+
+  let menuPopup = document.createElement("menupopup");
+  menuPopup.id = "browser_940307_menupopup";
+
+  let menuItem = document.createElement("menuitem");
+  menuItem.setAttribute("label", "Menu item");
+  menuItem.id = "browser_940307_menuitem";
+
+  menuPopup.appendChild(menuItem);
+  menuButton.appendChild(menuPopup);
+  PanelUI.contents.appendChild(menuButton);
+
+  yield PanelUI.show();
+  let hiddenAgain = promisePanelHidden(window);
+  let innerButton = document.getAnonymousElementByAttribute(menuButton, "anonid", "button");
+  EventUtils.synthesizeMouseAtCenter(innerButton, {});
+  yield hiddenAgain;
+
+  // Now click the dropmarker to show the menu
+  yield PanelUI.show();
+  hiddenAgain = promisePanelHidden(window);
+  let menuShown = promisePanelElementShown(window, menuPopup);
+  let dropmarker = document.getAnonymousElementByAttribute(menuButton, "type", "menu-button");
+  EventUtils.synthesizeMouseAtCenter(dropmarker, {});
+  yield menuShown;
+  // Panel should stay open:
+  ok(isPanelUIOpen(), "Panel should still be open");
+  let menuHidden = promisePanelElementHidden(window, menuPopup);
+  // Then click the menu item to close all the things
+  EventUtils.synthesizeMouseAtCenter(menuItem, {});
+  yield menuHidden;
+  yield hiddenAgain;
+  menuButton.remove();
+});
+
+add_task(function() {
+  let searchbar = document.getElementById("searchbar");
+  gCustomizeMode.addToPanel(searchbar);
+  let placement = CustomizableUI.getPlacementOfWidget("search-container");
+  is(placement.area, CustomizableUI.AREA_PANEL, "Should be in panel");
+  yield PanelUI.show();
+  yield waitForCondition(() => "value" in searchbar && searchbar.value === "");
+
+  searchbar.value = "foo";
+  searchbar.focus();
+  // Reaching into this context menu is pretty evil, but hey... it's a test.
+  let textbox = document.getAnonymousElementByAttribute(searchbar.textbox, "anonid", "textbox-input-box");
+  let contextmenu = document.getAnonymousElementByAttribute(textbox, "anonid", "input-box-contextmenu");
+  let contextMenuShown = promisePanelElementShown(window, contextmenu);
+  EventUtils.synthesizeMouseAtCenter(searchbar, {type: "contextmenu", button: 2});
+  yield contextMenuShown;
+
+  ok(isPanelUIOpen(), "Panel should still be open");
+
+  let selectAll = contextmenu.querySelector("[cmd='cmd_selectAll']");
+  let contextMenuHidden = promisePanelElementHidden(window, contextmenu);
+  EventUtils.synthesizeMouseAtCenter(selectAll, {});
+  yield contextMenuHidden;
+
+  ok(isPanelUIOpen(), "Panel should still be open");
+
+  let hiddenPanelPromise = promisePanelHidden(window);
+  EventUtils.synthesizeKey("VK_ESCAPE", {});
+  yield hiddenPanelPromise;
+  ok(!isPanelUIOpen(), "Panel should no longer be open");
+});
+
+registerCleanupFunction(function() {
+  if (button && button.parentNode) {
+    button.remove();
+  }
+  if (menuButton && menuButton.parentNode) {
+    menuButton.remove();
+  }
+  // Sadly this isn't task.jsm-enabled, so we can't wait for this to happen. But we should
+  // definitely close it here and hope it won't interfere with other tests.
+  // Of course, all the tests are meant to do this themselves, but if they fail...
+  if (isPanelUIOpen()) {
+    PanelUI.hide();
+  }
+});
+

browser/components/customizableui/test/head.js

@@ -250,6 +250,10 @@ function promisePanelElementHidden(win, aPanel) {
   return deferred.promise;
 }
 
+function isPanelUIOpen() {
+  return PanelUI.panel.state == "open" || PanelUI.panel.state == "showing";
+}
+
 function subviewShown(aSubview) {
   let deferred = Promise.defer();
   let win = aSubview.ownerDocument.defaultView;