Bug 993391 - Places async transactions: Implement "move bookmarks" command. r=mak.

browser/components/places/content/controller.js

@@ -133,7 +133,6 @@ PlacesController.prototype = {
       case "cmd_paste":
       case "cmd_delete":
       case "placesCmd_delete":
-      case "placesCmd_moveBookmarks":
       case "cmd_paste":
       case "placesCmd_paste":
       case "placesCmd_new:folder":

browser/components/places/content/moveBookmarks.js

@@ -23,27 +23,38 @@ var gMoveBookmarksDialog = {
   },
 
   onOK: function MBD_onOK(aEvent) {
-    var selectedNode = this.foldersTree.selectedNode;
-    NS_ASSERT(selectedNode,
-              "selectedNode must be set in a single-selection tree with initial selection set");
-    var selectedFolderID = PlacesUtils.getConcreteItemId(selectedNode);
+    let selectedNode = this.foldersTree.selectedNode;
+    let selectedFolderId = PlacesUtils.getConcreteItemId(selectedNode);
 
-    var transactions = [];
-    for (var i=0; i < this._nodes.length; i++) {
-      // Nothing to do if the node is already under the selected folder
-      if (this._nodes[i].parent.itemId == selectedFolderID)
-        continue;
+    if (!PlacesUIUtils.useAsyncTransactions) {
+      let transactions = [];
+      for (var i=0; i < this._nodes.length; i++) {
+        // Nothing to do if the node is already under the selected folder
+        if (this._nodes[i].parent.itemId == selectedFolderId)
+          continue;
 
-      let txn = new PlacesMoveItemTransaction(this._nodes[i].itemId,
-                                              selectedFolderID,
-                                              PlacesUtils.bookmarks.DEFAULT_INDEX);
-      transactions.push(txn);
+        let txn = new PlacesMoveItemTransaction(this._nodes[i].itemId,
+                                                selectedFolderId,
+                                                PlacesUtils.bookmarks.DEFAULT_INDEX);
+        transactions.push(txn);
+      }
+      if (transactions.length != 0) {
+        let txn = new PlacesAggregatedTransaction("Move Items", transactions);
+        PlacesUtils.transactionManager.doTransaction(txn);
+      }
+      return;
     }
 
-    if (transactions.length != 0) {
-      let txn = new PlacesAggregatedTransaction("Move Items", transactions);
-      PlacesUtils.transactionManager.doTransaction(txn);
-    }
+    PlacesTransactions.transact(function* () {
+      let newParentGUID = yield PlacesUtils.promiseItemGUID(selectedFolderId);
+      for (let node of this._nodes) {
+        // Nothing to do if the node is already under the selected folder.
+        if (node.parent.itemId == selectedFolderId)
+          continue;
+        yield PlacesTransactions.MoveItem({ GUID: node.bookmarkGuid
+                                          , newParentGUID: newParentGUID });
+      }
+    }.bind(this)).then(null, Components.utils.reportError);
   },
 
   newFolder: function MBD_newFolder() {

toolkit/components/places/PlacesTransactions.jsm

@@ -351,16 +351,20 @@ let PlacesTransactions = {
    * are not protected from consumers who use the raw places APIs directly.
    */
   transact: function (aToTransact) {
-    let generatorMode = typeof(aToTransact) == "function";
-    if (generatorMode) {
-      if (!aToTransact.isGenerator())
+    let isGeneratorObj =
+      o => Object.prototype.toString.call(o) ==  "[object Generator]";
+
+    let generator = null;
+    if (typeof(aToTransact) == "function") {
+      generator = aToTransact();
+      if (!isGeneratorObj(generator))
         throw new Error("aToTransact is not a generator function");
     }
-    else {
-      if (!TransactionsHistory.isProxifiedTransactionObject(aToTransact))
-        throw new Error("aToTransact is not a valid transaction object");
-      if (executedTransactions.has(aToTransact))
-        throw new Error("Transactions objects may not be recycled.");
+    else if (!TransactionsHistory.isProxifiedTransactionObject(aToTransact)) {
+      throw new Error("aToTransact is not a valid transaction object");
+    }
+    else if (executedTransactions.has(aToTransact)) {
+      throw new Error("Transactions objects may not be recycled.");
     }
 
     return Serialize(function* () {
@@ -387,7 +391,7 @@ let PlacesTransactions = {
           let next = error ?
                      aGenerator.throw(sendValue) : aGenerator.next(sendValue);
           sendValue = next.value;
-          if (Object.prototype.toString.call(sendValue) == "[object Generator]") {
+          if (isGeneratorObj(sendValue)) {
             sendValue = yield transactBatch(sendValue);
           }
           else if (typeof(sendValue) == "object" && sendValue) {
@@ -410,8 +414,8 @@ let PlacesTransactions = {
         return sendValue;
       }
 
-      if (generatorMode)
-        return yield transactBatch(aToTransact());
+      if (generator)
+        return yield transactBatch(generator);
       else
         return yield transactOneTransaction(aToTransact);
     }.bind(this));
@@ -887,9 +891,10 @@ PT.NewLivemark.prototype = Object.seal({
 /**
  * Transaction for moving an item.
  *
- * Required Input Properties: GUID, newParentGUID, newIndex.
+ * Required Input Properties: GUID, newParentGUID.
+ * Optional Input Properties  newIndex.
  */
-PT.MoveItem = DefineTransaction(["GUID", "newParentGUID", "newIndex"]);
+PT.MoveItem = DefineTransaction(["GUID", "newParentGUID"], ["newIndex"]);
 PT.MoveItem.prototype = Object.seal({
   execute: function* (aGUID, aNewParentGUID, aNewIndex) {
     let itemId = yield PlacesUtils.promiseItemId(aGUID),

toolkit/components/places/tests/unit/test_async_transactions.js

@@ -454,8 +454,7 @@ add_task(function* test_move_items_to_folder() {
   ensureUndoState([[bkm_b_txn, bkm_a_txn, folder_a_txn]], 0);
 
   let moveTxn = PT.MoveItem({ GUID:          bkm_a_info.GUID
-                            , newParentGUID: folder_a_info.GUID
-                            , newIndex:      bmsvc.DEFAULT_INDEX });
+                            , newParentGUID: folder_a_info.GUID });
   yield PT.transact(moveTxn);
 
   let ensureDo = () => {