Bug 562998 - Selecting commands from a bookmarks context menu while the window isn't active does nothing. r=mak. a=blocker.
☠☠ backed out by 21bfcb551f60 ☠ ☠
authorAsaf Romano (Mano) <mano@mozilla.com>
Sat, 22 Jan 2011 02:39:39 +0200
changeset 61124 dd398fdba56f162be8976a21b40eb4b8b22924f9
parent 61120 d341b2ece4e43d655c9a57f80cda919d85321996
child 61125 7a7b760d31acad0b5532b082f350ff6df53f7d18
child 61130 21bfcb551f60f2415c34bb042e03e1122d02dff1
push idunknown
push userunknown
push dateunknown
reviewersmak, blocker
bugs562998
milestone2.0b10pre
Bug 562998 - Selecting commands from a bookmarks context menu while the window isn't active does nothing. r=mak. a=blocker.
browser/base/content/browser-menubar.inc
browser/base/content/browser-places.js
browser/base/content/browser.css
browser/base/content/browser.xul
browser/components/places/content/controller.js
browser/components/places/content/places.js
browser/components/places/content/sidebarUtils.js
browser/components/places/src/PlacesUIUtils.jsm
browser/components/sidebar/src/nsSidebar.js
--- a/browser/base/content/browser-menubar.inc
+++ b/browser/base/content/browser-menubar.inc
@@ -424,18 +424,18 @@
         ondragover="PlacesMenuDNDHandler.onDragOver(event);"
         ondrop="PlacesMenuDNDHandler.onDrop(event);">
     <menupopup id="bookmarksMenuPopup"
 #ifndef XP_MACOSX
                placespopup="true"
 #endif
                context="placesContext"
                openInTabs="children"
-               oncommand="BookmarksEventHandler.onCommand(event);"
-               onclick="BookmarksEventHandler.onClick(event);"
+               oncommand="BookmarksEventHandler.onCommand(event, this.parentNode._placesView);"
+               onclick="BookmarksEventHandler.onClick(event, this.parentNode._placesView);"
                onpopupshowing="if (!this.parentNode._placesView)
                                  new PlacesMenu(event, 'place:folder=BOOKMARKS_MENU');"
                tooltip="bhTooltip" popupsinherittooltip="true">
       <menuitem id="menu_bookmarkThisPage"
                 label="&bookmarkThisPageCmd.label;"
                 command="Browser:AddBookmarkAs"
                 key="addBookmarkAsKb"/>
       <menuitem id="subscribeToPageMenuitem"
--- a/browser/base/content/browser-places.js
+++ b/browser/base/content/browser-places.js
@@ -707,18 +707,20 @@ 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.
    * 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.
    */
-  onClick: function BEH_onClick(aEvent) {
+  onClick: function BEH_onClick(aEvent, aView) {
     // 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;
@@ -738,35 +740,37 @@ var BookmarksEventHandler = {
       }
     }
 
     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);
+        PlacesUIUtils.openContainerNodeInTabs(target._placesNode, aEvent, aView);
     }
     else if (aEvent.button == 1) {
       // left-clicks with modifier are already served by onCommand
-      this.onCommand(aEvent);
+      this.onCommand(aEvent, aView);
     }
   },
 
   /**
    * 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 
    *        DOMEvent for the command
+   * @param aView
+   *        The places view which aEvent should be associated with.
    */
