Bug 1095069 - Cannot copy or delete bookmarks from search results in library. r=mano

--HG--
rename : browser/components/places/tests/browser/browser_library_left_pane_commands.js => browser/components/places/tests/browser/browser_library_commands.js

browser/base/content/browser-places.js

@@ -474,7 +474,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);

browser/components/places/PlacesUIUtils.jsm

@@ -22,6 +22,8 @@ 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");
@@ -473,7 +475,7 @@ this.PlacesUIUtils = {
   },
 
   _getTopBrowserWin: function PUIU__getTopBrowserWin() {
-    return Services.wm.getMostRecentWindow("navigator:browser");
+    return RecentWindow.getMostRecentBrowserWindow();
   },
 
   /**
@@ -619,6 +621,10 @@ this.PlacesUIUtils = {
       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);
   },

browser/components/places/content/controller.js

@@ -149,19 +149,20 @@ PlacesController.prototype = {
       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;
@@ -310,13 +311,11 @@ PlacesController.prototype = {
    * 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;
@@ -1024,7 +1023,7 @@ PlacesController.prototype = {
    *          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");
@@ -1545,7 +1544,7 @@ let PlacesControllerDragHelper = {
   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;
   },

browser/components/places/tests/browser/browser.ini

@@ -31,7 +31,7 @@ skip-if = e10s
 # 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/components/places/tests/browser/browser_423515.js

@@ -142,6 +142,7 @@ function test() {
       // 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

browser/components/places/tests/browser/browser_bookmarksProperties.js

@@ -34,9 +34,8 @@ 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);
 

browser/components/places/tests/browser/browser_library_commands.js

@@ -0,0 +1,235 @@
+/* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
+/* vim:set ts=2 sw=2 sts=2 et: */
+/* 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 = NetUtil.newURI("http://www.mozilla.org/");
+
+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);
+
+  // 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, 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");
+  let todayNode = historyNode.getChild(0);
+  let 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, 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");
+
+  // 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;
+
+  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, 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");
+
+  let toolbarNode = PlacesUtils.asContainer(PO._places.selectedNode);
+  toolbarNode.containerOpen = true;
+
+  // 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 });
+
+  // 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_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.
+  let promiseItemRemoved = promiseBookmarksNotification("onItemRemoved",
+                                                        () => query.guid == arguments[5]);
+  PO._places.controller.doCommand("cmd_delete");
+  yield promiseItemRemoved;
+
+  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");
+
+  // 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");
+
+  // 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");
+
+  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();
+});

browser/components/places/tests/browser/browser_library_infoBox.js

@@ -71,16 +71,24 @@ gTests.push({
       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);
@@ -98,15 +106,6 @@ gTests.push({
       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);

browser/components/places/tests/browser/browser_library_left_pane_commands.js

@@ -1,156 +0,0 @@
-/* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
-/* vim:set ts=2 sw=2 sts=2 et: */
-/* 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/";
-
-var gTests = [];
-var gLibrary;
-
-//------------------------------------------------------------------------------
-
-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");
-
-      // 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 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");
-
-      // 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;
-
-      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);
-  }
-});
-
-//------------------------------------------------------------------------------
-
-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.");
-
-    // 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");
-
-    var toolbarNode = PO._places.selectedNode
-                        .QueryInterface(Ci.nsINavHistoryContainerResultNode);
-    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");
-
-    // 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");
-
-    // 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");
-    }
-
-    toolbarNode.containerOpen = false;
-    nextTest();
-  }
-});
-
-//------------------------------------------------------------------------------
-
-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();
-  }
-}
-
-function test() {
-  waitForExplicitFinish();
-  // Sanity checks.
-  ok(PlacesUtils, "PlacesUtils is running in chrome context");
-  ok(PlacesUIUtils, "PlacesUIUtils is running in chrome context");
-
-  // Open Library.
-  gLibrary = openLibrary(nextTest);
-}

browser/components/places/tests/browser/head.js

@@ -43,7 +43,7 @@ function openLibrary(callback, aLeftPaneRoot) {
 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);
@@ -164,7 +164,7 @@ function addVisits(aPlaceInfo, aWindow, aCallback, aStack) {
       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
@@ -203,3 +203,206 @@ function synthesizeClickOnSelectedTreeCell(aTree, aOptions) {
   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;
+}

toolkit/components/places/nsNavHistoryResult.cpp

@@ -3508,9 +3508,23 @@ nsNavHistoryFolderResultNode::OnItemAdded(int64_t aItemId,
 {
   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.
@@ -3526,8 +3540,6 @@ nsNavHistoryFolderResultNode::OnItemAdded(int64_t aItemId,
     aIndex = mChildren.Count();
   }
 
-  RESTART_AND_RETURN_IF_ASYNC_PENDING();
-
   nsresult rv;
 
   // Check for query URIs, which are bookmarks, but treated as containers
@@ -3606,23 +3618,23 @@ nsNavHistoryFolderResultNode::OnItemRemoved(int64_t aItemId,
 
   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
@@ -3854,6 +3866,26 @@ nsNavHistoryFolderResultNode::OnItemMoved(int64_t aItemId,
 
   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
 
@@ -3865,13 +3897,11 @@ nsNavHistoryFolderResultNode::OnItemMoved(int64_t aItemId,
     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