Bug 1435562 - browser.tabs.warnOnOpen is now respected when opening multiple items from the library. r=Standard8
authorToby Ward <tobyfrederickward@gmail.com>
Tue, 04 Dec 2018 19:20:59 +0000
changeset 508517 81a6b32f2a8bf8e8579ecc6ae81ae7f0d12f0d91
parent 508516 ffc42da8138efd66299329615945c433bcafc2da
child 508518 766ba8a6c8754a0ed46567040023a9e4051353fe
push id1905
push userffxbld-merge
push dateMon, 21 Jan 2019 12:33:13 +0000
treeherdermozilla-release@c2fca1944d8c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersStandard8
bugs1435562
milestone65.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 1435562 - browser.tabs.warnOnOpen is now respected when opening multiple items from the library. r=Standard8 - Unified openContainerNodeInTabs and openURINodesInTabs in PlacesUIUtils into openMultipleLinksInTabs - Users are now warned when the amount of links to be opened is equal to or exceeds browser.tabs.maxOpenBeforeWarn Differential Revision: https://phabricator.services.mozilla.com/D12983
browser/base/content/browser-places.js
browser/components/places/PlacesUIUtils.jsm
browser/components/places/content/controller.js
browser/components/places/content/places.js
browser/components/places/tests/browser/browser.ini
browser/components/places/tests/browser/browser_library_warnOnOpen.js
browser/components/places/tests/browser/head.js
--- a/browser/base/content/browser-places.js
+++ b/browser/base/content/browser-places.js
@@ -743,17 +743,17 @@ var BookmarksEventHandler = {
       closeMenus(aEvent.target);
     }
 
     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);
