Bug 1095069 - Cannot copy or delete bookmarks from search results in library. r=mano
authorMarco Bonardo <mbonardo@mozilla.com>
Thu, 13 Nov 2014 10:19:19 +0100
changeset 215580 a3d02e44a476b026226162d7f760a7966efb1d63
parent 215579 45e081b71d891987cad01715e1e1e5c1635aeb17
child 215581 62df790c6e2ed977e51b40b967adfa0576d6025a
push id27821
push userryanvm@gmail.com
push dateThu, 13 Nov 2014 20:41:25 +0000
treeherdermozilla-central@7f0d92595432 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmano
bugs1095069
milestone36.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1095069 - Cannot copy or delete bookmarks from search results in library. r=mano
browser/base/content/browser-places.js
browser/components/places/PlacesUIUtils.jsm
browser/components/places/content/controller.js
browser/components/places/tests/browser/browser.ini
browser/components/places/tests/browser/browser_423515.js
browser/components/places/tests/browser/browser_bookmarksProperties.js
browser/components/places/tests/browser/browser_library_commands.js
browser/components/places/tests/browser/browser_library_infoBox.js
browser/components/places/tests/browser/browser_library_left_pane_commands.js
browser/components/places/tests/browser/head.js
toolkit/components/places/nsNavHistoryResult.cpp
--- a/browser/base/content/browser-places.js
+++ b/browser/base/content/browser-places.js
@@ -469,17 +469,18 @@ var PlacesCommandHook = {
    * Opens the Places Organizer. 
    * @param   aLeftPaneRoot
    *          The query to select in the organizer window - options
    *          are: History, AllBookmarks, BookmarksMenu, BookmarksToolbar,
    *          UnfiledBookmarks, Tags and Downloads.
    */
   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);
     }
     else {
       organizer.PlacesOrganizer.selectLeftPaneContainerByHierarchy(aLeftPaneRoot);
       organizer.focus();
     }
