Bug 423124 - CMD/CTRL+clicking a bookmark creates two new tabs. Patch by Marco Bonardo [mak77] <mak77@supereva.it>, r=me.
authormozilla.mano@sent.com
Mon, 17 Mar 2008 04:33:16 -0700
changeset 13169 373de5f8625ec1623be04107034564089b7695a7
parent 13168 ca9fd7937bac1c08255a47b6f915cca6e054a8e0
child 13170 150736fc1f19a6f3ea4ad42b11822d895bd2220b
push idunknown
push userunknown
push dateunknown
reviewersme
bugs423124
milestone1.9b5pre
Bug 423124 - CMD/CTRL+clicking a bookmark creates two new tabs. Patch by Marco Bonardo [mak77] <mak77@supereva.it>, r=me.
browser/base/content/browser-menubar.inc
browser/base/content/browser-places.js
browser/components/places/content/menu.xml
--- a/browser/base/content/browser-menubar.inc
+++ b/browser/base/content/browser-menubar.inc
@@ -429,18 +429,16 @@
                 command="Browser:ShowAllBookmarks"
                 key="manBookmarkKb"/>
       <menu id="bookmarksToolbarFolderMenu"
             class="menu-iconic bookmark-item"
             container="true">
         <menupopup id="bookmarksToolbarFolderPopup"
                    type="places"
                    context="placesContext"
-                   oncommand="BookmarksEventHandler.onCommand(event);"
-                   onclick="BookmarksEventHandler.onClick(event);"
                    onpopupshowing="BookmarksEventHandler.onPopupShowing(event);"/>
       </menu>
       <menuseparator builder="start"/>
     </menupopup>
   </menu>
 
             <menu id="tools-menu" label="&toolsMenu.label;" accesskey="&toolsMenu.accesskey;">
               <menupopup id="menu_ToolsPopup">
--- a/browser/base/content/browser-places.js
+++ b/browser/base/content/browser-places.js
@@ -594,34 +594,37 @@ var BookmarksEventHandler = {
     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;
+    // 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.
+    if (target.localName == "menu" || target.localName == "menuitem") {
+      for (node = target.parentNode; node; node = node.parentNode) {
+        if (node.localName == "menupopup")
+          node.hidePopup();
+        else if (node.localName != "menu")
+          break;
+      }
+    }
+
     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.
       if (target.localName == "menu" || target.localName == "toolbarbutton")
         PlacesUIUtils.openContainerNodeInTabs(target.node, aEvent);
     }
-    else
+    else if (aEvent.button == 1) {
+      // left-clicks with modifier are already served by onCommand
       this.onCommand(aEvent);
-
-    // 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();
-        else if (node.localName != "menu")
-          break;
-      }
     }
   },
 
   /**
    * Handler for command event for an item in the bookmarks toolbar.
    * Menus and submenus from the folder buttons bubble up to this handler.
    * Opens the item.
    * @param aEvent 
--- a/browser/components/places/content/menu.xml
+++ b/browser/components/places/content/menu.xml
@@ -944,23 +944,28 @@
               window.content.focus();
           ]]>
         </body>
       </method>
     </implementation>
     <handlers>
       <handler event="popupshowing" phase="capturing"><![CDATA[
         this._ensureInitialized();
-        if (event.target._resultNode)
-          this.onPopupShowing(event);
+        var popup = event.target;
+        // We should avoid to handle events of inner views
+        if (!popup._resultNode || PlacesUIUtils.getViewForNode(popup) != this)
+          return;
+
+        this.onPopupShowing(event);
       ]]></handler>
 
       <handler event="popuphidden"><![CDATA[
         var popup = event.target;
-        if (!popup._resultNode)
+        // We should avoid to handle events of inner views
+        if (!popup._resultNode || PlacesUIUtils.getViewForNode(popup) != this)
           return;
 
         // UI performance: folder queries are cheap, keep the resultnode open
         // so we don't rebuild its contents whenever the popup is reopened.
         if (!PlacesUtils.nodeIsFolder(popup._resultNode))
           popup._resultNode.containerOpen = false;
 
         // The autoopened attribute is set for folders which have been