+        PlacesUIUtils.openMultipleLinksInTabs(target._placesNode, aEvent, aView);
     } else if (aEvent.button == 1) {
       // left-clicks with modifier are already served by onCommand
       this.onCommand(aEvent);
     }
   },
 
   /**
    * Handler for command event for an item in the bookmarks toolbar.
--- a/browser/components/places/PlacesUIUtils.jsm
+++ b/browser/components/places/PlacesUIUtils.jsm
@@ -621,36 +621,46 @@ var PlacesUIUtils = {
     // loadTabs with aReplace set to false.
     browserWindow.gBrowser.loadTabs(urls, {
       inBackground: loadInBackground,
       replace: false,
       triggeringPrincipal: Services.scriptSecurityManager.getSystemPrincipal(),
     });
   },
 
-  openContainerNodeInTabs:
-  function PUIU_openContainerInTabs(aNode, aEvent, aView) {
-    let window = aView.ownerWindow;
-
-    let urlsToOpen = PlacesUtils.getURLsForContainerNode(aNode);
-    if (OpenInTabsUtils.confirmOpenInTabs(urlsToOpen.length, window)) {
-      this._openTabset(urlsToOpen, aEvent, window);
-    }
-  },
+  /**
+   * Loads a selected node's or nodes' URLs in tabs,
+   * warning the user when lots of URLs are being opened
+   *
+   * @param {object|array} nodeOrNodes
+   *          Contains the node or nodes that we're opening in tabs
+   * @param {event} event
+   *          The DOM mouse/key event with modifier keys set that track the
+   *          user's preferred destination window or tab.
+   * @param {object} view
+   *          The current view that contains the node or nodes selected for
+   *          opening
+   */
+  openMultipleLinksInTabs(nodeOrNodes, event, view) {
+    let window = view.ownerWindow;
+    let urlsToOpen = [];
 
-  openURINodesInTabs: function PUIU_openURINodesInTabs(aNodes, aEvent, aView) {
-    let window = aView.ownerWindow;
-
-    let urlsToOpen = [];
-    for (var i = 0; i < aNodes.length; i++) {
-      // Skip over separators and folders.
-      if (PlacesUtils.nodeIsURI(aNodes[i]))
-        urlsToOpen.push({uri: aNodes[i].uri, isBookmark: PlacesUtils.nodeIsBookmark(aNodes[i])});
+    if (PlacesUtils.nodeIsContainer(nodeOrNodes)) {
+      urlsToOpen = PlacesUtils.getURLsForContainerNode(nodeOrNodes);
+    } else {
+      for (var i = 0; i < nodeOrNodes.length; i++) {
+        // Skip over separators and folders.
+        if (PlacesUtils.nodeIsURI(nodeOrNodes[i])) {
+          urlsToOpen.push({uri: nodeOrNodes[i].uri, isBookmark: PlacesUtils.nodeIsBookmark(nodeOrNodes[i])});
+        }
+      }
     }
-    this._openTabset(urlsToOpen, aEvent, window);
+    if (OpenInTabsUtils.confirmOpenInTabs(urlsToOpen.length, window)) {
+      this._openTabset(urlsToOpen, event, window);
+    }
   },
 
   /**
    * Loads the node's URL in the appropriate tab or window given the
    * user's preference specified by modifier keys tracked by a
    * DOM mouse/key event.
    * @param   aNode
    *          An uri result node.
@@ -951,17 +961,17 @@ var PlacesUIUtils = {
                      (event.button == 1 || (event.button == 0 && modifKey)) &&
                      PlacesUtils.hasChildURIs(tree.view.nodeForTreeIndex(cell.row));
 
     if (event.button == 0 && isContainer && !openInTabs) {
       tbo.view.toggleOpenState(cell.row);
     } else if (!mouseInGutter && openInTabs &&
                event.originalTarget.localName == "treechildren") {
       tbo.view.selection.select(cell.row);
-      this.openContainerNodeInTabs(tree.selectedNode, event, tree);
+      this.openMultipleLinksInTabs(tree.selectedNode, event, tree);
     } else if (!mouseInGutter && !isContainer &&
                event.originalTarget.localName == "treechildren") {
       // Clear all other selection since we're loading a link now. We must
       // do this *before* attempting to load the link since openURL uses
       // selection as an indication of which link to load.
       tbo.view.selection.select(cell.row);
       this.openNodeWithEvent(tree.selectedNode, event);
     }
--- a/browser/components/places/content/controller.js
+++ b/browser/components/places/content/controller.js
@@ -641,20 +641,17 @@ PlacesController.prototype = {
    */
   openSelectionInTabs: function PC_openLinksInTabs(aEvent) {
     var node = this._view.selectedNode;
     var nodes = this._view.selectedNodes;
     // In the case of no selection, open the root node:
     if (!node && !nodes.length) {
       node = this._view.result.root;
     }
-    if (node && PlacesUtils.nodeIsContainer(node))
-      PlacesUIUtils.openContainerNodeInTabs(node, aEvent, this._view);
-    else
-      PlacesUIUtils.openURINodesInTabs(nodes, aEvent, this._view);
+    PlacesUIUtils.openMultipleLinksInTabs(node ? node : nodes, aEvent, this._view);
   },
 
   /**
    * Shows the Add Bookmark UI for the current insertion point.
    *
    * @param aType
    *        the type of the new item (bookmark/folder)
    */
--- a/browser/components/places/content/places.js
+++ b/browser/components/places/content/places.js
@@ -348,17 +348,17 @@ var PlacesOrganizer = {
 
     let node = this._places.selectedNode;
     if (node) {
       let middleClick = aEvent.button == 1 && aEvent.detail == 1;
       if (middleClick && PlacesUtils.nodeIsContainer(node)) {
         // The command execution function will take care of seeing if the
         // selection is a folder or a different container type, and will
         // load its contents in tabs.
-        PlacesUIUtils.openContainerNodeInTabs(node, aEvent, this._places);
+        PlacesUIUtils.openMultipleLinksInTabs(node, aEvent, this._places);
       }
     }
   },
 
   /**
    * Handle focus changes on the places list and the current content view.
    */
   updateDetailsPane: function PO_updateDetailsPane() {
@@ -1290,17 +1290,17 @@ var ContentTree = {
       let middleClick = aEvent.button == 1 && aEvent.detail == 1;
       if (PlacesUtils.nodeIsURI(node) && (doubleClick || middleClick)) {
         // Open associated uri in the browser.
         this.openSelectedNode(aEvent);
       } else if (middleClick && PlacesUtils.nodeIsContainer(node)) {
         // The command execution function will take care of seeing if the
         // selection is a folder or a different container type, and will
         // load its contents in tabs.
-        PlacesUIUtils.openContainerNodeInTabs(node, aEvent, this.view);
+        PlacesUIUtils.openMultipleLinksInTabs(node, aEvent, this.view);
       }
     }
   },
 
   onKeyPress: function CT_onKeyPress(aEvent) {
     if (aEvent.keyCode == KeyEvent.DOM_VK_RETURN)
       this.openSelectedNode(aEvent);
   },
--- a/browser/components/places/tests/browser/browser.ini
+++ b/browser/components/places/tests/browser/browser.ini
@@ -68,16 +68,17 @@ skip-if = (verify && debug && (os == 'ma
 [browser_library_middleclick.js]
 [browser_library_new_bookmark.js]
 [browser_library_open_leak.js]
 [browser_library_openFlatContainer.js]
 [browser_library_open_bookmark.js]
 [browser_library_panel_leak.js]
 [browser_library_search.js]
 [browser_library_views_liveupdate.js]
+[browser_library_warnOnOpen.js]
 [browser_markPageAsFollowedLink.js]
 [browser_panelview_bookmarks_delete.js]
 [browser_paste_bookmarks.js]
 subsuite = clipboard
 [browser_paste_into_tags.js]
 [browser_paste_resets_cut_highlights.js]
 subsuite = clipboard
 [browser_remove_bookmarks.js]
new file mode 100644
--- /dev/null
+++ b/browser/components/places/tests/browser/browser_library_warnOnOpen.js
@@ -0,0 +1,140 @@
+/* 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/. */
+
+/*
+ * Bug 1435562 - Test that browser.tabs.warnOnOpen is respected when
+ * opening multiple items from the Library. */
+
+"use strict";
+
+var gLibrary = null;
+
+add_task(async function setup() {
+  // Temporarily disable history, so we won't record pages navigation.
+  await SpecialPowers.pushPrefEnv({set: [
+    ["places.history.enabled", false],
+  ]});
+
+  // Open Library window.
+  gLibrary = await promiseLibrary();
+
+  registerCleanupFunction(async () => {
+    // We must close "Other Bookmarks" ready for other tests.
+    gLibrary.PlacesOrganizer.selectLeftPaneBuiltIn("UnfiledBookmarks");
+    gLibrary.PlacesOrganizer._places.selectedNode.containerOpen = false;
+
+    await PlacesUtils.bookmarks.eraseEverything();
+
+    // Close Library window.
+    await promiseLibraryClosed(gLibrary);
+  });
+});
+
+add_task(async function test_warnOnOpenFolder() {
+  // Generate a list of links larger than browser.tabs.maxOpenBeforeWarn
+  const MAX_LINKS = 16;
+  let children = [];
+  for (let i = 0; i < MAX_LINKS; i++) {
+    children.push({
+      title: `Folder Target ${i}`,
+      url: `http://example${i}.com`,
+    });
+  }
+
+  // Create a new folder containing our links.
+  await PlacesUtils.bookmarks.insertTree({
+    guid: PlacesUtils.bookmarks.unfiledGuid,
+    children: [{
+      title: "bigFolder",
+      type: PlacesUtils.bookmarks.TYPE_FOLDER,
+      children,
+    }],
+  });
+  info("Pushed test folder into the bookmarks tree");
+
+  // Select unsorted bookmarks root in the left pane.
+  gLibrary.PlacesOrganizer.selectLeftPaneBuiltIn("UnfiledBookmarks");
+  info("Got selection in the Library left pane");
+
+  // Get our bookmark in the right pane.
+  gLibrary.ContentTree.view.view.nodeForTreeIndex(0);
+  info("Got bigFolder in the right pane");
+
+  gLibrary.PlacesOrganizer._places.selectedNode.containerOpen = true;
+
+  // Middle-click on folder (opens all links in folder) and then cancel opening in the dialog
+  let promiseLoaded = BrowserTestUtils.promiseAlertDialog("cancel");
+  let bookmarkedNode = gLibrary.PlacesOrganizer._places.selectedNode.getChild(0);
+  mouseEventOnCell(gLibrary.PlacesOrganizer._places,
+    gLibrary.PlacesOrganizer._places.view.treeIndexForNode(bookmarkedNode),
+    0,
+    { button: 1 });
+
+  await promiseLoaded;
+
+  Assert.ok(true, "Expected dialog was shown when attempting to open folder with lots of links");
+
+  await PlacesUtils.bookmarks.eraseEverything();
+});
+
+add_task(async function test_warnOnOpenLinks() {
+  // Generate a list of links larger than browser.tabs.maxOpenBeforeWarn
+  const MAX_LINKS = 16;
+  let children = [];
+  for (let i = 0; i < MAX_LINKS; i++) {
+    children.push({
+      title: `Highlighted Target ${i}`,
+      url: `http://example${i}.com`,
+    });
+  }
+
+  // Insert the links into the tree
+  await PlacesUtils.bookmarks.insertTree({
+    guid: PlacesUtils.bookmarks.toolbarGuid,
+    children,
+  });
+  info("Pushed test folder into the bookmarks tree");
+
+  gLibrary.PlacesOrganizer.selectLeftPaneBuiltIn("BookmarksToolbar");
+  info("Got selection in the Library left pane");
+
+  // Select all the links
+  gLibrary.ContentTree.view.selectAll();
+
+  let placesContext = gLibrary.document.getElementById("placesContext");
+  let promiseContextMenu = BrowserTestUtils.waitForEvent(placesContext, "popupshown");
+
+  // Open up the context menu and select "Open All In Tabs" (the first item in the list)
+  synthesizeClickOnSelectedTreeCell(gLibrary.ContentTree.view, {
+    button: 2,
+    type: "contextmenu",
+  });
+
+  await promiseContextMenu;
+  info("Context menu opened as expected");
+
+  let openTabs = gLibrary.document.getElementById("placesContext_openLinks:tabs");
+  let promiseLoaded = BrowserTestUtils.promiseAlertDialog("cancel");
+
+  EventUtils.synthesizeMouseAtCenter(openTabs, {}, gLibrary);
+
+  await promiseLoaded;
+
+  Assert.ok(true, "Expected dialog was shown when attempting to open lots of selected links");
+
+  await PlacesUtils.bookmarks.eraseEverything();
+});
+
+function mouseEventOnCell(aTree, aRowIndex, aColumnIndex, aEventDetails) {
+  var selection = aTree.view.selection;
+  selection.select(aRowIndex);
+  aTree.treeBoxObject.ensureRowIsVisible(aRowIndex);
+  var column = aTree.columns[aColumnIndex];
+
+  // get cell coordinates
+  var rect = aTree.treeBoxObject.getCoordsForCellItem(aRowIndex, column, "text");
+
+  EventUtils.synthesizeMouse(aTree.body, rect.x, rect.y,
+    aEventDetails, gLibrary);
+}
--- a/browser/components/places/tests/browser/head.js
+++ b/browser/components/places/tests/browser/head.js
@@ -62,17 +62,17 @@ function promiseLibraryClosed(organizer)
 function promiseClipboard(aPopulateClipboardFn, aFlavor) {
   return new Promise((resolve, reject) => {
     waitForClipboard(data => !!data, aPopulateClipboardFn, resolve, reject, aFlavor);
   });
 }
 
 function synthesizeClickOnSelectedTreeCell(aTree, aOptions) {
   let tbo = aTree.treeBoxObject;
-  if (tbo.view.selection.count != 1)
+  if (tbo.view.selection.count < 1)
      throw new Error("The test node should be successfully selected");
   // Get selection rowID.
   let min = {}, max = {};
   tbo.view.selection.getRangeAt(0, min, max);
   let rowID = min.value;
   tbo.ensureRowIsVisible(rowID);
   // Calculate the click coordinates.
   var rect = tbo.getCoordsForCellItem(rowID, aTree.columns[0], "text");