Bug 260611 - Introduce pref browser.bookmarks.openInTabClosesMenu to optionally leave menu open for single bookmark menuitem clicks that open in new tab. r=mak
authorTawny Hoover <stayopenmenu@gmail.com>
Mon, 11 Sep 2017 12:12:21 -0400
changeset 430398 53a1b44951cc0268e02955743016d3920ed6b542
parent 430397 a7e7ef5e54eab4bffa3c800c198d2d8eab5ef89f
child 430399 4621001d8ba55088d3c08c7a4575a500e809984b
push id7761
push userjlund@mozilla.com
push dateFri, 15 Sep 2017 00:19:52 +0000
treeherdermozilla-beta@c38455951db4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs260611
milestone57.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
@@ -496,16 +496,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
@@ -1012,16 +1012,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"
@@ -1089,16 +1090,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);
+});