-  onCommand: function BEH_onCommand(aEvent) {
+  onCommand: function BEH_onCommand(aEvent, aView) {
     var target = aEvent.originalTarget;
     if (target._placesNode)
-      PlacesUIUtils.openNodeWithEvent(target._placesNode, aEvent);
+      PlacesUIUtils.openNodeWithEvent(target._placesNode, aEvent, aView);
   },
 
   fillInBHTooltip: function BEH_fillInBHTooltip(aDocument, aEvent) {
     var node;
     var cropped = false;
     var targetURI;
 
     if (aDocument.tooltipNode.localName == "treechildren") {
--- a/browser/base/content/browser.css
+++ b/browser/base/content/browser.css
@@ -187,24 +187,26 @@ splitmenu {
 }
 %endif
 
 /* ::::: location bar ::::: */
 #urlbar {
   -moz-binding: url(chrome://browser/content/urlbarBindings.xml#urlbar);
 }
 
-/* Some child nodes want to be ordered based on the locale's direction, while
-   everything else should be ltr. */
-.urlbar-input-box:-moz-locale-dir(rtl) {
-  direction: rtl;
+.uri-element-right-align:-moz-locale-dir(rtl),
+html|input.uri-element-right-align:-moz-locale-dir(rtl),
+.ac-url-text:-moz-locale-dir(rtl),
+.ac-title:-moz-locale-dir(rtl)  > description {
+  direction: ltr !important;
+  text-align: right !important;
 }
 
-html|*.urlbar-input {
-  direction: ltr;
+.urlbar-over-link-box:-moz-locale-dir(rtl) {
+  -moz-box-direction: reverse;
 }
 
 /* over-link in location bar */
 
 .urlbar-textbox-container[overlinkstate="fade-in"],
 .urlbar-over-link-layer[overlinkstate="fade-out"] {
   -moz-transition-property: color;
   -moz-transition-duration: 150ms;
--- a/browser/base/content/browser.xul
+++ b/browser/base/content/browser.xul
@@ -642,18 +642,18 @@
                        ondragenter="PlacesMenuDNDHandler.onDragEnter(event);"
                        ondragover="PlacesMenuDNDHandler.onDragOver(event);"
                        ondragexit="PlacesMenuDNDHandler.onDragExit(event);"
                        ondrop="PlacesMenuDNDHandler.onDrop(event);">
           <menupopup id="BMB_bookmarksPopup"
                      placespopup="true"
                      context="placesContext"
                      openInTabs="children"
-                     oncommand="BookmarksEventHandler.onCommand(event);"
-                     onclick="BookmarksEventHandler.onClick(event);"
+                     oncommand="BookmarksEventHandler.onCommand(event, this.parentNode._placesView);"
+                     onclick="BookmarksEventHandler.onClick(event, this.parentNode._placesView);"
                      onpopupshowing="BookmarksMenuButton.onPopupShowing(event);
                                      if (!this.parentNode._placesView)
                                        new PlacesMenu(event, 'place:folder=BOOKMARKS_MENU');"
                      tooltip="bhTooltip" popupsinherittooltip="true">
             <menuitem id="BMB_viewBookmarksToolbar"
                       placesanonid="view-toolbar"
                       toolbarId="PersonalToolbar"
                       type="checkbox"
@@ -742,18 +742,18 @@
              toolbarname="&personalbarCmd.label;" accesskey="&personalbarCmd.accesskey;"
              collapsed="true"
              customizable="true">
       <toolbaritem flex="1" id="personal-bookmarks" title="&bookmarksItem.title;"
                    removable="true">
         <hbox flex="1"
               id="PlacesToolbar"
               context="placesContext"
-              onclick="BookmarksEventHandler.onClick(event);"
-              oncommand="BookmarksEventHandler.onCommand(event);"
+              onclick="BookmarksEventHandler.onClick(event, this._placesView);"
+              oncommand="BookmarksEventHandler.onCommand(event, this._placesView);"
               tooltip="bhTooltip"
               popupsinherittooltip="true">
           <toolbarbutton class="bookmark-item bookmarks-toolbar-customize"
                          mousethrough="never"
                          label="&bookmarksToolbarItem.label;"/>
           <hbox flex="1">
             <hbox align="center">
               <image id="PlacesToolbarDropIndicator"
--- a/browser/components/places/content/controller.js
+++ b/browser/components/places/content/controller.js
@@ -259,23 +259,23 @@ PlacesController.prototype = {
       else
         host = PlacesUtils._uri(this._view.selectedNode.uri).host;
       PlacesUIUtils.privateBrowsing.removeDataFromDomain(host);
       break;
     case "cmd_selectAll":
       this.selectAll();
       break;
     case "placesCmd_open":
-      PlacesUIUtils.openNodeIn(this._view.selectedNode, "current");
+      PlacesUIUtils.openNodeIn(this._view.selectedNode, "current", this._view);
       break;
     case "placesCmd_open:window":
-      PlacesUIUtils.openNodeIn(this._view.selectedNode, "window");
+      PlacesUIUtils.openNodeIn(this._view.selectedNode, "window", this._view);
       break;
     case "placesCmd_open:tab":
-      PlacesUIUtils.openNodeIn(this._view.selectedNode, "tab");
+      PlacesUIUtils.openNodeIn(this._view.selectedNode, "tab", this._view);
       break;
     case "placesCmd_new:folder":
       this.newItem("folder");
       break;
     case "placesCmd_new:bookmark":
       this.newItem("bookmark");
       break;
     case "placesCmd_new:livemark":
@@ -295,18 +295,26 @@ PlacesController.prototype = {
       break;
     case "placesCmd_reloadMicrosummary":
       this.reloadSelectedMicrosummary();
       break;
     case "placesCmd_sortBy:name":
       this.sortFolderByName();
       break;
     case "placesCmd_createBookmark":
-      var node = this._view.selectedNode;
-      PlacesUIUtils.showMinimalAddBookmarkUI(PlacesUtils._uri(node.uri), node.title);
+      let node = this._view.selectedNode;
+      PlacesUIUtils.showBookmarkDialog({ action: "add"
+                                       , type: "bookmark"
+                                       , hiddenRows: [ "description"
+                                                     , "keyword"
+                                                     , "location"
+                                                     , "loadInSidebar" ]
+                                       , uri: PlacesUtils._uri(node.uri)
+                                       , title: node.title
+                                       }, window);
       break;
     }
   },
 
   onEvent: function PC_onEvent(eventName) { },
 
 
   /**
@@ -703,18 +711,21 @@ PlacesController.prototype = {
     var isRootItem = PlacesUtils.isRootItem(concreteId);
     var itemId = node.itemId;
     if (isRootItem || PlacesUtils.nodeIsTagQuery(node)) {
       // If this is a root or the Tags query we use the concrete itemId to catch
       // the correct title for the node.
       itemId = concreteId;
     }
 
-    PlacesUIUtils.showItemProperties(itemId, itemType,
-                                     isRootItem /* read only */);
+    PlacesUIUtils.showBookmarkDialog({ action: "edit"
+                                     , type: itemType
+                                     , itemId: itemId
+                                     , readOnly: isRootItem
+                                     }, window);
   },
 
   /**
    * This method can be run on a URI parameter to ensure that it didn't
    * receive a string instead of an nsIURI object.
    */
   _assertURINotString: function PC__assertURINotString(value) {
     NS_ASSERT((typeof(value) == "object") && !(value instanceof String),
@@ -736,117 +747,60 @@ PlacesController.prototype = {
   reloadSelectedMicrosummary: function PC_reloadSelectedMicrosummary() {
     var selectedNode = this._view.selectedNode;
     var mss = PlacesUtils.microsummaries;
     if (mss.hasMicrosummary(selectedNode.itemId))
       mss.refreshMicrosummary(selectedNode.itemId);
   },
 
   /**
-   * Gives the user a chance to cancel loading lots of tabs at once
-   */
-  _confirmOpenTabs: function(numTabsToOpen) {
-    var pref = Cc["@mozilla.org/preferences-service;1"].
-               getService(Ci.nsIPrefBranch);
-
-    const kWarnOnOpenPref = "browser.tabs.warnOnOpen";
-    var reallyOpen = true;
-    if (pref.getBoolPref(kWarnOnOpenPref)) {
-      if (numTabsToOpen >= pref.getIntPref("browser.tabs.maxOpenBeforeWarn")) {
-        var promptService = Cc["@mozilla.org/embedcomp/prompt-service;1"].
-                            getService(Ci.nsIPromptService);
-
-        // default to true: if it were false, we wouldn't get this far
-        var warnOnOpen = { value: true };
-
-        var messageKey = "tabs.openWarningMultipleBranded";
-        var openKey = "tabs.openButtonMultiple";
-        const BRANDING_BUNDLE_URI = "chrome://branding/locale/brand.properties";
-        var brandShortName = Cc["@mozilla.org/intl/stringbundle;1"].
-                             getService(Ci.nsIStringBundleService).
-                             createBundle(BRANDING_BUNDLE_URI).
-                             GetStringFromName("brandShortName");
-
-        var buttonPressed = promptService.confirmEx(window,
-          PlacesUIUtils.getString("tabs.openWarningTitle"),
-          PlacesUIUtils.getFormattedString(messageKey,
-            [numTabsToOpen, brandShortName]),
-          (promptService.BUTTON_TITLE_IS_STRING * promptService.BUTTON_POS_0)
-          + (promptService.BUTTON_TITLE_CANCEL * promptService.BUTTON_POS_1),
-          PlacesUIUtils.getString(openKey),
-          null, null,
-          PlacesUIUtils.getFormattedString("tabs.openWarningPromptMeBranded",
-            [brandShortName]),
-          warnOnOpen);
-
-         reallyOpen = (buttonPressed == 0);
-         // don't set the pref unless they press OK and it's false
-         if (reallyOpen && !warnOnOpen.value)
-           pref.setBoolPref(kWarnOnOpenPref, false);
-      }
-    }
-    return reallyOpen;
-  },
-
-  /**
    * Opens the links in the selected folder, or the selected links in new tabs.
    */
   openSelectionInTabs: function PC_openLinksInTabs(aEvent) {
     var node = this._view.selectedNode;
     if (node && PlacesUtils.nodeIsContainer(node))
-      PlacesUIUtils.openContainerNodeInTabs(this._view.selectedNode, aEvent);
+      PlacesUIUtils.openContainerNodeInTabs(this._view.selectedNode, aEvent, this._view);
     else
-      PlacesUIUtils.openURINodesInTabs(this._view.selectedNodes, aEvent);
+      PlacesUIUtils.openURINodesInTabs(this._view.selectedNodes, aEvent, this._view);
   },
 
   /**
    * Shows the Add Bookmark UI for the current insertion point.
    *
    * @param aType
    *        the type of the new item (bookmark/livemark/folder)
    */
   newItem: function PC_newItem(aType) {
-    var ip = this._view.insertionPoint;
+    let ip = this._view.insertionPoint;
     if (!ip)
       throw Cr.NS_ERROR_NOT_AVAILABLE;
 
-    var performed = false;
-    if (aType == "bookmark")
-      performed = PlacesUIUtils.showAddBookmarkUI(null, null, null, ip);
-    else if (aType == "livemark")
-      performed = PlacesUIUtils.showAddLivemarkUI(null, null, null, null, ip);
-    else // folder
-      performed = PlacesUIUtils.showAddFolderUI(null, ip);
-
+    let performed =
+      PlacesUIUtils.showBookmarkDialog({ action: "add"
+                                       , type: aType
+                                       , defaultInsertionPoint: ip
+                                       , hiddenRows: [ "folderPicker" ]
+                                       }, window);
     if (performed) {
-      // select the new item
-      var insertedNodeId = PlacesUtils.bookmarks
+      // Select the new item.
+      let insertedNodeId = PlacesUtils.bookmarks
                                       .getIdForItemAt(ip.itemId, ip.index);
       this._view.selectItems([insertedNodeId], false);
     }
   },
 
 
   /**
    * Create a new Bookmark folder somewhere. Prompts the user for the name
    * of the folder.
    */
   newFolder: function PC_newFolder() {
-    var ip = this._view.insertionPoint;
-    if (!ip)
-      throw Cr.NS_ERROR_NOT_AVAILABLE;
-
-    var performed = false;
-    performed = PlacesUIUtils.showAddFolderUI(null, ip);
-    if (performed) {
-      // select the new item
-      var insertedNodeId = PlacesUtils.bookmarks
-                                      .getIdForItemAt(ip.itemId, ip.index);
-      this._view.selectItems([insertedNodeId], false);
-    }
+    Cu.reportError("PlacesController.newFolder is deprecated and will be \
+                   removed in a future release.  Use newItem instead.");
+    this.newItem("folder");
   },
 
   /**
    * Create a new Bookmark separator somewhere.
    */
   newSeparator: function PC_newSeparator() {
     var ip = this._view.insertionPoint;
     if (!ip)
--- a/browser/components/places/content/places.js
+++ b/browser/components/places/content/places.js
@@ -338,17 +338,18 @@ var PlacesOrganizer = {
   openFlatContainer: function PO_openFlatContainerFlatContainer(aContainer) {
     if (aContainer.itemId != -1)
       this._places.selectItems([aContainer.itemId]);
     else if (PlacesUtils.nodeIsQuery(aContainer))
       this._places.selectPlaceURI(aContainer.uri);
   },
 
   openSelectedNode: function PO_openSelectedNode(aEvent) {
-    PlacesUIUtils.openNodeWithEvent(this._content.selectedNode, aEvent);
+    PlacesUIUtils.openNodeWithEvent(this._content.selectedNode, aEvent,
+                                    this._content.treeBoxObject.view);
   },
 
   /**
    * Returns the options associated with the query currently loaded in the
    * main places pane.
    */
   getCurrentOptions: function PO_getCurrentOptions() {
     return PlacesUtils.asQuery(this._content.result.root).queryOptions;
--- a/browser/components/places/content/sidebarUtils.js
+++ b/browser/components/places/content/sidebarUtils.js
@@ -81,31 +81,34 @@ var SidebarUtils = {
 
     if (aEvent.button == 0 && isContainer && !openInTabs) {
       tbo.view.toggleOpenState(row.value);
       return;
     }
     else if (!mouseInGutter && openInTabs &&
             aEvent.originalTarget.localName == "treechildren") {
       tbo.view.selection.select(row.value);
-      PlacesUIUtils.openContainerNodeInTabs(aTree.selectedNode, aEvent);
+      PlacesUIUtils.openContainerNodeInTabs(aTree.selectedNode, aEvent, tbo.view);
     }
     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.
       tbo.view.selection.select(row.value);
-      PlacesUIUtils.openNodeWithEvent(aTree.selectedNode, aEvent);
+      PlacesUIUtils.openNodeWithEvent(aTree.selectedNode, aEvent, tbo.view);
     }
   },
 
   handleTreeKeyPress: function SU_handleTreeKeyPress(aEvent) {
+    // XXX Bug 627901: Post Fx4, this method should take a tree parameter.
+    let node = aEvent.target.selectedNode;
+    let view = PlacesUIUtils.getViewForNode(node);
     if (aEvent.keyCode == KeyEvent.DOM_VK_RETURN)
-      PlacesUIUtils.openNodeWithEvent(aEvent.target.selectedNode, aEvent);
+      PlacesUIUtils.openNodeWithEvent(node, aEvent, view);
   },
 
   /**
    * The following function displays the URL of a node that is being
    * hovered over.
    */
   handleTreeMouseMove: function SU_handleTreeMouseMove(aEvent) {
     if (aEvent.target.localName != "treechildren")
--- a/browser/components/places/src/PlacesUIUtils.jsm
+++ b/browser/components/places/src/PlacesUIUtils.jsm
@@ -311,69 +311,31 @@ var PlacesUIUtils = {
                                                              data.uri;
           return this.ptm.createItem(PlacesUtils._uri(data.uri),
                                      container, index, title);
         }
     }
     return null;
   },
 
-  /**
-   * Methods to show the bookmarkProperties dialog in its various modes.
-   *
-   * The showMinimalAdd* methods open the dialog by its alternative URI. Thus
-   * they persist the dialog dimensions separately from the showAdd* methods.
-   * Note these variants also do not return the dialog "performed" state since
-   * they may not open the dialog modally.
-   */
+  _reportDeprecatedAddBookmarkMethod:
+  function PUIU__reportDeprecatedAddBookmarkMethod() {
+    let oldFuncName = arugments.callee.caller.name.slice(5); // remove PUIU_
+    Cu.reportError(oldFuncName + " is deprecated and will be removed in a \
+                   future release.  Use showBookmarkDialog instead");
+  },
 
   /**
-   * Shows the "Add Bookmark" dialog.
-   *
-   * @param [optional] aURI
-   *        An nsIURI object for which the "add bookmark" dialog is
-   *        to be shown.
-   * @param [optional] aTitle
-   *        The default title for the new bookmark.
-   * @param [optional] aDescription
-            The default description for the new bookmark
-   * @param [optional] aDefaultInsertionPoint
-   *        The default insertion point for the new item. If set, the folder
-   *        picker would be hidden unless aShowPicker is set to true, in which
-   *        case the dialog only uses the folder identifier from the insertion
-   *        point as the initially selected item in the folder picker.
-   * @param [optional] aShowPicker
-   *        see above
-   * @param [optional] aLoadInSidebar
-   *        If true, the dialog will default to load the new item in the
-   *        sidebar (as a web panel).
-   * @param [optional] aKeyword
-   *        The default keyword for the new bookmark. The keyword field
-   *        will be shown in the dialog if this is used.
-   * @param [optional] aPostData
-   *        POST data for POST-style keywords.
-   * @param [optional] aCharSet
-   *        The character set for the bookmarked page.
-   * @return true if any transaction has been performed.
-   *
-   * Notes:
-   *  - the location, description and "loadInSidebar" fields are
-   *    visible only if there is no initial URI (aURI is null).
-   *  - When aDefaultInsertionPoint is not set, the dialog defaults to the
-   *    bookmarks root folder.
+   * This is here for compatibility reasons, use ShowBookmarkDialog instead.
    */
-  showAddBookmarkUI: function PUIU_showAddBookmarkUI(aURI,
-                                                     aTitle,
-                                                     aDescription,
-                                                     aDefaultInsertionPoint,
-                                                     aShowPicker,
-                                                     aLoadInSidebar,
-                                                     aKeyword,
-                                                     aPostData,
-                                                     aCharSet) {
+  showAddBookmarkUI: function PUIU_showAddBookmarkUI(
+    aURI, aTitle, aDescription, aDefaultInsertionPoint, aShowPicker,
+    aLoadInSidebar, aKeyword, aPostData, aCharSet) {
+    this._reportDeprecatedAddBookmarkMethod();
+
     var info = {
       action: "add",
       type: "bookmark"
     };
 
     if (aURI)
       info.uri = aURI;
 
@@ -396,35 +358,28 @@ var PlacesUIUtils = {
     if (typeof(aKeyword) == "string") {
       info.keyword = aKeyword;
       if (typeof(aPostData) == "string")
         info.postData = aPostData;
       if (typeof(aCharSet) == "string")
         info.charSet = aCharSet;
     }
 
-    return this._showBookmarkDialog(info);
+    return this.showBookmarkDialog(info);
   },
 
   /**
-   * @see showAddBookmarkUI
-   * This opens the dialog with only the name and folder pickers visible by
-   * default.
-   *
-   * You can still pass in the various paramaters as the default properties
-   * for the new bookmark.
-   *
-   * The keyword field will be visible only if the aKeyword parameter
-   * was used.
+   * This is here for compatibility reasons, use ShowBookmarkDialog instead.
    */
   showMinimalAddBookmarkUI:
-  function PUIU_showMinimalAddBookmarkUI(aURI, aTitle, aDescription,
-                                         aDefaultInsertionPoint, aShowPicker,
-                                         aLoadInSidebar, aKeyword, aPostData,
-                                         aCharSet) {
+  function PUIU_showMinimalAddBookmarkUI(
+    aURI, aTitle, aDescription, aDefaultInsertionPoint, aShowPicker,
+    aLoadInSidebar, aKeyword, aPostData, aCharSet) {
+    this._reportDeprecatedAddBookmarkMethod();
+
     var info = {
       action: "add",
       type: "bookmark",
       hiddenRows: ["description"]
     };
     if (aURI)
       info.uri = aURI;
 
@@ -454,47 +409,30 @@ var PlacesUIUtils = {
       if (typeof(aPostData) == "string")
         info.postData = aPostData;
       if (typeof(aCharSet) == "string")
         info.charSet = aCharSet;
     }
     else
       info.hiddenRows.push("keyword");
 
-    return this._showBookmarkDialog(info, true);
+    return this.showBookmarkDialog(info);
   },
 
   /**
-   * Shows the "Add Live Bookmark" dialog.
-   *
-   * @param [optional] aFeedURI
-   *        The feed URI for which the dialog is to be shown (nsIURI).
-   * @param [optional] aSiteURI
-   *        The site URI for the new live-bookmark (nsIURI).
-   * @param [optional] aDefaultInsertionPoint
-   *        The default insertion point for the new item. If set, the folder
-   *        picker would be hidden unless aShowPicker is set to true, in which
-   *        case the dialog only uses the folder identifier from the insertion
-   *        point as the initially selected item in the folder picker.
-   * @param [optional] aShowPicker
-   *        see above
-   * @return true if any transaction has been performed.
-   *
-   * Notes:
-   *  - the feedURI and description fields are visible only if there is no
-   *    initial feed URI (aFeedURI is null).
-   *  - When aDefaultInsertionPoint is not set, the dialog defaults to the
-   *    bookmarks root folder.
+   * This is here for compatibility reasons, use ShowBookmarkDialog instead.
    */
   showAddLivemarkUI: function PUIU_showAddLivemarkURI(aFeedURI,
                                                       aSiteURI,
                                                       aTitle,
                                                       aDescription,
                                                       aDefaultInsertionPoint,
                                                       aShowPicker) {
+    this._reportDeprecatedAddBookmarkMethod();
+
     var info = {
       action: "add",
       type: "livemark"
     };
 
     if (aFeedURI)
       info.feedURI = aFeedURI;
     if (aSiteURI)
@@ -507,31 +445,29 @@ var PlacesUIUtils = {
     if (aDescription)
       info.description = aDescription;
 
     if (aDefaultInsertionPoint) {
       info.defaultInsertionPoint = aDefaultInsertionPoint;
       if (!aShowPicker)
         info.hiddenRows = ["folderPicker"];
     }
-    return this._showBookmarkDialog(info);
+    return this.showBookmarkDialog(info);
   },
 
   /**
-   * @see showAddLivemarkUI
-   * This opens the dialog with only the name and folder pickers visible by
-   * default.
-   *
-   * You can still pass in the various paramaters as the default properties
-   * for the new live-bookmark.
+   * This is here for compatibility reasons, use ShowBookmarkDialog instead.
    */
   showMinimalAddLivemarkUI:
-  function PUIU_showMinimalAddLivemarkURI(aFeedURI, aSiteURI, aTitle,
-                                          aDescription, aDefaultInsertionPoint,
-                                          aShowPicker) {
+  function PUIU_showMinimalAddLivemarkURI(
+    aFeedURI, aSiteURI, aTitle, aDescription, aDefaultInsertionPoint,
+    aShowPicker) {
+
+    this._reportDeprecatedAddBookmarkMethod();
+
     var info = {
       action: "add",
       type: "livemark",
       hiddenRows: ["feedLocation", "siteLocation", "description"]
     };
 
     if (aFeedURI)
       info.feedURI = aFeedURI;
@@ -545,129 +481,114 @@ var PlacesUIUtils = {
     if (aDescription)
       info.description = aDescription;
 
     if (aDefaultInsertionPoint) {
       info.defaultInsertionPoint = aDefaultInsertionPoint;
       if (!aShowPicker)
         info.hiddenRows.push("folderPicker");
     }
-    return this._showBookmarkDialog(info, true);
+    return this.showBookmarkDialog(info);
   },
 
   /**
-   * Show an "Add Bookmarks" dialog to allow the adding of a folder full
-   * of bookmarks corresponding to the objects in the uriList.  This will
-   * be called most often as the result of a "Bookmark All Tabs..." command.
-   *
-   * @param aURIList  List of nsIURI objects representing the locations
-   *                  to be bookmarked.
-   * @return true if any transaction has been performed.
+   * This is here for compatibility reasons, use ShowBookmarkDialog instead.
    */
   showMinimalAddMultiBookmarkUI: function PUIU_showAddMultiBookmarkUI(aURIList) {
+    this._reportDeprecatedAddBookmarkMethod();
+
     if (aURIList.length == 0)
       throw("showAddMultiBookmarkUI expects a list of nsIURI objects");
     var info = {
       action: "add",
       type: "folder",
       hiddenRows: ["description"],
       URIList: aURIList
     };
-    return this._showBookmarkDialog(info, true);
+    return this.showBookmarkDialog(info);
   },
 
   /**
-   * Opens the properties dialog for a given item identifier.
-   *
-   * @param aItemId
-   *        item identifier for which the properties are to be shown
-   * @param aType
-   *        item type, either "bookmark" or "folder"
-   * @param [optional] aReadOnly
-   *        states if properties dialog should be readonly
-   * @return true if any transaction has been performed.
+   * This is here for compatibility reasons, use ShowBookmarkDialog instead.
    */
   showItemProperties: function PUIU_showItemProperties(aItemId, aType, aReadOnly) {
+    this._reportDeprecatedAddBookmarkMethod();
+
     var info = {
       action: "edit",
       type: aType,
       itemId: aItemId,
       readOnly: aReadOnly
     };
-    return this._showBookmarkDialog(info);
+    return this.showBookmarkDialog(info);
   },
 
   /**
-   * Shows the "New Folder" dialog.
-   *
-   * @param [optional] aTitle
-   *        The default title for the new bookmark.
-   * @param [optional] aDefaultInsertionPoint
-   *        The default insertion point for the new item. If set, the folder
-   *        picker would be hidden unless aShowPicker is set to true, in which
-   *        case the dialog only uses the folder identifier from the insertion
-   *        point as the initially selected item in the folder picker.
-   * @param [optional] aShowPicker
-   *        see above
-   * @return true if any transaction has been performed.
+   * This is here for compatibility reasons, use ShowBookmarkDialog instead.
    */
   showAddFolderUI:
   function PUIU_showAddFolderUI(aTitle, aDefaultInsertionPoint, aShowPicker) {
+    this._reportDeprecatedAddBookmarkMethod();
+
     var info = {
       action: "add",
       type: "folder",
       hiddenRows: []
     };
 
     // allow default empty title
     if (typeof(aTitle) == "string")
       info.title = aTitle;
 
     if (aDefaultInsertionPoint) {
       info.defaultInsertionPoint = aDefaultInsertionPoint;
       if (!aShowPicker)
         info.hiddenRows.push("folderPicker");
     }
-    return this._showBookmarkDialog(info);
+    return this.showBookmarkDialog(info);
   },
 
+
   /**
-   * Shows the bookmark dialog corresponding to the specified info
+   * Shows the bookmark dialog corresponding to the specified info.
    *
    * @param aInfo
    *        Describes the item to be edited/added in the dialog.
    *        See documentation at the top of bookmarkProperties.js
-   * @param aMinimalUI
-   *        [optional] if true, the dialog is opened by its alternative
-   *        chrome: uri.
+   * @param aWindow
+   *        Owner window for the new dialog.
    *
+   * @see documentation at the top of bookmarkProperties.js
    * @return true if any transaction has been performed, false otherwise.
    */
-  _showBookmarkDialog: function PUIU__showBookmarkDialog(aInfo, aMinimalUI) {
-    var dialogURL = aMinimalUI ?
+  showBookmarkDialog:
+  function PUIU_showBookmarkDialog(aInfo, aParentWindow) {
+    if (!aParentWindow) {
+      aParentWindow = this._getWindow(null);
+    }
+
+    // Preserve size attributes differently based on the fact the dialog has
+    // a folder picker or not.
+    let minimalUI = "hiddenRows" in aInfo &&
+                    aInfo.hiddenRows.indexOf("folderPicker") != -1;
+    let dialogURL = minimalUI ?
                     "chrome://browser/content/places/bookmarkProperties2.xul" :
                     "chrome://browser/content/places/bookmarkProperties.xul";
 
-    var features;
-    if (aMinimalUI)
-      features = "centerscreen,chrome,modal,resizable=yes";
-    else
-      features = "centerscreen,chrome,modal,resizable=no";
-    this._getCurrentActiveWin().openDialog(dialogURL, "",  features, aInfo);
+    let features =
+      "centerscreen,chrome,modal,resizable=" + (minimalUI ? "yes" : "no");
+
+    aParentWindow.openDialog(dialogURL, "",  features, aInfo);
     return ("performed" in aInfo && aInfo.performed);
   },
 
   _getTopBrowserWin: function PUIU__getTopBrowserWin() {
     return Services.wm.getMostRecentWindow("navigator:browser");
   },
 
-  _getCurrentActiveWin: function PUIU__getCurrentActiveWin() {
-    return focusManager.activeWindow;
-  },
-
   /**
    * Returns the closet ancestor places view for the given DOM node
    * @param aNode
    *        a DOM node
    * @return the closet ancestor places view if exists, null otherwsie.
    */
   getViewForNode: function PUIU_getViewForNode(aNode) {
     let node = aNode;
@@ -784,17 +705,18 @@ var PlacesUIUtils = {
     if (PlacesUtils.annotations.itemHasAnnotation(aItemId, this.DESCRIPTION_ANNO))
       return PlacesUtils.annotations.getItemAnnotation(aItemId, this.DESCRIPTION_ANNO);
     return "";
   },
 
   /**
    * Gives the user a chance to cancel loading lots of tabs at once
    */
-  _confirmOpenInTabs: function PUIU__confirmOpenInTabs(numTabsToOpen) {
+  _confirmOpenInTabs:
+  function PUIU__confirmOpenInTabs(numTabsToOpen, aWindow) {
     const WARN_ON_OPEN_PREF = "browser.tabs.warnOnOpen";
     var reallyOpen = true;
 
     if (Services.prefs.getBoolPref(WARN_ON_OPEN_PREF)) {
       if (numTabsToOpen >= Services.prefs.getIntPref("browser.tabs.maxOpenBeforeWarn")) {
         // default to true: if it were false, we wouldn't get this far
         var warnOnOpen = { value: true };
 
@@ -802,17 +724,17 @@ var PlacesUIUtils = {
         var openKey = "tabs.openButtonMultiple";
         const BRANDING_BUNDLE_URI = "chrome://branding/locale/brand.properties";
         var brandShortName = Cc["@mozilla.org/intl/stringbundle;1"].
                              getService(Ci.nsIStringBundleService).
                              createBundle(BRANDING_BUNDLE_URI).
                              GetStringFromName("brandShortName");
 
         var buttonPressed = Services.prompt.confirmEx(
-          this._getCurrentActiveWin(),
+          aWindow,
           this.getString("tabs.openWarningTitle"),
           this.getFormattedString(messageKey, [numTabsToOpen, brandShortName]),
           (Services.prompt.BUTTON_TITLE_IS_STRING * Services.prompt.BUTTON_POS_0) +
             (Services.prompt.BUTTON_TITLE_CANCEL * Services.prompt.BUTTON_POS_1),
           this.getString(openKey), null, null,
           this.getFormattedString("tabs.openWarningPromptMeBranded",
                                   [brandShortName]),
           warnOnOpen
@@ -826,106 +748,153 @@ var PlacesUIUtils = {
     }
 
     return reallyOpen;
   },
 
   /** aItemsToOpen needs to be an array of objects of the form:
     * {uri: string, isBookmark: boolean}
     */
-  _openTabset: function PUIU__openTabset(aItemsToOpen, aEvent) {
+  _openTabset: function PUIU__openTabset(aItemsToOpen, aEvent, aWindow) {
     if (!aItemsToOpen.length)
       return;
 
     var urls = [];
     for (var i = 0; i < aItemsToOpen.length; i++) {
       var item = aItemsToOpen[i];
       if (item.isBookmark)
         this.markPageAsFollowedBookmark(item.uri);
       else
         this.markPageAsTyped(item.uri);
 
       urls.push(item.uri);
     }
 
-    var browserWindow = this._getTopBrowserWin();
-    var where = browserWindow ?
-                browserWindow.whereToOpenLink(aEvent, false, true) : "window";
+    var where = aWindow.whereToOpenLink(aEvent, false, true);
     if (where == "window") {
-      let win = this._getCurrentActiveWin();
-      win.openDialog(win.getBrowserURL(), "_blank",
-                     "chrome,all,dialog=no", urls.join("|"));
+      aWindow.openDialog(win.getBrowserURL(), "_blank",
+                             "chrome,all,dialog=no", urls.join("|"));
       return;
     }
 
     var loadInBackground = where == "tabshifted" ? true : false;
     var replaceCurrentTab = where == "tab" ? false : true;
+    var browserWindow = this._getTopBrowserWin();
     browserWindow.gBrowser.loadTabs(urls, loadInBackground, replaceCurrentTab);
   },
 
-  openContainerNodeInTabs: function PUIU_openContainerInTabs(aNode, aEvent) {
-    var urlsToOpen = PlacesUtils.getURLsForContainerNode(aNode);
-    if (!this._confirmOpenInTabs(urlsToOpen.length))
+  /**
+   * Helper method for methods which are forced to take a view/window
+   * parameter as an optional parameter.  It will be removed post Fx4.
+   */
+  _getWindow: function PUIU__getWindow(aView) {
+    if (aView) {
+      // Pratically, this is the case for places trees.
+      if (aView instanceof Components.interfaces.nsIDOMNode)
+        return aView.ownerDocument.defaultView;
+
+      return Cu.getGlobalForObject(aView);
+    }
+
+    let caller = arguments.callee.caller;
+
+    // If a view wasn't expected, the method should have got a window.
+    if (aView === null) {
+      Components.utils.reportError("The api has changed. A window should be \
+                                    passed to " + caller.name + ".  Not \
+                                    passing a window will throw in a future \
+                                    release.");
+    }
+    else {
+      Components.utils.reportError("The api has changed. A places view \
+                                    should be passed to " + caller.name + ".  \
+                                    Not passing a view will throw in a future \
+                                    release.");
+    }
+
+    // This could certainly break in some edge cases (like bug 562998), but
+    // that's the best we should do for those extreme backwards-compatibility cases.
+    let topBrowserWin = this._getTopBrowserWin();
+    return topBrowserWin ? topBrowserWin : focusManager.focusedWindow;
+  },
+
+  openContainerNodeInTabs:
+  function PUIU_openContainerInTabs(aNode, aEvent, aView) {
+    let window = this._getWindow(aView);
+
+    let urlsToOpen = PlacesUtils.getURLsForContainerNode(aNode);
+    if (!this._confirmOpenInTabs(urlsToOpen.length, window))
       return;
 
-    this._openTabset(urlsToOpen, aEvent);
+    this._openTabset(urlsToOpen, aEvent, window);
   },
 
-  openURINodesInTabs: function PUIU_openURINodesInTabs(aNodes, aEvent) {
-    var urlsToOpen = [];
+  openURINodesInTabs: function PUIU_openURINodesInTabs(aNodes, aEvent, aView) {
+    let window = this._getWindow(aView);
+
+    let urlsToOpen = [];
     for (var i=0; i < aNodes.length; i++) {
-      // skip over separators and folders
+      // Skip over separators and folders.
       if (PlacesUtils.nodeIsURI(aNodes[i]))
         urlsToOpen.push({uri: aNodes[i].uri, isBookmark: PlacesUtils.nodeIsBookmark(aNodes[i])});
     }
-    this._openTabset(urlsToOpen, aEvent);
+    this._openTabset(urlsToOpen, aEvent, window);
   },
 
   /**
    * Loads the node's URL in the appropriate tab or window or as a web
    * panel given the user's preference specified by modifier keys tracked by a
    * DOM mouse/key event.
    * @param   aNode
    *          An uri result node.
    * @param   aEvent
    *          The DOM mouse/key event with modifier keys set that track the
    *          user's preferred destination window or tab.
+   * @param   aView
+   *          The controller associated with aNode.
    */
-  openNodeWithEvent: function PUIU_openNodeWithEvent(aNode, aEvent) {
-    this.openNodeIn(aNode, this._getCurrentActiveWin().whereToOpenLink(aEvent));
+  openNodeWithEvent:
+  function PUIU_openNodeWithEvent(aNode, aEvent, aView) {
+    let window = this._getWindow(aView);
+    this._openNodeIn(aNode, window.whereToOpenLink(aEvent), window);
   },
 
   /**
    * Loads the node's URL in the appropriate tab or window or as a
    * web panel.
    * see also openUILinkIn
    */
-  openNodeIn: function PUIU_openNodeIn(aNode, aWhere) {
+  openNodeIn: function PUIU_openNodeIn(aNode, aWhere, aView) {
+    let window = this._getWindow(aView);
+    this._openNodeIn(aNode, aWhere, window);
+  },
+
+  _openNodeIn: function PUIU_openNodeIn(aNode, aWhere, aWindow) {
     if (aNode && PlacesUtils.nodeIsURI(aNode) &&
-        this.checkURLSecurity(aNode, this._getCurrentActiveWin())) {
-      var isBookmark = PlacesUtils.nodeIsBookmark(aNode);
+        this.checkURLSecurity(aNode, aWindow)) {
+      let isBookmark = PlacesUtils.nodeIsBookmark(aNode);
 
       if (isBookmark)
         this.markPageAsFollowedBookmark(aNode.uri);
       else
         this.markPageAsTyped(aNode.uri);
 
       // Check whether the node is a bookmark which should be opened as
       // a web panel
       if (aWhere == "current" && isBookmark) {
         if (PlacesUtils.annotations
                        .itemHasAnnotation(aNode.itemId, this.LOAD_IN_SIDEBAR_ANNO)) {
-          var browserWin = this._getTopBrowserWin();
+          let browserWin = this._getTopBrowserWin();
           if (browserWin) {
             browserWin.openWebPanel(aNode.title, aNode.uri);
             return;
           }
         }
       }
-      this._getCurrentActiveWin().openUILinkIn(aNode.uri, aWhere);
+      aWindow.openUILinkIn(aNode.uri, aWhere);
     }
   },
 
   /**
    * Helper for guessing scheme from an url string.
    * Used to avoid nsIURI overhead in frequently called UI functions.
    *
    * @param aUrlString the url to guess the scheme from.
--- a/browser/components/sidebar/src/nsSidebar.js
+++ b/browser/components/sidebar/src/nsSidebar.js
@@ -99,30 +99,41 @@ function(aTitle, aContentURL, aCustomize
     return this.addPanelInternal(aTitle, aContentURL, aCustomizeURL, true);
 }
 
 nsSidebar.prototype.addPanelInternal =
 function (aTitle, aContentURL, aCustomizeURL, aPersist)
 {
     var WINMEDSVC = Components.classes['@mozilla.org/appshell/window-mediator;1']
                               .getService(Components.interfaces.nsIWindowMediator);
+
+    // XXX Bug 620418: We shouldn't do this anymore. Instead, we should find the
+    // global object for our caller and use it.
     var win = WINMEDSVC.getMostRecentWindow( "navigator:browser" );
-                                                                                
     if (!sidebarURLSecurityCheck(aContentURL))
       return;
 
     var uri = null;
     var ioService = Components.classes["@mozilla.org/network/io-service;1"]
                               .getService(Components.interfaces.nsIIOService);
     try {
       uri = ioService.newURI(aContentURL, null, null);
     }
     catch(ex) { return; }
 
-    win.PlacesUIUtils.showMinimalAddBookmarkUI(uri, aTitle, null, null, true, true);
+    win.PlacesUIUtils.showBookmarkDialog({ action: "add"
+                                         , type: "bookmark"
+                                         , hiddenRows: [ "description"
+                                                       , "keyword"
+                                                       , "location"
+                                                       , "loadInSidebar" ]
+                                         , uri: uri
+                                         , title: aTitle
+                                         , loadBookmarkInSidebar: true
+                                         }, win);
 }
 
 nsSidebar.prototype.validateSearchEngine =
 function (engineURL, iconURL)
 {
   try
   {
     // Make sure the URLs are HTTP, HTTPS, or FTP.