--- a/browser/components/places/PlacesUIUtils.jsm
+++ b/browser/components/places/PlacesUIUtils.jsm
@@ -17,16 +17,18 @@ Cu.import("resource://gre/modules/Places
 XPCOMUtils.defineLazyModuleGetter(this, "PluralForm",
                                   "resource://gre/modules/PluralForm.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "PrivateBrowsingUtils",
                                   "resource://gre/modules/PrivateBrowsingUtils.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "NetUtil",
                                   "resource://gre/modules/NetUtil.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "Task",
                                   "resource://gre/modules/Task.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "RecentWindow",
+                                  "resource:///modules/RecentWindow.jsm");
 
 // PlacesUtils exposes multiple symbols, so we can't use defineLazyModuleGetter.
 Cu.import("resource://gre/modules/PlacesUtils.jsm");
 
 XPCOMUtils.defineLazyModuleGetter(this, "PlacesTransactions",
                                   "resource://gre/modules/PlacesTransactions.jsm");
 
 #ifdef MOZ_SERVICES_CLOUDSYNC
@@ -468,17 +470,17 @@ this.PlacesUIUtils = {
     let features =
       "centerscreen,chrome,modal,resizable=" + (hasFolderPicker ? "yes" : "no");
 
     aParentWindow.openDialog(dialogURL, "",  features, aInfo);
     return ("performed" in aInfo && aInfo.performed);
   },
 
   _getTopBrowserWin: function PUIU__getTopBrowserWin() {
-    return Services.wm.getMostRecentWindow("navigator:browser");
+    return RecentWindow.getMostRecentBrowserWindow();
   },
 
   /**
    * 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.
    */
@@ -614,16 +616,20 @@ this.PlacesUIUtils = {
     // livemark.
     if (aNode.itemId == -1) {
       // Rather than executing a db query, checking the existence of the feedURI
       // annotation, detect livemark children by the fact that they are the only
       // 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);
   },
 
   /**
    * DO NOT USE THIS API IN ADDONS. IT IS VERY LIKELY TO CHANGE WHEN THE SWITCH
    * TO GUIDS IS COMPLETE (BUG 1071511).
    *
--- a/browser/components/places/content/controller.js
+++ b/browser/components/places/content/controller.js
@@ -144,29 +144,30 @@ PlacesController.prototype = {
       return PlacesTransactions.topUndoEntry != null;
     case "cmd_redo":
       if (!PlacesUIUtils.useAsyncTransactions)
         return PlacesUtils.transactionManager.numberOfRedoItems > 0;
 
       return PlacesTransactions.topRedoEntry != null;
     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 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;
     case "cmd_paste":
     case "placesCmd_paste":
       return this._canInsert(true) && this._isClipboardDataPasteable();
     case "cmd_selectAll":
       if (this._view.selType != "single") {
@@ -305,23 +306,21 @@ PlacesController.prototype = {
 
 
   /**
    * Determine whether or not the selection can be removed, either by the
    * delete or cut operations based on whether or not any of its contents
    * are non-removable. We don't need to worry about recursion here since it
    * is a policy decision that a removable item not be placed inside a non-
    * removable item.
-   * @param aIsMoveCommand
-   *        True if the command for which this method is called only moves the
-   *        selected items to another container, false otherwise.
+   *
    * @return 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;
 
     var root = this._view.result.root;
 
     for (var j = 0; j < ranges.length; j++) {
       var nodes = ranges[j];
@@ -1019,17 +1018,17 @@ PlacesController.prototype = {
 
   /**
    * Removes the selection
    * @param   aTxnName
    *          A name for the transaction if this is being performed
    *          as part of another operation.
    */
   remove: Task.async(function* (aTxnName) {
-    if (!this._hasRemovableSelection(false))
+    if (!this._hasRemovableSelection())
       return;
 
     NS_ASSERT(aTxnName !== undefined, "Must supply Transaction Name");
 
     var root = this._view.result.root;
 
     if (PlacesUtils.nodeIsFolder(root)) {
       if (PlacesUIUtils.useAsyncTransactions)
@@ -1540,17 +1539,17 @@ let PlacesControllerDragHelper = {
    *
    * @param   aUnwrappedNode
    *          A node unwrapped by PlacesUtils.unwrapNodes().
    * @return True if the node can be moved, false otherwise.
    */
   canMoveUnwrappedNode: function (aUnwrappedNode) {
     return aUnwrappedNode.id > 0 &&
            !PlacesUtils.isRootItem(aUnwrappedNode.id) &&
-           !PlacesUIUtils.isContentsReadOnly(aUnwrappedNode.parent) ||
+           (!aUnwrappedNode.parent || !PlacesUIUtils.isContentsReadOnly(aUnwrappedNode.parent)) &&
            aUnwrappedNode.parent != PlacesUtils.tagsFolderId &&
            aUnwrappedNode.grandParentId != PlacesUtils.tagsFolderId;
   },
 
   /**
    * Determines if a node can be moved.
    *
    * @param   aNode
--- a/browser/components/places/tests/browser/browser.ini
+++ b/browser/components/places/tests/browser/browser.ini
@@ -26,17 +26,17 @@ skip-if = e10s # Bug ?????? - clipboard 
 [browser_history_sidebar_search.js]
 [browser_bookmarksProperties.js]
 skip-if = e10s
 
 [browser_forgetthissite_single.js]
 # disabled for very frequent oranges - bug 551540
 skip-if = true
 
-[browser_library_left_pane_commands.js]
+[browser_library_commands.js]
 [browser_drag_bookmarks_on_toolbar.js]
 skip-if = e10s # Bug ?????? - test fails - "Number of dragged items should be the same. - Got 0, expected 1"
 [browser_library_middleclick.js]
 [browser_library_views_liveupdate.js]
 [browser_views_liveupdate.js]
 
 [browser_sidebarpanels_click.js]
 # temporarily disabled for breaking the treeview - bug 658744
--- a/browser/components/places/tests/browser/browser_423515.js
+++ b/browser/components/places/tests/browser/browser_423515.js
@@ -137,16 +137,17 @@ function test() {
   });
 
   // test that a tag container cannot be moved
   tests.push({
     populate: function() {
       // tag a uri
       this.uri = makeURI("http://foo.com");
       PlacesUtils.tagging.tagURI(this.uri, ["bar"]);
+      registerCleanupFunction(() => PlacesUtils.tagging.untagURI(this.uri, ["bar"]));
     },
     validate: function() {
       // get tag root
       var query = PlacesUtils.history.getNewQuery();
       var options = PlacesUtils.history.getNewQueryOptions();
       options.resultType = Ci.nsINavHistoryQueryOptions.RESULTS_AS_TAG_QUERY;
       var tagsNode = PlacesUtils.history.executeQuery(query, options).root;
 
--- a/browser/components/places/tests/browser/browser_bookmarksProperties.js
+++ b/browser/components/places/tests/browser/browser_bookmarksProperties.js
@@ -29,19 +29,18 @@ const ACTION_ADD = 1;
 const TYPE_FOLDER = 0;
 const TYPE_BOOKMARK = 1;
 
 const TEST_URL = "http://www.example.com/";
 
 const DIALOG_URL = "chrome://browser/content/places/bookmarkProperties.xul";
 const DIALOG_URL_MINIMAL_UI = "chrome://browser/content/places/bookmarkProperties2.xul";
 
-var wm = Cc["@mozilla.org/appshell/window-mediator;1"].
-         getService(Ci.nsIWindowMediator);
-var win = wm.getMostRecentWindow("navigator:browser");
+Cu.import("resource:///modules/RecentWindow.jsm");
+let win = RecentWindow.getMostRecentBrowserWindow();
 var ww = Cc["@mozilla.org/embedcomp/window-watcher;1"].
          getService(Ci.nsIWindowWatcher);
 
 function add_bookmark(aURI) {
   var bId = PlacesUtils.bookmarks
                        .insertBookmark(PlacesUtils.unfiledBookmarksFolderId,
                                        aURI,
                                        PlacesUtils.bookmarks.DEFAULT_INDEX,
rename from browser/components/places/tests/browser/browser_library_left_pane_commands.js
rename to browser/components/places/tests/browser/browser_library_commands.js
--- a/browser/components/places/tests/browser/browser_library_left_pane_commands.js
+++ b/browser/components/places/tests/browser/browser_library_commands.js
@@ -3,154 +3,233 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 /**
  *  Test enabled commands in the left pane folder of the Library.
  */
 
-const TEST_URI = "http://www.mozilla.org/";
+const TEST_URI = NetUtil.newURI("http://www.mozilla.org/");
 
-var gTests = [];
-var gLibrary;
+registerCleanupFunction(function* () {
+  yield PlacesUtils.bookmarks.eraseEverything();
+  yield promiseClearHistory();
+});
 
-//------------------------------------------------------------------------------
+add_task(function* test_date_container() {
+  let library = yield promiseLibrary();
+  info("Ensure date containers under History cannot be cut but can be deleted");
+
+  yield promiseAddVisits(TEST_URI);
 
-gTests.push({
-  desc: "Bug 489351 - Date containers under History in Library cannot be deleted/cut",
-  run: function() {
-    function addVisitsCallback() {
-      // Select and open the left pane "History" query.
-      var PO = gLibrary.PlacesOrganizer;
-      PO.selectLeftPaneQuery('History');
-      isnot(PO._places.selectedNode, null, "We correctly selected History");
+  // Select and open the left pane "History" query.
+  let PO = library.PlacesOrganizer;
+
+  PO.selectLeftPaneQuery('History');
+  isnot(PO._places.selectedNode, null, "We correctly selected History");
 
-      // Check that both delete and cut commands are disabled.
-      ok(!PO._places.controller.isCommandEnabled("cmd_cut"),
-         "Cut command is disabled");
-      ok(!PO._places.controller.isCommandEnabled("cmd_delete"),
-         "Delete command is disabled");
-      var historyNode = PO._places.selectedNode
-                          .QueryInterface(Ci.nsINavHistoryContainerResultNode);
-      historyNode.containerOpen = true;
+  // Check that both delete and cut commands are disabled, cause this is
+  // a child of the left pane folder.
+  ok(PO._places.controller.isCommandEnabled("cmd_copy"),
+     "Copy command is enabled");
+  ok(!PO._places.controller.isCommandEnabled("cmd_cut"),
+     "Cut command is disabled");
+  ok(!PO._places.controller.isCommandEnabled("cmd_delete"),
+     "Delete command is disabled");
+  let historyNode = PlacesUtils.asContainer(PO._places.selectedNode);
+  historyNode.containerOpen = true;
 
-      // Check that we have a child container. It is "Today" container.
-      is(historyNode.childCount, 1, "History node has one child");
-      var todayNode = historyNode.getChild(0);
-      var todayNodeExpectedTitle = PlacesUtils.getString("finduri-AgeInDays-is-0");
-      is(todayNode.title, todayNodeExpectedTitle,
-         "History child is the expected container");
-
-      // Select "Today" container.
-      PO._places.selectNode(todayNode);
-      is(PO._places.selectedNode, todayNode,
-         "We correctly selected Today container");
-      // Check that delete command is enabled but cut command is disabled.
-      ok(!PO._places.controller.isCommandEnabled("cmd_cut"),
-         "Cut command is disabled");
-      ok(PO._places.controller.isCommandEnabled("cmd_delete"),
-         "Delete command is enabled");
+  // Check that we have a child container. It is "Today" container.
+  is(historyNode.childCount, 1, "History node has one child");
+  let todayNode = historyNode.getChild(0);
+  let todayNodeExpectedTitle = PlacesUtils.getString("finduri-AgeInDays-is-0");
+  is(todayNode.title, todayNodeExpectedTitle,
+     "History child is the expected container");
 
-      // Execute the delete command and check visit has been removed.
-      PO._places.controller.doCommand("cmd_delete");
-
-      // Test live update of "History" query.
-      is(historyNode.childCount, 0, "History node has no more children");
-
-      historyNode.containerOpen = false;
+  // Select "Today" container.
+  PO._places.selectNode(todayNode);
+  is(PO._places.selectedNode, todayNode,
+     "We correctly selected Today container");
+  // Check that delete command is enabled but cut command is disabled, cause
+  // this is an history item.
+  ok(PO._places.controller.isCommandEnabled("cmd_copy"),
+     "Copy command is enabled");
+  ok(!PO._places.controller.isCommandEnabled("cmd_cut"),
+     "Cut command is disabled");
+  ok(PO._places.controller.isCommandEnabled("cmd_delete"),
+     "Delete command is enabled");
 
-      let testURI = NetUtil.newURI(TEST_URI);
-      PlacesUtils.asyncHistory.isURIVisited(testURI, function(aURI, aIsVisited) {
-        ok(!aIsVisited, "Visit has been removed");
-        nextTest();
-      });
-    }
-    addVisits(
-      {uri: NetUtil.newURI(TEST_URI), visitDate: Date.now() * 1000,
-        transition: PlacesUtils.history.TRANSITION_TYPED},
-      window,
-      addVisitsCallback);
-  }
+  // Execute the delete command and check visit has been removed.
+  let promiseURIRemoved = promiseHistoryNotification("onDeleteURI",
+                                                     () => TEST_URI.equals(arguments[0]));
+  PO._places.controller.doCommand("cmd_delete");
+  yield promiseURIRemoved;
+
+  // Test live update of "History" query.
+  is(historyNode.childCount, 0, "History node has no more children");
+
+  historyNode.containerOpen = false;
+
+  ok(!(yield promiseIsURIVisited(TEST_URI)), "Visit has been removed");
+
+  library.close();
 });
 
-//------------------------------------------------------------------------------
+add_task(function* test_query_on_toolbar() {
+  let library = yield promiseLibrary();
+  info("Ensure queries can be cut or deleted");
+
+  // Select and open the left pane "Bookmarks Toolbar" folder.
+  let PO = library.PlacesOrganizer;
 
-gTests.push({
-  desc: "Bug 490156 - Can't delete smart bookmark containers",
-  run: function() {
-    // Select and open the left pane "Bookmarks Toolbar" folder.
-    var PO = gLibrary.PlacesOrganizer;
-    PO.selectLeftPaneQuery('BookmarksToolbar');
-    isnot(PO._places.selectedNode, null, "We have a valid selection");
-    is(PlacesUtils.getConcreteItemId(PO._places.selectedNode),
-       PlacesUtils.toolbarFolderId,
-       "We have correctly selected bookmarks toolbar node.");
+  PO.selectLeftPaneQuery('BookmarksToolbar');
+  isnot(PO._places.selectedNode, null, "We have a valid selection");
+  is(PlacesUtils.getConcreteItemId(PO._places.selectedNode),
+     PlacesUtils.toolbarFolderId,
+     "We have correctly selected bookmarks toolbar node.");
 
-    // Check that both cut and delete commands are disabled.
-    ok(!PO._places.controller.isCommandEnabled("cmd_cut"),
-       "Cut command is disabled");
-    ok(!PO._places.controller.isCommandEnabled("cmd_delete"),
-       "Delete command is disabled");
+  // Check that both cut and delete commands are disabled, cause this is a child
+  // of AllBookmarksFolderId.
+  ok(PO._places.controller.isCommandEnabled("cmd_copy"),
+     "Copy command is enabled");
+  ok(!PO._places.controller.isCommandEnabled("cmd_cut"),
+     "Cut command is disabled");
+  ok(!PO._places.controller.isCommandEnabled("cmd_delete"),
+     "Delete command is disabled");
 
-    var toolbarNode = PO._places.selectedNode
-                        .QueryInterface(Ci.nsINavHistoryContainerResultNode);
-    toolbarNode.containerOpen = true;
+  let toolbarNode = PlacesUtils.asContainer(PO._places.selectedNode);
+  toolbarNode.containerOpen = true;
 
-    // Add an History query to the toolbar.
-    PlacesUtils.bookmarks.insertBookmark(PlacesUtils.toolbarFolderId,
-                                         NetUtil.newURI("place:sort=4"),
-                                         0, // Insert at start.
-                                         "special_query");
-    // Get first child and check it is the "Most Visited" smart bookmark.
-    ok(toolbarNode.childCount > 0, "Toolbar node has children");
-    var queryNode = toolbarNode.getChild(0);
-    is(queryNode.title, "special_query", "Query node is correctly selected");
+  // Add an History query to the toolbar.
+  let query = yield PlacesUtils.bookmarks.insert({ type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
+                                                   url: "place:sort=4",
+                                                   title: "special_query",
+                                                   parentGuid: PlacesUtils.bookmarks.toolbarGuid,
+                                                   index: 0 });
 
-    // Select query node.
-    PO._places.selectNode(queryNode);
-    is(PO._places.selectedNode, queryNode, "We correctly selected query node");
+  // Get first child and check it is the just inserted query.
+  ok(toolbarNode.childCount > 0, "Toolbar node has children");
+  let queryNode = toolbarNode.getChild(0);
+  is(queryNode.title, "special_query", "Query node is correctly selected");
+
+  // Select query node.
+  PO._places.selectNode(queryNode);
+  is(PO._places.selectedNode, queryNode, "We correctly selected query node");
 
-    // Check that both cut and delete commands are enabled.
-    ok(PO._places.controller.isCommandEnabled("cmd_cut"),
-       "Cut command is enabled");
-    ok(PO._places.controller.isCommandEnabled("cmd_delete"),
-       "Delete command is enabled");
+  // Check that both cut and delete commands are enabled.
+  ok(PO._places.controller.isCommandEnabled("cmd_copy"),
+     "Copy command is enabled");
+  ok(PO._places.controller.isCommandEnabled("cmd_cut"),
+     "Cut command is enabled");
+  ok(PO._places.controller.isCommandEnabled("cmd_delete"),
+     "Delete command is enabled");
 
-    // Execute the delete command and check bookmark has been removed.
-    PO._places.controller.doCommand("cmd_delete");
-    try {
-      PlacesUtils.bookmarks.getFolderIdForItem(queryNode.itemId);  
-      ok(false, "Unable to remove query node bookmark");
-    } catch(ex) {
-      ok(true, "Query node bookmark has been correctly removed");
-    }
+  // Execute the delete command and check bookmark has been removed.
+  let promiseItemRemoved = promiseBookmarksNotification("onItemRemoved",
+                                                        () => query.guid == arguments[5]);
+  PO._places.controller.doCommand("cmd_delete");
+  yield promiseItemRemoved;
 
-    toolbarNode.containerOpen = false;
-    nextTest();
-  }
+  is((yield PlacesUtils.bookmarks.fetch(query.guid)), null,
+     "Query node bookmark has been correctly removed");
+
+  toolbarNode.containerOpen = false;
+
+  library.close();
 });
 
-//------------------------------------------------------------------------------
+add_task(function* test_search_contents() {
+  let item = yield PlacesUtils.bookmarks.insert({ type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
+                                                  url: "http://example.com/",
+                                                  title: "example page",
+                                                  parentGuid: PlacesUtils.bookmarks.unfiledGuid,
+                                                  index: 0 });
+
+  let library = yield promiseLibrary();
+  info("Ensure query contents can be cut or deleted");
+
+  // Select and open the left pane "Bookmarks Toolbar" folder.
+  let PO = library.PlacesOrganizer;
+
+  PO.selectLeftPaneQuery('BookmarksToolbar');
+  isnot(PO._places.selectedNode, null, "We have a valid selection");
+  is(PlacesUtils.getConcreteItemId(PO._places.selectedNode),
+     PlacesUtils.toolbarFolderId,
+     "We have correctly selected bookmarks toolbar node.");
+
+  let searchBox = library.document.getElementById("searchFilter");
+  searchBox.value = "example";
+  library.PlacesSearchBox.search(searchBox.value);
+
+  let bookmarkNode = library.ContentTree.view.selectedNode;
+  is(bookmarkNode.uri, "http://example.com/", "Found the expected bookmark");
+
+  // Check that both cut and delete commands are enabled.
+  ok(library.ContentTree.view.controller.isCommandEnabled("cmd_copy"),
+     "Copy command is enabled");
+  ok(library.ContentTree.view.controller.isCommandEnabled("cmd_cut"),
+     "Cut command is enabled");
+  ok(library.ContentTree.view.controller.isCommandEnabled("cmd_delete"),
+     "Delete command is enabled");
+
+  library.close();
+});
+
+add_task(function* test_tags() {
+  let item = yield PlacesUtils.bookmarks.insert({ type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
+                                                  url: "http://example.com/",
+                                                  title: "example page",
+                                                  parentGuid: PlacesUtils.bookmarks.unfiledGuid,
+                                                  index: 0 });
+  PlacesUtils.tagging.tagURI(NetUtil.newURI("http://example.com/"), ["test"]);
+
+  let library = yield promiseLibrary();
+  info("Ensure query contents can be cut or deleted");
 
-function nextTest() {
-  if (gTests.length) {
-    var test = gTests.shift();
-    info("Start of test: " + test.desc);
-    test.run();
-  }
-  else {
-    // Close Library window.
-    gLibrary.close();
-    // No need to cleanup anything, we have a correct left pane now.
-    finish();
-  }
-}
+  // Select and open the left pane "Bookmarks Toolbar" folder.
+  let PO = library.PlacesOrganizer;
+
+  PO.selectLeftPaneQuery('Tags');
+  let tagsNode = PO._places.selectedNode;
+  isnot(tagsNode, null, "We have a valid selection");
+  let tagsTitle = PlacesUtils.getString("TagsFolderTitle");
+  is(tagsNode.title, tagsTitle,
+     "Tags has been properly selected");
+
+  // Check that both cut and delete commands are disabled.
+  ok(PO._places.controller.isCommandEnabled("cmd_copy"),
+     "Copy command is enabled");
+  ok(!PO._places.controller.isCommandEnabled("cmd_cut"),
+     "Cut command is disabled");
+  ok(!PO._places.controller.isCommandEnabled("cmd_delete"),
+     "Delete command is disabled");
 
-function test() {
-  waitForExplicitFinish();
-  // Sanity checks.
-  ok(PlacesUtils, "PlacesUtils is running in chrome context");
-  ok(PlacesUIUtils, "PlacesUIUtils is running in chrome context");
+  // Now select the tag.
+  PlacesUtils.asContainer(tagsNode).containerOpen = true;
+  let tag = tagsNode.getChild(0);
+  PO._places.selectNode(tag);
+  is(PO._places.selectedNode.title, "test",
+     "The created tag has been properly selected");
+
+  // Check that cut is disabled but delete is enabled.
+  ok(PO._places.controller.isCommandEnabled("cmd_copy"),
+     "Copy command is enabled");
+  ok(!PO._places.controller.isCommandEnabled("cmd_cut"),
+     "Cut command is disabled");
+  ok(PO._places.controller.isCommandEnabled("cmd_delete"),
+     "Delete command is enabled");
 
-  // Open Library.
-  gLibrary = openLibrary(nextTest);
-}
+  let bookmarkNode = library.ContentTree.view.selectedNode;
+  is(bookmarkNode.uri, "http://example.com/", "Found the expected bookmark");
+
+  // Check that both cut and delete commands are enabled.
+  ok(library.ContentTree.view.controller.isCommandEnabled("cmd_copy"),
+     "Copy command is enabled");
+  ok(!library.ContentTree.view.controller.isCommandEnabled("cmd_cut"),
+     "Cut command is disabled");
+  ok(library.ContentTree.view.controller.isCommandEnabled("cmd_delete"),
+     "Delete command is enabled");
+
+  tagsNode.containerOpen = false;
+
+  library.close();
+});
--- a/browser/components/places/tests/browser/browser_library_infoBox.js
+++ b/browser/components/places/tests/browser/browser_library_infoBox.js
@@ -66,26 +66,34 @@ gTests.push({
       isnot(PO._places.selectedNode, null,
             "Correctly selected bookmarks menu node.");
       checkInfoBoxSelected(PO);
       ok(infoBoxExpanderWrapper.hidden,
          "Expander button is hidden for bookmarks menu node.");
       checkAddInfoFieldsCollapsed(PO);
 
       // open recently bookmarked node
+      PlacesUtils.bookmarks.insertBookmark(PlacesUtils.bookmarksMenuFolderId,
+                                           NetUtil.newURI("place:folder=BOOKMARKS_MENU" +
+                                                          "&folder=UNFILED_BOOKMARKS" +
+                                                          "&folder=TOOLBAR" +
+                                                          "&queryType=" + Ci.nsINavHistoryQueryOptions.QUERY_TYPE_BOOKMARKS +
+                                                          "&sort=" + Ci.nsINavHistoryQueryOptions.SORT_BY_DATEADDED_DESCENDING +
+                                                          "&maxResults=10" +
+                                                          "&excludeQueries=1"),
+                                           0, "Recent Bookmarks");
+      PlacesUtils.bookmarks.insertBookmark(PlacesUtils.bookmarksMenuFolderId,
+                                           NetUtil.newURI("http://mozilla.org/"),
+                                           1, "Mozilla");
       var menuNode = PO._places.selectedNode.
                      QueryInterface(Ci.nsINavHistoryContainerResultNode);
       menuNode.containerOpen = true;
       childNode = menuNode.getChild(0);
       isnot(childNode, null, "Bookmarks menu child node exists.");
-      var recentlyBookmarkedTitle = PlacesUIUtils.
-                                    getString("recentlyBookmarkedTitle");
-      isnot(recentlyBookmarkedTitle, null,
-            "Correctly got the recently bookmarked title locale string.");
-      is(childNode.title, recentlyBookmarkedTitle,
+      is(childNode.title, "Recent Bookmarks",
          "Correctly selected recently bookmarked node.");
       PO._places.selectNode(childNode);
       checkInfoBoxSelected(PO);
       ok(!infoBoxExpanderWrapper.hidden,
          "Expander button is not hidden for recently bookmarked node.");
       checkAddInfoFieldsNotCollapsed(PO);
 
       // open first bookmark
@@ -93,25 +101,16 @@ gTests.push({
       ok(view.rowCount > 0, "Bookmark item exists.");
       view.selection.select(0);
       checkInfoBoxSelected(PO);
       ok(!infoBoxExpanderWrapper.hidden,
          "Expander button is not hidden for bookmark item.");
       checkAddInfoFieldsNotCollapsed(PO);
       checkAddInfoFields(PO, "bookmark item");
 
-      // make sure additional fields are still hidden in second bookmark item
-      ok(view.rowCount > 1, "Second bookmark item exists.");
-      view.selection.select(1);
-      checkInfoBoxSelected(PO);
-      ok(!infoBoxExpanderWrapper.hidden,
-         "Expander button is not hidden for second bookmark item.");
-      checkAddInfoFieldsNotCollapsed(PO);
-      checkAddInfoFields(PO, "second bookmark item");
-
       menuNode.containerOpen = false;
 
       waitForClearHistory(nextTest);
     }
     // add a visit to browser history
     addVisits(
       { uri: PlacesUtils._uri(TEST_URI), visitDate: Date.now() * 1000,
         transition: PlacesUtils.history.TRANSITION_TYPED },
--- a/browser/components/places/tests/browser/head.js
+++ b/browser/components/places/tests/browser/head.js
@@ -38,17 +38,17 @@ function openLibrary(callback, aLeftPane
  * If one is opens returns itm otherwise it opens a new one.
  *
  * @param aLeftPaneRoot
  *        Hierarchy to open and select in the left pane.
  */
 function promiseLibrary(aLeftPaneRoot) {
   let deferred = Promise.defer();
   let library = Services.wm.getMostRecentWindow("Places:Organizer");
-  if (library) {
+  if (library && !library.closed) {
     if (aLeftPaneRoot)
       library.PlacesOrganizer.selectLeftPaneContainerByHierarchy(aLeftPaneRoot);
     deferred.resolve(library);
   }
   else {
     openLibrary(aLibrary => deferred.resolve(aLibrary), aLeftPaneRoot);
   }
   return deferred.promise;
@@ -159,17 +159,17 @@ function addVisits(aPlaceInfo, aWindow, 
 
   // Create mozIVisitInfo for each entry.
   let now = Date.now();
   for (let i = 0; i < places.length; i++) {
     if (!places[i].title) {
       places[i].title = "test visit for " + places[i].uri.spec;
     }
     places[i].visits = [{
-      transitionType: places[i].transition === undefined ? Ci.nsINavHistoryService.TRANSITION_LINK
+      transitionType: places[i].transition === undefined ? PlacesUtils.history.TRANSITION_LINK
                                                          : places[i].transition,
       visitDate: places[i].visitDate || (now++) * 1000,
       referrerURI: places[i].referrer
     }];
   }
 
   aWindow.PlacesUtils.asyncHistory.updatePlaces(
     places,
@@ -198,8 +198,211 @@ function synthesizeClickOnSelectedTreeCe
   // Calculate the click coordinates.
   var rect = tbo.getCoordsForCellItem(rowID, aTree.columns[0], "text");
   var x = rect.x + rect.width / 2;
   var y = rect.y + rect.height / 2;
   // Simulate the click.
   EventUtils.synthesizeMouse(aTree.body, x, y, aOptions || {},
                              aTree.ownerDocument.defaultView);
 }
+
+/**
+ * Asynchronously adds visits to a page.
+ *
+ * @param aPlaceInfo
+ *        Can be an nsIURI, in such a case a single LINK visit will be added.
+ *        Otherwise can be an object describing the visit to add, or an array
+ *        of these objects:
+ *          { uri: nsIURI of the page,
+ *            transition: one of the TRANSITION_* from nsINavHistoryService,
+ *            [optional] title: title of the page,
+ *            [optional] visitDate: visit date in microseconds from the epoch
+ *            [optional] referrer: nsIURI of the referrer for this visit
+ *          }
+ *
+ * @return {Promise}
+ * @resolves When all visits have been added successfully.
+ * @rejects JavaScript exception.
+ */
+function promiseAddVisits(aPlaceInfo)
+{
+  let deferred = Promise.defer();
+  let places = [];
+  if (aPlaceInfo instanceof Ci.nsIURI) {
+    places.push({ uri: aPlaceInfo });
+  }
+  else if (Array.isArray(aPlaceInfo)) {
+    places = places.concat(aPlaceInfo);
+  } else {
+    places.push(aPlaceInfo)
+  }
+
+  // Create mozIVisitInfo for each entry.
+  let now = Date.now();
+  for (let i = 0; i < places.length; i++) {
+    if (!places[i].title) {
+      places[i].title = "test visit for " + places[i].uri.spec;
+    }
+    places[i].visits = [{
+      transitionType: places[i].transition === undefined ? PlacesUtils.history.TRANSITION_LINK
+                                                         : places[i].transition,
+      visitDate: places[i].visitDate || (now++) * 1000,
+      referrerURI: places[i].referrer
+    }];
+  }
+
+  PlacesUtils.asyncHistory.updatePlaces(
+    places,
+    {
+      handleError: function AAV_handleError(aResultCode, aPlaceInfo) {
+        let ex = new Components.Exception("Unexpected error in adding visits.",
+                                          aResultCode);
+        deferred.reject(ex);
+      },
+      handleResult: function () {},
+      handleCompletion: function UP_handleCompletion() {
+        deferred.resolve();
+      }
+    }
+  );
+
+  return deferred.promise;
+}
+
+/**
+ * Asynchronously check a url is visited.
+ *
+ * @param aURI The URI.
+ * @return {Promise}
+ * @resolves When the check has been added successfully.
+ * @rejects JavaScript exception.
+ */
+function promiseIsURIVisited(aURI) {
+  let deferred = Promise.defer();
+
+  PlacesUtils.asyncHistory.isURIVisited(aURI, function(aURI, aIsVisited) {
+    deferred.resolve(aIsVisited);
+  });
+
+  return deferred.promise;
+}
+
+/**
+ * Waits for all pending async statements on the default connection.
+ *
+ * @return {Promise}
+ * @resolves When all pending async statements finished.
+ * @rejects Never.
+ *
+ * @note The result is achieved by asynchronously executing a query requiring
+ *       a write lock.  Since all statements on the same connection are
+ *       serialized, the end of this write operation means that all writes are
+ *       complete.  Note that WAL makes so that writers don't block readers, but
+ *       this is a problem only across different connections.
+ */
+function promiseAsyncUpdates()
+{
+  let deferred = Promise.defer();
+
+  let db = DBConn();
+  let begin = db.createAsyncStatement("BEGIN EXCLUSIVE");
+  begin.executeAsync();
+  begin.finalize();
+
+  let commit = db.createAsyncStatement("COMMIT");
+  commit.executeAsync({
+    handleResult: function () {},
+    handleError: function () {},
+    handleCompletion: function(aReason)
+    {
+      deferred.resolve();
+    }
+  });
+  commit.finalize();
+
+  return deferred.promise;
+}
+
+function promiseBookmarksNotification(notification, conditionFn) {
+  info(`Waiting for ${notification}`);
+  return new Promise((resolve, reject) => {
+    let proxifiedObserver = new Proxy({}, {
+      get: (target, name) => {
+        if (name == "QueryInterface")
+          return XPCOMUtils.generateQI([ Ci.nsINavBookmarkObserver ]);
+        if (name == notification)
+          return () => {
+            if (conditionFn.apply(this, arguments)) {
+              clearTimeout(timeout);
+              PlacesUtils.bookmarks.removeObserver(proxifiedObserver, false);
+              executeSoon(resolve);
+            }
+          }
+        return () => {};
+      }
+    });
+    PlacesUtils.bookmarks.addObserver(proxifiedObserver, false);
+    let timeout = setTimeout(() => {
+      PlacesUtils.bookmarks.removeObserver(proxifiedObserver, false);
+      reject(new Error("Timed out while waiting for bookmarks notification"));
+    }, 2000);
+  });
+}
+
+function promiseHistoryNotification(notification, conditionFn) {
+  info(`Waiting for ${notification}`);
+  return new Promise((resolve, reject) => {
+    let proxifiedObserver = new Proxy({}, {
+      get: (target, name) => {
+        if (name == "QueryInterface")
+          return XPCOMUtils.generateQI([ Ci.nsINavHistoryObserver ]);
+        if (name == notification)
+          return () => {
+            if (conditionFn.apply(this, arguments)) {
+              clearTimeout(timeout);
+              PlacesUtils.history.removeObserver(proxifiedObserver, false);
+              executeSoon(resolve);
+            }
+          }
+        return () => {};
+      }
+    });
+    PlacesUtils.history.addObserver(proxifiedObserver, false);
+    let timeout = setTimeout(() => {
+      PlacesUtils.history.removeObserver(proxifiedObserver, false);
+      reject(new Error("Timed out while waiting for history notification"));
+    }, 2000);
+  });
+}
+
+/**
+ * Clears history asynchronously.
+ *
+ * @return {Promise}
+ * @resolves When history has been cleared.
+ * @rejects Never.
+ */
+function promiseClearHistory() {
+  let promise = promiseTopicObserved(PlacesUtils.TOPIC_EXPIRATION_FINISHED);
+  PlacesUtils.bhistory.removeAllPages();
+  return promise;
+}
+
+/**
+ * Allows waiting for an observer notification once.
+ *
+ * @param topic
+ *        Notification topic to observe.
+ *
+ * @return {Promise}
+ * @resolves The array [subject, data] from the observed notification.
+ * @rejects Never.
+ */
+function promiseTopicObserved(topic)
+{
+  let deferred = Promise.defer();
+  info("Waiting for observer topic " + topic);
+  Services.obs.addObserver(function PTO_observe(subject, topic, data) {
+    Services.obs.removeObserver(PTO_observe, topic);
+    deferred.resolve([subject, data]);
+  }, topic, false);
+  return deferred.promise;
+}
--- a/toolkit/components/places/nsNavHistoryResult.cpp
+++ b/toolkit/components/places/nsNavHistoryResult.cpp
@@ -3503,36 +3503,48 @@ nsNavHistoryFolderResultNode::OnItemAdde
                                           nsIURI* aURI,
                                           const nsACString& aTitle,
                                           PRTime aDateAdded,
                                           const nsACString& aGUID,
                                           const nsACString& aParentGUID)
 {
   NS_ASSERTION(aParentFolder == mItemId, "Got wrong bookmark update");
 
+  RESTART_AND_RETURN_IF_ASYNC_PENDING();
+
+  {
+    uint32_t index;
+    nsNavHistoryResultNode* node = FindChildById(aItemId, &index);
+    // Bug 1097528.
+    // It's possible our result registered due to a previous notification, for
+    // example the Library left pane could have refreshed and replaced the
+    // right pane as a consequence. In such a case our contents are already
+    // up-to-date.  That's OK.
+    if (node)
+      return NS_OK;
+  }
+
   bool excludeItems = (mResult && mResult->mRootNode->mOptions->ExcludeItems()) ||
-                        (mParent && mParent->mOptions->ExcludeItems()) ||
-                        mOptions->ExcludeItems();
+                      (mParent && mParent->mOptions->ExcludeItems()) ||
+                      mOptions->ExcludeItems();
 
   // here, try to do something reasonable if the bookmark service gives us
   // a bogus index.
   if (aIndex < 0) {
     NS_NOTREACHED("Invalid index for item adding: <0");
     aIndex = 0;
   }
   else if (aIndex > mChildren.Count()) {
     if (!excludeItems) {
       // Something wrong happened while updating indexes.
       NS_NOTREACHED("Invalid index for item adding: greater than count");
     }
     aIndex = mChildren.Count();
   }
 
-  RESTART_AND_RETURN_IF_ASYNC_PENDING();
-
   nsresult rv;
 
   // Check for query URIs, which are bookmarks, but treated as containers
   // in results and views.
   bool isQuery = false;
   if (aItemType == nsINavBookmarksService::TYPE_BOOKMARK) {
     NS_ASSERTION(aURI, "Got a null URI when we are a bookmark?!");
     nsAutoCString itemURISpec;
@@ -3601,33 +3613,33 @@ nsNavHistoryFolderResultNode::OnItemRemo
   // its list.
   if (mItemId == aItemId)
     return NS_OK;
 
   NS_ASSERTION(aParentFolder == mItemId, "Got wrong bookmark update");
 
   RESTART_AND_RETURN_IF_ASYNC_PENDING();
 
-  bool excludeItems = (mResult && mResult->mRootNode->mOptions->ExcludeItems()) ||
-                        (mParent && mParent->mOptions->ExcludeItems()) ||
-                        mOptions->ExcludeItems();
-
   // don't trust the index from the bookmark service, find it ourselves.  The
   // sorting could be different, or the bookmark services indices and ours might
   // be out of sync somehow.
   uint32_t index;
   nsNavHistoryResultNode* node = FindChildById(aItemId, &index);
+    // Bug 1097528.
+    // It's possible our result registered due to a previous notification, for
+    // example the Library left pane could have refreshed and replaced the
+    // right pane as a consequence. In such a case our contents are already
+    // up-to-date.  That's OK.
   if (!node) {
-    if (excludeItems)
-      return NS_OK;
-
-    NS_NOTREACHED("Removing item we don't have");
-    return NS_ERROR_FAILURE;
+    return NS_OK;
   }
 
+  bool excludeItems = (mResult && mResult->mRootNode->mOptions->ExcludeItems()) ||
+                        (mParent && mParent->mOptions->ExcludeItems()) ||
+                        mOptions->ExcludeItems();
   if ((node->IsURI() || node->IsSeparator()) && excludeItems) {
     // don't update items when we aren't displaying them, but we do need to
     // adjust everybody's bookmark indices to account for the removal
     ReindexRange(aIndex, INT32_MAX, -1);
     return NS_OK;
   }
 
   if (!StartIncrementalUpdate())
@@ -3849,34 +3861,52 @@ nsNavHistoryFolderResultNode::OnItemMove
                                           const nsACString& aOldParentGUID,
                                           const nsACString& aNewParentGUID)
 {
   NS_ASSERTION(aOldParent == mItemId || aNewParent == mItemId,
                "Got a bookmark message that doesn't belong to us");
 
   RESTART_AND_RETURN_IF_ASYNC_PENDING();
 
+  uint32_t index;
+  nsNavHistoryResultNode* node = FindChildById(aItemId, &index);
+  // Bug 1097528.
+  // It's possible our result registered due to a previous notification, for
+  // example the Library left pane could have refreshed and replaced the
+  // right pane as a consequence. In such a case our contents are already
+  // up-to-date.  That's OK.
+  if (node && aNewParent == mItemId && index == static_cast<uint32_t>(aNewIndex))
+    return NS_OK;
+  if (!node && aOldParent == mItemId)
+    return NS_OK;
+
+  bool excludeItems = (mResult && mResult->mRootNode->mOptions->ExcludeItems()) ||
+                      (mParent && mParent->mOptions->ExcludeItems()) ||
+                      mOptions->ExcludeItems();
+  if (node && excludeItems && (node->IsURI() || node->IsSeparator())) {
+    // Don't update items when we aren't displaying them.
+    return NS_OK;
+  }
+
   if (!StartIncrementalUpdate())
     return NS_OK; // entire container was refreshed for us
 
   if (aOldParent == aNewParent) {
     // getting moved within the same folder, we don't want to do a remove and
     // an add because that will lose your tree state.
 
     // adjust bookmark indices
     ReindexRange(aOldIndex + 1, INT32_MAX, -1);
     ReindexRange(aNewIndex, INT32_MAX, 1);
 
-    uint32_t index;
-    nsNavHistoryResultNode* node = FindChildById(aItemId, &index);
+    MOZ_ASSERT(node, "Can't find folder that is moving!");
     if (!node) {
-      NS_NOTREACHED("Can't find folder that is moving!");
       return NS_ERROR_FAILURE;
     }
-    NS_ASSERTION(index < uint32_t(mChildren.Count()), "Invalid index!");
+    MOZ_ASSERT(index < uint32_t(mChildren.Count()), "Invalid index!");
     node->mBookmarkIndex = aNewIndex;
 
     // adjust position
     EnsureItemPosition(index);
     return NS_OK;
   } else {
     // moving between two different folders, just do a remove and an add
     nsCOMPtr<nsIURI> itemURI;