diff --git a/browser/components/places/content/controller.js b/browser/components/places/content/controller.js index 827673436c77..1bc37819ca08 100644 --- a/browser/components/places/content/controller.js +++ b/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": diff --git a/browser/components/places/content/moveBookmarks.js b/browser/components/places/content/moveBookmarks.js index 964604f6d3f3..65e3a54cf7f0 100644 --- a/browser/components/places/content/moveBookmarks.js +++ b/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() { diff --git a/toolkit/components/places/PlacesTransactions.jsm b/toolkit/components/places/PlacesTransactions.jsm index 14ca75eb079b..86f4ad9e7857 100644 --- a/toolkit/components/places/PlacesTransactions.jsm +++ b/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), diff --git a/toolkit/components/places/tests/unit/test_async_transactions.js b/toolkit/components/places/tests/unit/test_async_transactions.js index be5359398ecb..cd53e3e4e84c 100644 --- a/toolkit/components/places/tests/unit/test_async_transactions.js +++ b/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 = () => {