Bug 260611 - Introduce pref browser.bookmarks.openInTabClosesMenu to optionally leave menu open for single bookmark menuitem clicks that open in new tab. r?mak draft
authorTawny Hoover <stayopenmenu@gmail.com>
Mon, 11 Sep 2017 12:12:21 -0400
changeset 663998 3392bb586f0fa7f409ecd084deb07ee0c337d523
parent 663729 9873269746359dbcccbd5b44f0e82c0225289945
child 731340 4bb85d9520477e4dd35c2bc153b3ba70184e4bc4
push id79586
push userbmo:standard8@mozilla.com
push dateWed, 13 Sep 2017 15:56:39 +0000
reviewersmak
bugs260611
milestone57.0a1
Bug 260611 - Introduce pref browser.bookmarks.openInTabClosesMenu to optionally leave menu open for single bookmark menuitem clicks that open in new tab. r?mak MozReview-Commit-ID: HreM02pdzWP
browser/app/profile/firefox.js
browser/base/content/browser-menubar.inc
browser/base/content/browser-places.js
browser/base/content/browser.xul
browser/components/places/PlacesUIUtils.jsm
browser/components/places/content/placesOverlay.xul
browser/components/places/tests/browser/browser.ini
browser/components/places/tests/browser/browser_stayopenmenu.js
--- a/browser/app/profile/firefox.js
+++ b/browser/app/profile/firefox.js
@@ -488,16 +488,19 @@ pref("browser.bookmarks.autoExportHTML",
 // The maximum number of daily bookmark backups to
 // keep in {PROFILEDIR}/bookmarkbackups. Special values:
 // -1: unlimited
 //  0: no backups created (and deletes all existing backups)
 pref("browser.bookmarks.max_backups",             15);
 
 pref("browser.bookmarks.showRecentlyBookmarked",  true);
 
+// Whether menu should close after Ctrl-click, middle-click, etc.
+pref("browser.bookmarks.openInTabClosesMenu", true);
+
 // Scripts & Windows prefs
 pref("dom.disable_open_during_load",              true);
 pref("javascript.options.showInConsole",          true);
 #ifdef DEBUG
 pref("general.warnOnAboutConfig",                 false);
 #endif
 
 // This is the pref to control the location bar, change this to true to
--- a/browser/base/content/browser-menubar.inc
+++ b/browser/base/content/browser-menubar.inc
@@ -380,16 +380,17 @@
         ondragover="PlacesMenuDNDHandler.onDragOver(event);"
         ondrop="PlacesMenuDNDHandler.onDrop(event);">
     <menupopup id="bookmarksMenuPopup"
 #ifndef XP_MACOSX
                placespopup="true"
 #endif
                context="placesContext"
                openInTabs="children"
+               onmouseup="BookmarksEventHandler.onMouseUp(event);"
                oncommand="BookmarksEventHandler.onCommand(event);"
                onclick="BookmarksEventHandler.onClick(event, this.parentNode._placesView);"
                onpopupshowing="BookmarkingUI.onMainMenuPopupShowing(event);
                                if (!this.parentNode._placesView)
                                  new PlacesMenu(event, 'place:folder=BOOKMARKS_MENU');"
                tooltip="bhTooltip" popupsinherittooltip="true">
       <menuitem id="bookmarksShowAll"
                 label="&showAllBookmarks2.label;"
--- a/browser/base/content/browser-places.js
+++ b/browser/base/content/browser-places.js
@@ -888,40 +888,61 @@ var BookmarksEventHandler = {
    * Left-click is handled in the onCommand function.
    * When items are middle-clicked (or clicked with modifier), open in tabs.
    * If the click came through a menu, close the menu.
    * @param aEvent
    *        DOMEvent for the click
    * @param aView
    *        The places view which aEvent should be associated with.
    */
+
+  onMouseUp(aEvent) {
+    // Handles left-click with modifier if not browser.bookmarks.openInTabClosesMenu.
+    if (aEvent.button != 0 || PlacesUIUtils.openInTabClosesMenu)
+      return;
+    let target = aEvent.originalTarget;
+    if (target.tagName != "menuitem")
+      return;
+    let modifKey = AppConstants.platform === "macosx" ? aEvent.metaKey
+                                                      : aEvent.ctrlKey;
+    // Don't keep menu open for 'Open all in Tabs'.
+    if (modifKey && !target.classList.contains("openintabs-menuitem")) {
+      target.setAttribute("closemenu", "none");
+    }
+  },
+
   onClick: function BEH_onClick(aEvent, aView) {
     // Only handle middle-click or left-click with modifiers.
     let modifKey;
     if (AppConstants.platform == "macosx") {
       modifKey = aEvent.metaKey || aEvent.shiftKey;
     } else {
       modifKey = aEvent.ctrlKey || aEvent.shiftKey;
     }
 
     if (aEvent.button == 2 || (aEvent.button == 0 && !modifKey))
       return;
 
     var target = aEvent.originalTarget;
-    // If this event bubbled up from a menu or menuitem, close the menus.
-    // Do this before opening tabs, to avoid hiding the open tabs confirm-dialog.
-    if (target.localName == "menu" || target.localName == "menuitem") {
-      for (let node = target.parentNode; node; node = node.parentNode) {
-        if (node.localName == "menupopup")
-          node.hidePopup();
-        else if (node.localName != "menu" &&
-                 node.localName != "hbox" &&
-                 node.localName != "vbox" )
-          break;
-      }
+    // If this event bubbled up from a menu or menuitem,
+    // close the menus if browser.bookmarks.openInTabClosesMenu.
+    if ((PlacesUIUtils.openInTabClosesMenu && target.tagName == "menuitem") ||
+        target.tagName == "menu" ||
+        target.classList.contains("openintabs-menuitem")) {
+      closeMenus(aEvent.target);
+    }
+    // Command already precesssed so remove any closemenu attr set in onMouseUp.
+    if (aEvent.button == 0 &&
+        target.tagName == "menuitem" &&
+        target.getAttribute("closemenu") == "none") {
+      // On Mac we need to extend when we remove the flag, to avoid any pre-close
+      // animations.
+      setTimeout(() => {
+        target.removeAttribute("closemenu");
+      }, 500);
     }
 
     if (target._placesNode && PlacesUtils.nodeIsContainer(target._placesNode)) {
       // Don't open the root folder in tabs when the empty area on the toolbar
       // is middle-clicked or when a non-bookmark item except for Open in Tabs)
       // in a bookmarks menupopup is middle-clicked.
       if (target.localName == "menu" || target.localName == "toolbarbutton")
         PlacesUIUtils.openContainerNodeInTabs(target._placesNode, aEvent, aView);
--- a/browser/base/content/browser.xul
+++ b/browser/base/content/browser.xul
@@ -1010,16 +1010,17 @@
         <toolbarbutton id="bookmarks-toolbar-button"
                        class="toolbarbutton-1"
                        flex="1"
                        label="&bookmarksToolbarItem.label;"
                        oncommand="PlacesToolbarHelper.onPlaceholderCommand();"/>
         <hbox flex="1"
               id="PlacesToolbar"
               context="placesContext"
+              onmouseup="BookmarksEventHandler.onMouseUp(event);"
               onclick="BookmarksEventHandler.onClick(event, this._placesView);"
               oncommand="BookmarksEventHandler.onCommand(event);"
               tooltip="bhTooltip"
               popupsinherittooltip="true">
           <hbox flex="1">
             <hbox id="PlacesToolbarDropIndicatorHolder" align="center" collapsed="true">
               <image id="PlacesToolbarDropIndicator"
                      mousethrough="always"
@@ -1087,16 +1088,17 @@
                      oncommand="BookmarkingUI.onCommand(event);">
         <observes element="bookmarkThisPageBroadcaster" attribute="starred"/>
         <observes element="bookmarkThisPageBroadcaster" attribute="buttontooltiptext"/>
         <menupopup id="BMB_bookmarksPopup"
                    class="cui-widget-panel cui-widget-panelview cui-widget-panelWithFooter PanelUI-subView"
                    placespopup="true"
                    context="placesContext"
                    openInTabs="children"
+                   onmouseup="BookmarksEventHandler.onMouseUp(event);"
                    oncommand="BookmarksEventHandler.onCommand(event);"
                    onclick="BookmarksEventHandler.onClick(event, this.parentNode._placesView);"
                    onpopupshowing="BookmarkingUI.onPopupShowing(event);
                                    BookmarkingUI.attachPlacesView(event, this);"
                    tooltip="bhTooltip" popupsinherittooltip="true">
           <menuitem id="BMB_viewBookmarksSidebar"
                     class="subviewbutton"
                     label="&viewBookmarksSidebar2.label;"
--- a/browser/components/places/PlacesUIUtils.jsm
+++ b/browser/components/places/PlacesUIUtils.jsm
@@ -1571,16 +1571,18 @@ XPCOMUtils.defineLazyGetter(PlacesUIUtil
   } catch (ex) { }
   return false;
 });
 
 XPCOMUtils.defineLazyPreferenceGetter(PlacesUIUtils, "loadBookmarksInBackground",
                                       PREF_LOAD_BOOKMARKS_IN_BACKGROUND, false);
 XPCOMUtils.defineLazyPreferenceGetter(PlacesUIUtils, "loadBookmarksInTabs",
                                       PREF_LOAD_BOOKMARKS_IN_TABS, false);
+XPCOMUtils.defineLazyPreferenceGetter(PlacesUIUtils, "openInTabClosesMenu",
+  "browser.bookmarks.openInTabClosesMenu", false);
 
 XPCOMUtils.defineLazyServiceGetter(this, "URIFixup",
                                    "@mozilla.org/docshell/urifixup;1",
                                    "nsIURIFixup");
 
 XPCOMUtils.defineLazyGetter(this, "bundle", function() {
   const PLACES_STRING_BUNDLE_URI =
     "chrome://browser/locale/places/places.properties";
--- a/browser/components/places/content/placesOverlay.xul
+++ b/browser/components/places/content/placesOverlay.xul
@@ -89,16 +89,20 @@
     <command id="placesCmd_paste"
              oncommand="goDoPlacesCommand('placesCmd_paste');"/>
     <command id="placesCmd_delete"
              oncommand="goDoPlacesCommand('placesCmd_delete');"/>
   </commandset>
 
   <menupopup id="placesContext"
              onpopupshowing="this._view = PlacesUIUtils.getViewForNode(document.popupNode);
+                             if (!PlacesUIUtils.openInTabClosesMenu) {
+                               document.getElementById ('placesContext_open:newtab')
+                               .setAttribute('closemenu', 'single');
+                             }
                              return this._view.buildContextMenu(this);"
              onpopuphiding="this._view.destroyContextMenu();">
     <menuitem id="placesContext_open"
               command="placesCmd_open"
               label="&cmd.open.label;"
               accesskey="&cmd.open.accesskey;"
               default="true"
               selectiontype="single"
--- a/browser/components/places/tests/browser/browser.ini
+++ b/browser/components/places/tests/browser/browser.ini
@@ -52,16 +52,17 @@ subsuite = clipboard
 [browser_library_search.js]
 [browser_library_views_liveupdate.js]
 [browser_markPageAsFollowedLink.js]
 [browser_paste_into_tags.js]
 subsuite = clipboard
 [browser_sidebarpanels_click.js]
 skip-if = true # temporarily disabled for breaking the treeview - bug 658744
 [browser_sort_in_library.js]
+[browser_stayopenmenu.js]
 [browser_toolbar_drop_text.js]
 [browser_toolbar_overflow.js]
 [browser_toolbarbutton_menu_context.js]
 [browser_views_iconsupdate.js]
 support-files =
   favicon-normal16.png
 [browser_views_liveupdate.js]
 [browser_bookmark_all_tabs.js]
new file mode 100644
--- /dev/null
+++ b/browser/components/places/tests/browser/browser_stayopenmenu.js
@@ -0,0 +1,206 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// Menus should stay open (if pref is set) after ctrl-click, middle-click,
+// and contextmenu's "Open in a new tab" click.
+
+async function locateBookmarkAndTestCtrlClick(menupopup) {
+  let menuitem = null;
+  for (let node of menupopup.childNodes) {
+    if (node.label == "Test1") {
+      menuitem = node;
+      let promiseTabOpened = BrowserTestUtils.waitForNewTab(gBrowser, null);
+      EventUtils.synthesizeMouseAtCenter(menuitem,
+        AppConstants.platform === "macosx" ? {metaKey: true} : {ctrlKey: true});
+      let newTab = await promiseTabOpened;
+      ok(true, "Bookmark ctrl-click opened new tab.");
+      await BrowserTestUtils.removeTab(newTab);
+      break;
+    }
+  }
+  return menuitem;
+}
+
+async function testContextmenu(menuitem) {
+  let doc = menuitem.ownerDocument;
+  let cm = doc.getElementById("placesContext");
+  let promiseEvent = BrowserTestUtils.waitForEvent(cm, "popupshown");
+  EventUtils.synthesizeMouseAtCenter(menuitem, {type: "contextmenu", button: 2});
+  await promiseEvent;
+  let promiseTabOpened = BrowserTestUtils.waitForNewTab(gBrowser, null);
+  EventUtils.synthesizeKey("KEY_ArrowDown", {code: "ArrowDown"});
+  BrowserTestUtils.waitForEvent(menuitem, "DOMMenuItemActive");
+  EventUtils.synthesizeKey("KEY_ArrowDown", {code: "ArrowDown"});
+  BrowserTestUtils.waitForEvent(menuitem, "DOMMenuItemActive");
+  EventUtils.sendKey("return");
+  let newTab = await promiseTabOpened;
+  return newTab;
+}
+
+add_task(async function test_setup() {
+  // Ensure BMB is available in UI.
+  let origBMBlocation = CustomizableUI.getPlacementOfWidget("bookmarks-menu-button");
+  if (!origBMBlocation) {
+    CustomizableUI.addWidgetToArea("bookmarks-menu-button", CustomizableUI.AREA_NAVBAR);
+  }
+
+  await SpecialPowers.pushPrefEnv({
+    "set": [["browser.bookmarks.openInTabClosesMenu", false]]});
+  // Ensure menubar visible.
+  let menubar = document.getElementById("toolbar-menubar");
+  let menubarVisible = isToolbarVisible(menubar);
+  if (!menubarVisible) {
+    setToolbarVisibility(menubar, true);
+    info("Menubar made visible");
+  }
+  // Ensure Bookmarks Toolbar Visible.
+  let toolbar = document.getElementById("PersonalToolbar");
+  let toolbarHidden = toolbar.collapsed;
+  if (toolbarHidden) {
+    await promiseSetToolbarVisibility(toolbar, true);
+    info("Bookmarks toolbar made visible");
+  }
+  // Create our test bookmarks.
+  await PlacesUtils.bookmarks.insert({
+    parentGuid: PlacesUtils.bookmarks.menuGuid,
+    url: "http://example.com/",
+    title: "Test1"
+  });
+  let folder =  await PlacesUtils.bookmarks.insert({
+    parentGuid: PlacesUtils.bookmarks.toolbarGuid,
+    type: PlacesUtils.bookmarks.TYPE_FOLDER,
+    title: "TEST_TITLE",
+    index: 0
+  });
+  await PlacesUtils.bookmarks.insert({
+    parentGuid: folder.guid,
+    url: "http://example.com/",
+    title: "Test1"
+  });
+
+  registerCleanupFunction(async function() {
+    await PlacesUtils.bookmarks.eraseEverything();
+    // if BMB was not originally in UI, remove it.
+    if (!origBMBlocation) {
+      CustomizableUI.removeWidgetFromArea("bookmarks-menu-button");
+    }
+    // Restore menubar to original visibility.
+    setToolbarVisibility(menubar, menubarVisible);
+    // Restore original bookmarks toolbar visibility.
+    if (toolbarHidden) {
+      await promiseSetToolbarVisibility(toolbar, false);
+    }
+  });
+});
+
+add_task(async function testStayopenBookmarksClicks() {
+  // Test Bookmarks Menu Button stayopen clicks - Ctrl-click.
+  let BMB = document.getElementById("bookmarks-menu-button");
+  let BMBpopup = document.getElementById("BMB_bookmarksPopup");
+  let promiseEvent = BrowserTestUtils.waitForEvent(BMBpopup, "popupshown");
+  EventUtils.synthesizeMouseAtCenter(BMB, {});
+  await promiseEvent;
+  info("Popupshown on Bookmarks-Menu-Button");
+  var menuitem = await locateBookmarkAndTestCtrlClick(BMBpopup);
+  ok(BMB.open, "Bookmarks Menu Button's Popup should still be open.");
+
+  // Test Bookmarks Menu Button stayopen clicks: middle-click.
+  let promiseTabOpened = BrowserTestUtils.waitForNewTab(gBrowser, null);
+  EventUtils.synthesizeMouseAtCenter(menuitem, {button: 1});
+  let newTab = await promiseTabOpened;
+  ok(true, "Bookmark middle-click opened new tab.");
+  await BrowserTestUtils.removeTab(newTab);
+  ok(BMB.open, "Bookmarks Menu Button's Popup should still be open.");
+
+  // Test Bookmarks Menu Button stayopen clicks - 'Open in new tab' on context menu.
+  newTab = await testContextmenu(menuitem);
+  ok(true, "Bookmark contextmenu opened new tab.");
+  ok(BMB.open, "Bookmarks Menu Button's Popup should still be open.");
+  promiseEvent = BrowserTestUtils.waitForEvent(BMBpopup, "popuphidden");
+  BMB.open = false;
+  await promiseEvent;
+  info("Closing menu");
+  await BrowserTestUtils.removeTab(newTab);
+
+  // Disable the rest of the tests on Mac due to Mac's handling of menus being
+  // slightly different to the other platforms.
+  if (AppConstants.platform === "macosx") {
+    return;
+  }
+
+  // Test Bookmarks Menu (menubar) stayopen clicks: Ctrl-click.
+  let BM = document.getElementById("bookmarksMenu");
+  let BMpopup = document.getElementById("bookmarksMenuPopup");
+  promiseEvent = BrowserTestUtils.waitForEvent(BMpopup, "popupshown");
+  EventUtils.synthesizeMouseAtCenter(BM, {});
+  await promiseEvent;
+  info("Popupshowing on Bookmarks Menu");
+  menuitem = await locateBookmarkAndTestCtrlClick(BMpopup);
+  ok(BM.open, "Bookmarks Menu's Popup should still be open.");
+
+  // Test Bookmarks Menu (menubar) stayopen clicks: middle-click.
+  promiseTabOpened = BrowserTestUtils.waitForNewTab(gBrowser, null);
+  EventUtils.synthesizeMouseAtCenter(menuitem, {button: 1});
+  newTab = await promiseTabOpened;
+  ok(true, "Bookmark middle-click opened new tab.");
+  await BrowserTestUtils.removeTab(newTab);
+  ok(BM.open, "Bookmarks Menu's Popup should still be open.");
+
+  // Test Bookmarks Menu (menubar) stayopen clicks: 'Open in new tab' on context menu.
+  newTab = await testContextmenu(menuitem);
+  ok(true, "Bookmark contextmenu opened new tab.");
+  await BrowserTestUtils.removeTab(newTab);
+  ok(BM.open, "Bookmarks Menu's Popup should still be open.");
+  promiseEvent = BrowserTestUtils.waitForEvent(BMpopup, "popuphidden");
+  BM.open = false;
+  await promiseEvent;
+
+  // Test Bookmarks Toolbar stayopen clicks - Ctrl-click.
+  let BT = document.getElementById("PlacesToolbarItems");
+  let toolbarbutton = BT.firstChild;
+  ok(toolbarbutton, "Folder should be first item on Bookmarks Toolbar.");
+  let buttonMenupopup = toolbarbutton.firstChild;
+  ok(buttonMenupopup.tagName == "menupopup", "Found toolbar button's menupopup.");
+  promiseEvent = BrowserTestUtils.waitForEvent(buttonMenupopup, "popupshown");
+  EventUtils.synthesizeMouseAtCenter(toolbarbutton, {});
+  await promiseEvent;
+  ok(true, "Bookmarks toolbar folder's popup is open.");
+  menuitem = buttonMenupopup.firstChild.nextSibling;
+  promiseTabOpened = BrowserTestUtils.waitForNewTab(gBrowser, null);
+  EventUtils.synthesizeMouseAtCenter(menuitem, {ctrlKey: true});
+  newTab = await promiseTabOpened;
+  ok(true, "Bookmark in folder on bookmark's toolbar ctrl-click opened new tab.");
+  ok(toolbarbutton.open, "Popup of folder on bookmark's toolbar should still be open.");
+  promiseEvent = BrowserTestUtils.waitForEvent(buttonMenupopup, "popuphidden");
+  toolbarbutton.open = false;
+  await promiseEvent;
+  await BrowserTestUtils.removeTab(newTab);
+
+  // Test Bookmarks Toolbar stayopen clicks: middle-click.
+  promiseEvent = BrowserTestUtils.waitForEvent(buttonMenupopup, "popupshown");
+  EventUtils.synthesizeMouseAtCenter(toolbarbutton, {});
+  await promiseEvent;
+  ok(true, "Bookmarks toolbar folder's popup is open.");
+  promiseTabOpened = BrowserTestUtils.waitForNewTab(gBrowser, null);
+  EventUtils.synthesizeMouseAtCenter(menuitem, {button: 1});
+  newTab = await promiseTabOpened;
+  ok(true, "Bookmark in folder on Bookmarks Toolbar middle-click opened new tab.");
+  ok(toolbarbutton.open, "Popup of folder on bookmark's toolbar should still be open.");
+  promiseEvent = BrowserTestUtils.waitForEvent(buttonMenupopup, "popuphidden");
+  toolbarbutton.open = false;
+  await promiseEvent;
+  await BrowserTestUtils.removeTab(newTab);
+
+  // Test Bookmarks Toolbar stayopen clicks: 'Open in new tab' on context menu.
+  promiseEvent = BrowserTestUtils.waitForEvent(buttonMenupopup, "popupshown");
+  EventUtils.synthesizeMouseAtCenter(toolbarbutton, {});
+  await promiseEvent;
+  ok(true, "Bookmarks toolbar folder's popup is open.");
+  newTab = await testContextmenu(menuitem);
+  ok(true, "Bookmark on Bookmarks Toolbar contextmenu opened new tab.");
+  ok(toolbarbutton.open, "Popup of folder on bookmark's toolbar should still be open.");
+  promiseEvent = BrowserTestUtils.waitForEvent(buttonMenupopup, "popuphidden");
+  toolbarbutton.open = false;
+  await promiseEvent;
+  await BrowserTestUtils.removeTab(newTab);
+});