Fixes several bugs with open in tabs: makes menus set selection in command events in case DOMMenuItemActive doesn't get sent, only shows Open in Tabs menuitem if there are two or more items in the menu, and respects browser.tabs.loadFolderAndReplace and browser.tabs.loadBookmarksInBackground prefs. Also moves #include of debug.js to browser.js as this was causing JavaScript errors on the mac. r=ben@mozilla.org sr=darin@meer.net bug=330130

browser/base/content/browser.js

@@ -6491,7 +6491,12 @@ var BookmarksEventHandler = {
    *        DOMEvent for the command
    */
   onCommand: function BM_onCommand(event) {
-    PlacesController.mouseLoadURI(event);
+    // If this is the special "Open in Tabs" menuitem, load all the menuitems in tabs.
+    if (event.target.hasAttribute("openInTabs"))
+      PlacesController.openLinksInTabs();
+    // If this is a normal bookmark, just load the bookmark's URI.
+    else
+      PlacesController.mouseLoadURI(event);
   },
   
   /**
@@ -6504,11 +6509,25 @@ var BookmarksEventHandler = {
   onPopupShowing: function BM_onPopupShowing(event) {
     if (event.target.localName == "menupopup" &&
         event.target.id != "bookmarksMenuPopup") {
-      var separator = document.createElement("menuseparator");
-      event.target.appendChild(separator);
-      var openInTabs = document.createElement("menuitem");
-      openInTabs.setAttribute("command", "placesCmd_open:tabsEnabled");
-      event.target.appendChild(openInTabs);
+
+      // Show "Open in Tabs" menuitem if there are at least
+      // two menuitems with places result nodes.
+      var numNodes = 0;
+      var currentChild = event.target.firstChild;
+      while (currentChild && numNodes < 2) {
+        if (currentChild.node && currentChild.localName == "menuitem")
+          numNodes++;
+        currentChild = currentChild.nextSibling;
+      }
+      if (numNodes >= 2) {
+        var separator = document.createElement("menuseparator");
+        event.target.appendChild(separator);
+        var openInTabs = document.createElement("menuitem");
+        openInTabs.setAttribute("openInTabs", "true");
+        var strings = document.getElementById("placeBundle");
+        openInTabs.setAttribute("label", strings.getString("menuOpenInTabs.label"));
+        event.target.appendChild(openInTabs);
+      }
     }
   },
   
@@ -6527,4 +6546,6 @@ var BookmarksEventHandler = {
   }
 };
 
+#include ../../../toolkit/content/debug.js
+
 #endif

browser/components/places/content/commands.inc

@@ -53,8 +53,6 @@
          The open:tabsEnabled is always enabled, and is shown in bookmarks menus. -->
     <command id="placesCmd_open:tabs" label="&cmd.open_tabs.label;" accesskey="&cmd.open_tabs.accesskey;"
              oncommand="PlacesController.openLinksInTabs();"/>
-    <command id="placesCmd_open:tabsEnabled" label="&cmd.open_tabs.label;" accesskey="&cmd.open_tabs.accesskey;"
-             oncommand="PlacesController.openLinksInTabs();"/>
   </commandset>
   <commandset type="container">
     <command id="placesCmd_sortby:name" label="&cmd.sortby_name.label;" accesskey="&cmd.sortby_name.accesskey;"/>

browser/components/places/content/controller.js

@@ -643,7 +643,6 @@ var PlacesController = {
       this.nodeIsFolder(this._activeView.selectedNode);
     this._setEnabled("placesCmd_open:tabs", 
       singleFolderSelected || !hasSingleSelection);
-    this._setEnabled("placesCmd_open:tabsEnabled", true); // Always on
     
     // Some views, like menupopups, destroy their result as they hide, but they
     // are still the "last-active" view. Don't barf. 
@@ -903,17 +902,70 @@ var PlacesController = {
   openLinksInTabs: function PC_openLinksInTabs() {
     var node = this._activeView.selectedNode;
     if (this._activeView.hasSingleSelection && this.nodeIsFolder(node)) {
+      // Check prefs to see whether to open over existing tabs.
+      var doReplace = getBoolPref("browser.tabs.loadFolderAndReplace");
+      var loadInBackground = getBoolPref("browser.tabs.loadBookmarksInBackground");
+      // Get the start index to open tabs at
+      var browser = this.browserWindow.getBrowser();
+      var tabPanels = browser.browsers;
+      var tabCount = tabPanels.length;
+      var firstIndex;
+      // If browser.tabs.loadFolderAndReplace pref is set, load over all the
+      // tabs starting with the first one.
+      if (doReplace)
+        firstIndex = 0;
+      // If the pref is not set, only load over the blank tabs at the end, if any.
+      else {
+        for (firstIndex = tabCount - 1; firstIndex >= 0; --firstIndex)
+          if (browser.browsers[firstIndex].currentURI.spec != "about:blank")
+            break;
+        ++firstIndex;
+      }
+
+      // Open each uri in the folder in a tab.
+      var index = firstIndex;
       asFolder(node);
       var wasOpen = node.containerOpen;
       node.containerOpen = true;
       var cc = node.childCount;
       for (var i = 0; i < cc; ++i) {
         var childNode = node.getChild(i);
-        if (this.nodeIsURI(childNode))
-          this.browserWindow.openNewTabWith(childNode.uri,
-              null, null);
+        if (this.nodeIsURI(childNode)) {
+          // If there are tabs to load over, load the uri into the next tab.
+          if (index < tabCount)
+            tabPanels[index].loadURI(childNode.uri);
+          // Otherwise, create a new tab to load the uri into.
+          else
+            browser.addTab(childNode.uri);
+          ++index;
+        }
       }
       node.containerOpen = wasOpen;
+      
+      // If no bookmarks were loaded, just bail.
+      if (index == firstIndex)
+        return;
+
+      // focus the first tab if prefs say to
+      if (!loadInBackground || doReplace) {
+        // Select the first tab in the group.
+        // Set newly selected tab after quick timeout, otherwise hideous focus problems
+        // can occur because new presshell is not ready to handle events
+        function selectNewForegroundTab(browser, tab) {
+          browser.selectedTab = tab;
+        }
+        var tabs = browser.mTabContainer.childNodes;
+        setTimeout(selectNewForegroundTab, 0, browser, tabs[firstIndex]);
+      }
+
+      // Close any remaining open tabs that are left over.
+      // (Always skipped when we append tabs)
+      for (var i = tabCount - 1; i >= index; --i)
+        browser.removeTab(tabs[i]);
+
+      // and focus the content
+      this.browserWindow.content.focus();
+
     }
     else {
       var nodes = this._activeView.getSelectionNodes();

browser/components/places/content/menu.xml

@@ -135,7 +135,7 @@
           // of exceptions and asserts will fire.
           if (this._DNDObserver._overFolder.node)
             this._DNDObserver._clearOverFolder();
-            
+
           this._cleanMenu();
           const XULNS = "http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul";
           if (PlacesController.nodeIsContainer(this._resultNode))
@@ -635,7 +635,7 @@
           // node has a command but no data associated with it, it should
           // act on the entire menu.
           this._selection = event.target.node;
-          if (event.target.node == null && event.target.hasAttribute("command"))
+          if (event.target.node == null)
             this._selection = this._resultNode;
           // Set the active view to this node.
           PlacesController.activeView = this;
@@ -679,6 +679,10 @@
         if (this.eventValid(event))
           this.setSelectionForEvent(event);
       ]]></handler>
+      <handler event="command"><![CDATA[
+        if (this.eventValid(event))
+          this.setSelectionForEvent(event);
+      ]]></handler>
       <handler event="draggesture"><![CDATA[
         if (event.target.localName == "menuitem")
           // TODO--allow menu drag if shift (or alt??) key is down

toolkit/content/globalOverlay.js

@@ -256,5 +256,3 @@ function FillInTooltip ( tipElement )
   }
   return retVal;
 }
-
-#include debug.js