Bug 402558 - urls from bookmarks folder in sidebar don't open in tabs on middle-click. Patch by Marco Bonardo [mak77] <mak77@supereva.it>, r=me.

browser/base/content/browser-places.js

@@ -582,20 +582,24 @@ var BookmarksEventHandler = {
   /**
    * Handler for click event for an item in the bookmarks toolbar or menu.
    * Menus and submenus from the folder buttons bubble up to this handler.
-   * Only handle middle-click; left-click is handled in the onCommand function.
-   * When items are middle-clicked, open them in tabs.
+   * 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
    */
   onClick: function BT_onClick(aEvent) {
-    // Only handle middle-clicks.
-    if (aEvent.button != 1)
+    // Only handle middle-click or left-click with modifiers.
+#ifdef XP_MACOSX
+    var modifKey = aEvent.metaKey || aEvent.shiftKey;
+#else
+    var modifKey = aEvent.ctrlKey || aEvent.shiftKey;
+#endif
+    if (aEvent.button == 2 || (aEvent.button == 0 && !modifKey))
       return;
 
     var target = aEvent.originalTarget;
-    var view = PlacesUIUtils.getViewForNode(target);
-    if (target.node && PlacesUtils.nodeIsFolder(target.node)) {
+    if (target.node && PlacesUtils.nodeIsContainer(target.node)) {
       // 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.
@@ -605,27 +609,15 @@ var BookmarksEventHandler = {
     else
       this.onCommand(aEvent);
 
-    // If this event bubbled up from a menu or menuitem,
-    // close the menus.
-    if (target.localName == "menu" ||
-        target.localName == "menuitem") {
-      var node = target.parentNode;
-      while (node && 
-             (node.localName == "menu" || 
-              node.localName == "menupopup")) {
+    // If this event bubbled up from a menu or menuitem, close the menus.
+    if (target.localName == "menu" || target.localName == "menuitem") {
+      for (node = target.parentNode; node; node = node.parentNode) {
         if (node.localName == "menupopup")
           node.hidePopup();
-
-        node = node.parentNode;
+        else if (node.localName != "menu")
+          break;
       }
     }
-    // The event target we get if someone middle clicks on a bookmark in the
-    // bookmarks toolbar overflow menu is different from if they click on a
-    // bookmark in a folder or in the bookmarks menu; handle this case
-    // separately.
-    var bookmarksBar = document.getElementById("bookmarksBarContent");
-    if (bookmarksBar._chevron.getAttribute("open") == "true")
-      bookmarksBar._chevron.firstChild.hidePopup();
   },
 
   /**
@@ -727,6 +719,8 @@ var BookmarksEventHandler = {
         target._endOptOpenAllInTabs = document.createElement("menuitem");
         target._endOptOpenAllInTabs.setAttribute("oncommand",
             "PlacesUIUtils.openContainerNodeInTabs(this.parentNode._resultNode, event);");
+        target._endOptOpenAllInTabs.setAttribute("onclick",
+            "checkForMiddleClick(this, event); event.stopPropagation();");
         target._endOptOpenAllInTabs.setAttribute("label",
             gNavigatorBundle.getString("menuOpenAllInTabs.label"));
         target.appendChild(target._endOptOpenAllInTabs);

browser/components/places/content/bookmarksPanel.xul

@@ -73,7 +73,7 @@
         hidecolumnpicker="true"
         context="placesContext"
         onkeypress="SidebarUtils.handleTreeKeyPress(event);"
-        onclick="SidebarUtils.handleTreeClick(this, event);"
+        onclick="SidebarUtils.handleTreeClick(this, event, true);"
         onmousemove="SidebarUtils.handleTreeMouseMove(event);"
         onmouseout="SidebarUtils.clearURLFromStatusBar();">
     <treecols>

browser/components/places/content/controller.js

@@ -598,8 +598,7 @@ PlacesController.prototype = {
       if (!openContainerInTabsItem.hidden && this._view.selectedNode &&
           PlacesUtils.nodeIsContainer(this._view.selectedNode)) {
         openContainerInTabsItem.disabled =
-          PlacesUtils.getURLsForContainerNode(this._view.selectedNode)
-                     .length == 0;
+          !PlacesUtils.hasChildURIs(this._view.selectedNode);
       }
       else {
         // see selectiontype rule in the overlay

browser/components/places/content/history-panel.xul

@@ -121,7 +121,7 @@
         context="placesContext"
         hidecolumnpicker="true"
         onkeypress="SidebarUtils.handleTreeKeyPress(event);"
-        onclick="SidebarUtils.handleTreeClick(this, event);"
+        onclick="SidebarUtils.handleTreeClick(this, event, true);"
         onmousemove="SidebarUtils.handleTreeMouseMove(event);"
         onmouseout="SidebarUtils.clearURLFromStatusBar();">
     <treecols>

browser/components/places/content/sidebarUtils.js

@@ -21,6 +21,7 @@
 #
 # Contributor(s):
 #   Dan Mills <thunder@mozilla.com> (Ported from history-panel.js)
+#   Marco Bonardo <mak77@supereva.it>
 #
 # Alternatively, the contents of this file may be used under the terms of
 # either the GNU General Public License Version 2 or later (the "GPL"), or
@@ -38,8 +39,11 @@
 
 var SidebarUtils = {
   handleTreeClick: function SU_handleTreeClick(aTree, aEvent, aGutterSelect) {
-    var tbo = aTree.treeBoxObject;
+    // right-clicks are not handled here
+    if (aEvent.button == 2)
+      return;
 
+    var tbo = aTree.treeBoxObject;
     var row = { }, col = { }, obj = { };
     tbo.getCellAt(aEvent.clientX, aEvent.clientY, row, col, obj);
 
@@ -53,19 +57,33 @@ var SidebarUtils = {
                                x, y, w, h);
       mouseInGutter = aEvent.clientX < x.value;
     }
-    
-    var modifKey = aEvent.shiftKey || aEvent.ctrlKey || aEvent.altKey || 
-                   aEvent.metaKey  || (aEvent.button != 0);
-    if (!modifKey && tbo.view.isContainer(row.value)) {
+
+#ifdef XP_MACOSX
+    var modifKey = aEvent.metaKey || aEvent.shiftKey;
+#else
+    var modifKey = aEvent.ctrlKey || aEvent.shiftKey;
+#endif
+
+    var isContainer = tbo.view.isContainer(row.value);
+    var openInTabs = isContainer &&
+                     (aEvent.button == 1 ||
+                      (aEvent.button == 0 && modifKey)) &&
+                     PlacesUtils.hasChildURIs(tbo.view.nodeForTreeIndex(row.value));
+
+    if (aEvent.button == 0 && isContainer && !openInTabs) {
       tbo.view.toggleOpenState(row.value);
       return;
     }
-    if (!mouseInGutter && 
-        aEvent.originalTarget.localName == "treechildren" && 
-        (aEvent.button == 0 || aEvent.button == 1)) {
+    else if (!mouseInGutter && openInTabs &&
+            aEvent.originalTarget.localName == "treechildren") {
+      tbo.view.selection.select(row.value);
+      PlacesUIUtils.openContainerNodeInTabs(aTree.selectedNode, aEvent);
+    }
+    else if (!mouseInGutter && !isContainer &&
+             aEvent.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. 
+      // selection as an indication of which link to load.
       tbo.view.selection.select(row.value);
       PlacesUIUtils.openNodeWithEvent(aTree.selectedNode, aEvent);
     }

toolkit/components/places/src/utils.js

@@ -871,11 +871,44 @@ var PlacesUtils = {
     return -1;
   },
 
+  // Returns true if a container has uris in its first level
+  // Has better performances than checking getURLsForContainerNode(node).length
+  hasChildURIs: function PU_hasChildURIs(aNode) {
+    if (!this.nodeIsContainer(aNode))
+      return false;
+
+    // in the Library left pane we use excludeItems
+    if (this.nodeIsFolder(aNode) && asQuery(aNode).queryOptions.excludeItems) {
+      var itemId = PlacesUtils.getConcreteItemId(aNode);
+      var contents = this.getFolderContents(itemId, false, false).root;
+      for (var i = 0; i < contents.childCount; ++i) {
+        var child = contents.getChild(i);
+        if (this.nodeIsURI(child))
+          return true;
+      }
+      return false;
+    }
+
+    var wasOpen = aNode.containerOpen;
+    if (!wasOpen)
+      aNode.containerOpen = true;
+    var found = false;
+    for (var i = 0; i < aNode.childCount && !found; i++) {
+      var child = aNode.getChild(i);
+      if (this.nodeIsURI(child))
+        found = true;
+    }
+    if (!wasOpen)
+      aNode.containerOpen = false;
+    return found;
+  },
+
   getURLsForContainerNode: function PU_getURLsForContainerNode(aNode) {
     let urls = [];
     if (this.nodeIsFolder(aNode) && asQuery(aNode).queryOptions.excludeItems) {
       // grab manually
-      let contents = this.getFolderContents(aNode.itemId, false, false).root;
+      var itemId = this.getConcreteItemId(aNode);
+      let contents = this.getFolderContents(itemId, false, false).root;
       for (let i = 0; i < contents.childCount; ++i) {
         let child = contents.getChild(i);
         if (this.nodeIsURI(child))