diff --git a/browser/base/content/browser-places.js b/browser/base/content/browser-places.js index 933439406b..977852d4e5 100644 --- a/browser/base/content/browser-places.js +++ b/browser/base/content/browser-places.js @@ -458,7 +458,8 @@ var PlacesCommandHook = { */ showPlacesOrganizer: function PCH_showPlacesOrganizer(aLeftPaneRoot) { var organizer = Services.wm.getMostRecentWindow("Places:Organizer"); - if (!organizer) { + // Due to bug 528706, getMostRecentWindow can return closed windows. + if (!organizer || organizer.closed) { // No currently open places window, so open one with the specified mode. openDialog("chrome://browser/content/places/places.xul", "", "chrome,toolbar=yes,dialog=no,resizable", aLeftPaneRoot); diff --git a/browser/components/places/PlacesUIUtils.jsm b/browser/components/places/PlacesUIUtils.jsm index 968666554f..291c258395 100644 --- a/browser/components/places/PlacesUIUtils.jsm +++ b/browser/components/places/PlacesUIUtils.jsm @@ -18,7 +18,9 @@ XPCOMUtils.defineLazyModuleGetter(this, "PluralForm", XPCOMUtils.defineLazyModuleGetter(this, "PrivateBrowsingUtils", "resource://gre/modules/PrivateBrowsingUtils.jsm"); - +XPCOMUtils.defineLazyModuleGetter(this, "RecentWindow", + "resource:///modules/RecentWindow.jsm"); + XPCOMUtils.defineLazyGetter(this, "PlacesUtils", function() { Cu.import("resource://gre/modules/PlacesUtils.jsm"); return PlacesUtils; @@ -397,7 +399,7 @@ this.PlacesUIUtils = { }, _getTopBrowserWin: function PUIU__getTopBrowserWin() { - return Services.wm.getMostRecentWindow("navigator:browser"); + return RecentWindow.getMostRecentBrowserWindow(); }, /** @@ -542,6 +544,10 @@ this.PlacesUIUtils = { // direct non-bookmark children of bookmark folders. return !PlacesUtils.nodeIsFolder(parentNode); } + + // Generally it's always possible to remove children of a query. + if (PlacesUtils.nodeIsQuery(parentNode)) + return true; // Otherwise it has to be a child of an editable folder. return !this.isContentsReadOnly(parentNode); diff --git a/browser/components/places/content/controller.js b/browser/components/places/content/controller.js index 432289bb2a..331e39354c 100644 --- a/browser/components/places/content/controller.js +++ b/browser/components/places/content/controller.js @@ -130,19 +130,20 @@ PlacesController.prototype = { return PlacesUtils.transactionManager.numberOfRedoItems > 0; case "cmd_cut": case "placesCmd_cut": - var nodes = this._view.selectedNodes; - // If selection includes history nodes there's no reason to allow cut. - for (var i = 0; i < nodes.length; i++) { - if (nodes[i].itemId == -1) + case "placesCmd_moveBookmarks": + for (let node of this._view.selectedNodes) { + // If selection includes history nodes or tags-as-bookmark, disallow + // cutting. + if (node.itemId == -1 || + (node.parent && PlacesUtils.nodeIsTagQuery(node.parent))) { return false; + } } - // Otherwise fallback to cmd_delete check. + // Otherwise fall through to the cmd_delete check. case "cmd_delete": case "placesCmd_delete": case "placesCmd_deleteDataHost": - return this._hasRemovableSelection(false); - case "placesCmd_moveBookmarks": - return this._hasRemovableSelection(true); + return this._hasRemovableSelection(); case "cmd_copy": case "placesCmd_copy": return this._view.hasSelection; @@ -298,7 +299,7 @@ PlacesController.prototype = { * @returns true if all nodes in the selection can be removed, * false otherwise. */ - _hasRemovableSelection: function PC__hasRemovableSelection(aIsMoveCommand) { + _hasRemovableSelection() { var ranges = this._view.removableSelectionRanges; if (!ranges.length) return false; @@ -1249,7 +1250,7 @@ PlacesController.prototype = { * as part of another operation. */ remove: function PC_remove(aTxnName) { - if (!this._hasRemovableSelection(false)) + if (!this._hasRemovableSelection()) return; NS_ASSERT(aTxnName !== undefined, "Must supply Transaction Name");