From 25f9c666412e0952f82b65293ea920d910ec2757 Mon Sep 17 00:00:00 2001 From: Asaf Romano Date: Wed, 17 Sep 2014 14:59:23 +0300 Subject: [PATCH] Bug 1058566 - Transaction for duplicating any places item. r=mak. --- .../places/content/moveBookmarks.js | 4 +- toolkit/components/places/PlacesBackups.jsm | 3 +- .../components/places/PlacesTransactions.jsm | 309 ++++++++++-------- toolkit/components/places/PlacesUtils.jsm | 28 +- .../tests/unit/test_async_transactions.js | 257 +++++++++++++-- .../tests/unit/test_promiseBookmarksTree.js | 11 +- 6 files changed, 431 insertions(+), 181 deletions(-) diff --git a/browser/components/places/content/moveBookmarks.js b/browser/components/places/content/moveBookmarks.js index e9135ace9a70..51db97d13d26 100644 --- a/browser/components/places/content/moveBookmarks.js +++ b/browser/components/places/content/moveBookmarks.js @@ -51,8 +51,8 @@ var gMoveBookmarksDialog = { // 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 }); + yield PlacesTransactions.Move({ GUID: node.bookmarkGuid + , newParentGUID: newParentGUID }); } }.bind(this)).then(null, Components.utils.reportError); }, diff --git a/toolkit/components/places/PlacesBackups.jsm b/toolkit/components/places/PlacesBackups.jsm index 68e4fb7d5eca..3be508d4b566 100644 --- a/toolkit/components/places/PlacesBackups.jsm +++ b/toolkit/components/places/PlacesBackups.jsm @@ -508,7 +508,8 @@ this.PlacesBackups = { excludeItemsCallback: aItem => { return aItem.annos && aItem.annos.find(a => a.name == PlacesUtils.EXCLUDE_FROM_BACKUP_ANNO); - } + }, + includeItemIds: true }); try { diff --git a/toolkit/components/places/PlacesTransactions.jsm b/toolkit/components/places/PlacesTransactions.jsm index 55bb9e4fadd5..5d0c7a1969ae 100644 --- a/toolkit/components/places/PlacesTransactions.jsm +++ b/toolkit/components/places/PlacesTransactions.jsm @@ -752,10 +752,111 @@ function* ExecuteCreateItem(aTransaction, aParentGUID, aCreateItemFunction, // lastModified. PlacesUtils.bookmarks.setItemDateAdded(itemId, dateAdded); PlacesUtils.bookmarks.setItemLastModified(itemId, lastModified); + PlacesUtils.bookmarks.setItemLastModified(parentId, dateAdded); }; return guid; } +/** + * Creates items (all types) from a bookmarks tree representation, as defined + * in PlacesUtils.promiseBookmarksTree. + * + * @param aBookmarksTree + * the bookmarks tree object. You may pass either a bookmarks tree + * returned by promiseBookmarksTree, or a manually defined one. + * @param [optional] aRestoring (default: false) + * Whether or not the items are restored. Only in restore mode, are + * the guid, dateAdded and lastModified properties honored. + * @note the id, root and charset properties of items in aBookmarksTree are + * always ignored. The index property is ignored for all items but the + * root one. + * @return {Promise} + */ +function* createItemsFromBookmarksTree(aBookmarksTree, aRestoring = false) { + function extractLivemarkDetails(aAnnos) { + let feedURI = null, siteURI = null; + aAnnos = aAnnos.filter( + aAnno => { + switch (aAnno.name) { + case PlacesUtils.LMANNO_FEEDURI: + feedURI = NetUtil.newURI(aAnno.value); + return false; + case PlacesUtils.LMANNO_SITEURI: + siteURI = NetUtil.newURI(aAnno.value); + return false; + default: + return true; + } + } ); + return [feedURI, siteURI]; + } + + function* createItem(aItem, + aParentGUID, + aIndex = PlacesUtils.bookmarks.DEFAULT_INDEX) { + let itemId; + let guid = aRestoring ? aItem.guid : undefined; + let parentId = yield PlacesUtils.promiseItemId(aParentGUID); + let annos = aItem.annos ? [...aItem.annos] : []; + switch (aItem.type) { + case PlacesUtils.TYPE_X_MOZ_PLACE: { + let uri = NetUtil.newURI(aItem.uri); + itemId = PlacesUtils.bookmarks.insertBookmark( + parentId, uri, aIndex, aItem.title, guid); + if ("keyword" in aItem) + PlacesUtils.bookmarks.setKeywordForBookmark(itemId, aItem.keyword); + if ("tags" in aItem) { + PlacesUtils.tagging.tagURI(uri, aItem.tags.split(",")); + } + break; + } + case PlacesUtils.TYPE_X_MOZ_PLACE_CONTAINER: { + // Either a folder or a livemark + let [feedURI, siteURI] = extractLivemarkDetails(annos); + if (!feedURI) { + itemId = PlacesUtils.bookmarks.createFolder( + parentId, aItem.title, aIndex, guid); + if (guid === undefined) + guid = yield PlacesUtils.promiseItemGUID(itemId); + if ("children" in aItem) { + for (let child of aItem.children) { + yield createItem(child, guid); + } + } + } + else { + let livemark = + yield PlacesUtils.livemarks.addLivemark({ title: aItem.title + , feedURI: feedURI + , siteURI: siteURI + , parentId: parentId + , index: aIndex + , guid: guid}); + itemId = livemark.id; + } + break; + } + case PlacesUtils.TYPE_X_MOZ_PLACE_SEPARATOR: { + itemId = PlacesUtils.bookmarks.insertSeparator(parentId, aIndex, guid); + break; + } + } + if (annos.length > 0) + PlacesUtils.setAnnotationsForItem(itemId, annos); + + if (aRestoring) { + if ("dateAdded" in aItem) + PlacesUtils.bookmarks.setItemDateAdded(itemId, aItem.dateAdded); + if ("lastModified" in aItem) + PlacesUtils.bookmarks.setItemLastModified(itemId, aItem.lastModified); + } + return itemId; + } + return yield createItem(aBookmarksTree, + aBookmarksTree.parentGUID, + aBookmarksTree.index); +} + /***************************************************************************** * The Standard Places Transactions. * @@ -862,32 +963,40 @@ PT.NewLivemark = DefineTransaction(["feedURI", "title", "parentGUID"], ["siteURI", "index", "annotations"]); PT.NewLivemark.prototype = Object.seal({ execute: function* (aFeedURI, aTitle, aParentGUID, aSiteURI, aIndex, aAnnos) { - let createItem = function* (aGUID = "") { - let parentId = yield PlacesUtils.promiseItemId(aParentGUID); - let livemarkInfo = { - title: aTitle - , feedURI: aFeedURI - , parentId: parentId - , index: aIndex - , siteURI: aSiteURI }; - if (aGUID) - livemarkInfo.guid = aGUID; - + let livemarkInfo = { title: aTitle + , feedURI: aFeedURI + , siteURI: aSiteURI + , index: aIndex }; + let createItem = function* () { + livemarkInfo.parentId = yield PlacesUtils.promiseItemId(aParentGUID); let livemark = yield PlacesUtils.livemarks.addLivemark(livemarkInfo); if (aAnnos) PlacesUtils.setAnnotationsForItem(livemark.id, aAnnos); + if ("dateAdded" in livemarkInfo) { + PlacesUtils.bookmarks.setItemDateAdded(livemark.id, + livemarkInfo.dateAdded); + PlacesUtils.bookmarks.setItemLastModified(livemark.id, + livemarkInfo.lastModified); + } return livemark; }; - let guid = (yield createItem()).guid; + let livemark = yield createItem(); this.undo = function* () { - yield PlacesUtils.livemarks.removeLivemark({ guid: guid }); + livemarkInfo.guid = livemark.guid; + if (!("dateAdded" in livemarkInfo)) { + livemarkInfo.dateAdded = + PlacesUtils.bookmarks.getItemDateAdded(livemark.id); + livemarkInfo.lastModified = + PlacesUtils.bookmarks.getItemLastModified(livemark.id); + } + yield PlacesUtils.livemarks.removeLivemark(livemark); }; this.redo = function* () { - yield createItem(guid); + livemark = yield createItem(); }; - return guid; + return livemark.guid; } }); @@ -897,8 +1006,8 @@ PT.NewLivemark.prototype = Object.seal({ * Required Input Properties: GUID, newParentGUID. * Optional Input Properties newIndex. */ -PT.MoveItem = DefineTransaction(["GUID", "newParentGUID"], ["newIndex"]); -PT.MoveItem.prototype = Object.seal({ +PT.Move = DefineTransaction(["GUID", "newParentGUID"], ["newIndex"]); +PT.Move.prototype = Object.seal({ execute: function* (aGUID, aNewParentGUID, aNewIndex) { let itemId = yield PlacesUtils.promiseItemId(aGUID), oldParentId = PlacesUtils.bookmarks.getFolderIdForItem(itemId), @@ -1095,127 +1204,21 @@ PT.SortByName.prototype = { * * Required Input Properties: GUID. */ -PT.RemoveItem = DefineTransaction(["GUID"]); -PT.RemoveItem.prototype = { +PT.Remove = DefineTransaction(["GUID"]); +PT.Remove.prototype = { execute: function* (aGUID) { const bms = PlacesUtils.bookmarks; - let itemsToRestoreOnUndo = []; - function* saveItemRestoreData(aItem, aNode = null) { - if (!aItem || !aItem.GUID) - throw new Error("invalid item object"); - - let itemId = aNode ? - aNode.itemId : yield PlacesUtils.promiseItemId(aItem.GUID); - if (itemId == -1) - throw new Error("Unexpected non-bookmarks node"); - - aItem.itemType = function() { - if (aNode) { - switch (aNode.type) { - case aNode.RESULT_TYPE_SEPARATOR: - return bms.TYPE_SEPARATOR; - case aNode.RESULT_TYPE_URI: // regular bookmarks - case aNode.RESULT_TYPE_FOLDER_SHORTCUT: // place:folder= bookmarks - case aNode.RESULT_TYPE_QUERY: // smart bookmarks - return bms.TYPE_BOOKMARK; - case aNode.RESULT_TYPE_FOLDER: - return bms.TYPE_FOLDER; - default: - throw new Error("Unexpected node type"); - } - } - return bms.getItemType(itemId); - }(); - - let node = aNode; - if (!node && aItem.itemType == bms.TYPE_FOLDER) - node = PlacesUtils.getFolderContents(itemId).root; - - // dateAdded, lastModified and annotations apply to all types. - aItem.dateAdded = node ? node.dateAdded : bms.getItemDateAdded(itemId); - aItem.lastModified = node ? - node.lastModified : bms.getItemLastModified(itemId); - aItem.annotations = PlacesUtils.getAnnotationsForItem(itemId); - - // For the first-level item, we don't have the parent. - if (!aItem.parentGUID) { - let parentId = PlacesUtils.bookmarks.getFolderIdForItem(itemId); - aItem.parentGUID = yield PlacesUtils.promiseItemGUID(parentId); - // For the first-level item, we also need the index. - // Note: node.bookmarkIndex doesn't work for root nodes. - aItem.index = bms.getItemIndex(itemId); - } - - // Separators don't have titles. - if (aItem.itemType != bms.TYPE_SEPARATOR) { - aItem.title = node ? node.title : bms.getItemTitle(itemId); - - if (aItem.itemType == bms.TYPE_BOOKMARK) { - aItem.uri = - node ? NetUtil.newURI(node.uri) : bms.getBookmarkURI(itemId); - aItem.keyword = PlacesUtils.bookmarks.getKeywordForBookmark(itemId); - - // This may be the last bookmark (excluding the tag-items themselves) - // for the URI, so we need to preserve the tags. - let tags = PlacesUtils.tagging.getTagsForURI(aItem.uri);; - if (tags.length > 0) - aItem.tags = tags; - } - else { // folder - // We always have the node for folders - aItem.readOnly = node.childrenReadOnly; - for (let i = 0; i < node.childCount; i++) { - let childNode = node.getChild(i); - let childItem = - { GUID: yield PlacesUtils.promiseItemGUID(childNode.itemId) - , parentGUID: aItem.GUID }; - itemsToRestoreOnUndo.push(childItem); - yield saveItemRestoreData(childItem, childNode); - } - node.containerOpen = false; - } - } + let itemInfo = null; + try { + itemInfo = yield PlacesUtils.promiseBookmarksTree(aGUID); } - - let item = { GUID: aGUID, parentGUID: null }; - itemsToRestoreOnUndo.push(item); - yield saveItemRestoreData(item); - - let itemId = yield PlacesUtils.promiseItemId(aGUID); - PlacesUtils.bookmarks.removeItem(itemId); - this.undo = function() { - for (let item of itemsToRestoreOnUndo) { - let parentId = yield PlacesUtils.promiseItemId(item.parentGUID); - let index = "index" in item ? - item.index : PlacesUtils.bookmarks.DEFAULT_INDEX; - let itemId; - if (item.itemType == bms.TYPE_SEPARATOR) { - itemId = bms.insertSeparator(parentId, index, item.GUID); - } - else if (item.itemType == bms.TYPE_BOOKMARK) { - itemId = bms.insertBookmark(parentId, item.uri, index, item.title, - item.GUID); - } - else { // folder - itemId = bms.createFolder(parentId, item.title, index, item.GUID); - } - - if (item.itemType == bms.TYPE_BOOKMARK) { - if (item.keyword) - bms.setKeywordForBookmark(itemId, item.keyword); - if ("tags" in item) - PlacesUtils.tagging.tagURI(item.uri, item.tags); - } - else if (item.readOnly === true) { - bms.setFolderReadonly(itemId, true); - } - - PlacesUtils.setAnnotationsForItem(itemId, item.annotations); - PlacesUtils.bookmarks.setItemDateAdded(itemId, item.dateAdded); - PlacesUtils.bookmarks.setItemLastModified(itemId, item.lastModified); - } - }; + catch(ex) { + throw new Error("Failed to get info for the specified item (guid: " + + aGUID + "). Ex: " + ex); + } + PlacesUtils.bookmarks.removeItem(yield PlacesUtils.promiseItemId(aGUID)); + this.undo = createItemsFromBookmarksTree.bind(null, itemInfo, true); } }; @@ -1270,3 +1273,41 @@ PT.UntagURI.prototype = { this.redo = () => { PlacesUtils.tagging.untagURI(aURI, aTags); }; } }; + +/** + * Transaction for copying an item. + * + * Required Input Properties: guid, newParentGUID + * Optional Input Properties: newIndex. + */ +PT.Copy = DefineTransaction(["GUID", "newParentGUID"], + ["newIndex"]); +PT.Copy.prototype = { + execute: function* (aGUID, aNewParentGUID, aNewIndex) { + let creationInfo = null; + try { + creationInfo = yield PlacesUtils.promiseBookmarksTree(aGUID); + } + catch(ex) { + throw new Error("Failed to get info for the specified item (guid: " + + aGUID + "). Ex: " + ex); + } + creationInfo.parentGUID = aNewParentGUID; + creationInfo.index = aNewIndex; + + let newItemId = yield createItemsFromBookmarksTree(creationInfo, false); + let newItemInfo = null; + this.undo = function* () { + if (!newItemInfo) { + let newItemGUID = yield PlacesUtils.promiseItemGUID(newItemId); + newItemInfo = yield PlacesUtils.promiseBookmarksTree(newItemGUID); + } + PlacesUtils.bookmarks.removeItem(newItemId); + }; + this.redo = function* () { + newItemId = yield createItemsFromBookmarksTree(newItemInfo, true); + } + + return yield PlacesUtils.promiseItemGUID(newItemId); + } +}; diff --git a/toolkit/components/places/PlacesUtils.jsm b/toolkit/components/places/PlacesUtils.jsm index ecb43815794c..a9f6ead1a59f 100644 --- a/toolkit/components/places/PlacesUtils.jsm +++ b/toolkit/components/places/PlacesUtils.jsm @@ -1592,13 +1592,16 @@ this.PlacesUtils = { * this option can slow down the process significantly if the * callback does anything that's not relatively trivial. It is * highly recommended to avoid any synchronous I/O or DB queries. + * - includeItemIds: opt-in to include the deprecated id property. + * Use it if you must. It'll be removed once the switch to guids is + * complete. * * @return {Promise} * @resolves to a JS object that represents either a single item or a * bookmarks tree. Each node in the tree has the following properties set: * - guid (string): the item's guid (same as aItemGUID for the top item). - * - [deprecated] id (number): the item's id. Only use it if you must. It'll - * be removed once the switch to guids is complete. + * - [deprecated] id (number): the item's id. This is only if + * aOptions.includeItemIds is set. * - type (number): the item's type. @see PlacesUtils.TYPE_X_* * - title (string): the item's title. If it has no title, this property * isn't set. @@ -1644,10 +1647,17 @@ this.PlacesUtils = { item[prop] = val; } }; - copyProps("id" ,"guid", "title", "index", "dateAdded", "lastModified"); + copyProps("guid", "title", "index", "dateAdded", "lastModified"); if (aIncludeParentGUID) copyProps("parentGUID"); + let itemId = aRow.getResultByName("id"); + if (aOptions.includeItemIds) + item.id = itemId; + + // Cache it for promiseItemId consumers regardless. + GUIDHelper.idsForGUIDs.set(item.guid, itemId); + let type = aRow.getResultByName("type"); if (type == Ci.nsINavBookmarksService.TYPE_BOOKMARK) copyProps("charset", "tags", "iconuri"); @@ -1655,7 +1665,7 @@ this.PlacesUtils = { // Add annotations. if (aRow.getResultByName("has_annos")) { try { - item.annos = PlacesUtils.getAnnotationsForItem(item.id); + item.annos = PlacesUtils.getAnnotationsForItem(itemId); } catch (e) { Cu.reportError("Unexpected error while reading annotations " + e); } @@ -1667,20 +1677,20 @@ this.PlacesUtils = { // If this throws due to an invalid url, the item will be skipped. item.uri = NetUtil.newURI(aRow.getResultByName("url")).spec; // Keywords are cached, so this should be decently fast. - let keyword = PlacesUtils.bookmarks.getKeywordForBookmark(item.id); + let keyword = PlacesUtils.bookmarks.getKeywordForBookmark(itemId); if (keyword) item.keyword = keyword; break; case Ci.nsINavBookmarksService.TYPE_FOLDER: item.type = PlacesUtils.TYPE_X_MOZ_PLACE_CONTAINER; // Mark root folders. - if (item.id == PlacesUtils.placesRootId) + if (itemId == PlacesUtils.placesRootId) item.root = "placesRoot"; - else if (item.id == PlacesUtils.bookmarksMenuFolderId) + else if (itemId == PlacesUtils.bookmarksMenuFolderId) item.root = "bookmarksMenuFolder"; - else if (item.id == PlacesUtils.unfiledBookmarksFolderId) + else if (itemId == PlacesUtils.unfiledBookmarksFolderId) item.root = "unfiledBookmarksFolder"; - else if (item.id == PlacesUtils.toolbarFolderId) + else if (itemId == PlacesUtils.toolbarFolderId) item.root = "toolbarFolder"; break; case Ci.nsINavBookmarksService.TYPE_SEPARATOR: diff --git a/toolkit/components/places/tests/unit/test_async_transactions.js b/toolkit/components/places/tests/unit/test_async_transactions.js index a4dcbd5105cf..28522d1678a5 100644 --- a/toolkit/components/places/tests/unit/test_async_transactions.js +++ b/toolkit/components/places/tests/unit/test_async_transactions.js @@ -165,28 +165,34 @@ function ensureUndoState(aExpectedEntries = [], aExpectedUndoPosition = 0) { } function ensureItemsAdded(...items) { - do_check_eq(observer.itemsAdded.size, items.length); + Assert.equal(observer.itemsAdded.size, items.length); for (let item of items) { - do_check_true(observer.itemsAdded.has(item.GUID)); + Assert.ok(observer.itemsAdded.has(item.GUID)); let info = observer.itemsAdded.get(item.GUID); - do_check_eq(info.parentGUID, item.parentGUID); - if ("title" in item) - do_check_eq(info.title, item.title); - if ("index" in item) - do_check_eq(info.index, item.index); - if ("itemType" in item) - do_check_eq(info.itemType, item.itemType); + Assert.equal(info.parentGUID, item.parentGUID); + for (let propName of ["title", "index", "itemType"]) { + if (propName in item) + Assert.equal(info[propName], item[propName]); + } + if ("uri" in item) + Assert.ok(info.uri.equals(item.uri)); } } function ensureItemsRemoved(...items) { - do_check_eq(observer.itemsRemoved.size, items.length); + Assert.equal(observer.itemsRemoved.size, items.length); for (let item of items) { - do_check_true(observer.itemsRemoved.has(item.GUID)); - let info = observer.itemsRemoved.get(item.GUID); - do_check_eq(info.parentGUID, item.parentGUID); - if ("index" in item) - do_check_eq(info.index, item.index); + // We accept both guids and full info object here. + if (typeof(item) == "string") { + Assert.ok(observer.itemsRemoved.has(item)); + } + else { + Assert.ok(observer.itemsRemoved.has(item.GUID)); + let info = observer.itemsRemoved.get(item.GUID); + Assert.equal(info.parentGUID, item.parentGUID); + if ("index" in item) + Assert.equal(info.index, item.index); + } } } @@ -245,6 +251,80 @@ function* createTestFolderInfo(aTitle = "Test Folder") { , title: "Test Folder" }; } +function isLivemarkTree(aTree) { + return !!aTree.annos && + aTree.annos.some( a => a.name == PlacesUtils.LMANNO_FEEDURI ); +} + +function* ensureLivemarkCreatedByAddLivemark(aLivemarkGUID) { + // This throws otherwise. + yield PlacesUtils.livemarks.getLivemark({ guid: aLivemarkGUID }); +} + +// Checks if two bookmark trees (as returned by promiseBookmarksTree) are the +// same. +// false value for aCheckParentAndPosition is ignored if aIsRestoredItem is set. +function* ensureEqualBookmarksTrees(aOriginal, + aNew, + aIsRestoredItem = true, + aCheckParentAndPosition = false) { + // Note "id" is not-enumerable, and is therefore skipped by Object.keys (both + // ours and the one at deepEqual). This is fine for us because ids are not + // restored by Redo. + if (aIsRestoredItem) { + Assert.deepEqual(aOriginal, aNew); + if (isLivemarkTree(aNew)) + yield ensureLivemarkCreatedByAddLivemark(aNew.guid); + return; + } + + for (let property of Object.keys(aOriginal)) { + if (property == "children") { + Assert.equal(aOriginal.children.length, aNew.children.length); + for (let i = 0; i < aOriginal.children.length; i++) { + yield ensureEqualBookmarksTrees(aOriginal.children[i], + aNew.children[i], + false, + true); + } + } + else if (property == "guid") { + // guid shouldn't be copied if the item was not restored. + Assert.notEqual(aOriginal.guid, aNew.guid); + } + else if (property == "dateAdded") { + // dateAdded shouldn't be copied if the item was not restored. + Assert.ok(is_time_ordered(aOriginal.dateAdded, aNew.dateAdded)); + } + else if (property == "lastModified") { + // same same, except for the never-changed case + if (!aOriginal.lastModified) + Assert.ok(!aNew.lastModified); + else + Assert.ok(is_time_ordered(aOriginal.lastModified, aNew.lastModified)); + } + else if (aCheckParentAndPosition || + (property != "parentGUID" && property != "index")) { + Assert.deepEqual(aOriginal[property], aNew[property]); + } + } + + if (isLivemarkTree(aNew)) + yield ensureLivemarkCreatedByAddLivemark(aNew.guid); +} + +function* ensureBookmarksTreeRestoredCorrectly(aOriginalBookmarksTree) { + let restoredTree = + yield PlacesUtils.promiseBookmarksTree(aOriginalBookmarksTree.guid); + yield ensureEqualBookmarksTrees(aOriginalBookmarksTree, restoredTree); +} + +function* ensureNonExistent(...aGUIDs) { + for (let guid of aGUIDs) { + Assert.strictEqual((yield PlacesUtils.promiseBookmarksTree(guid)), null); + } +} + add_task(function* test_invalid_transact_calls() { try { PT.transact({ execute: () => {}, undo: () => {}, redo: () => {}}); @@ -453,8 +533,8 @@ 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 }); + let moveTxn = PT.Move({ GUID: bkm_a_info.GUID + , newParentGUID: folder_a_info.GUID }); yield PT.transact(moveTxn); let ensureDo = () => { @@ -494,9 +574,9 @@ add_task(function* test_move_items_to_folder() { ensureUndoState([ [folder_b_txn] , [bkm_b_txn, bkm_a_txn, folder_a_txn] ], 0); - moveTxn = PT.MoveItem({ GUID: bkm_a_info.GUID - , newParentGUID: folder_b_info.GUID - , newIndex: bmsvc.DEFAULT_INDEX }); + moveTxn = PT.Move({ GUID: bkm_a_info.GUID + , newParentGUID: folder_b_info.GUID + , newIndex: bmsvc.DEFAULT_INDEX }); yield PT.transact(moveTxn); ensureDo = () => { @@ -558,14 +638,14 @@ add_task(function* test_remove_folder() { yield ensureItemsAdded(folder_level_1_info, folder_level_2_info); observer.reset(); - let remove_folder_2_txn = PT.RemoveItem(folder_level_2_info); + let remove_folder_2_txn = PT.Remove(folder_level_2_info); yield PT.transact(remove_folder_2_txn); ensureUndoState([ [remove_folder_2_txn] , [folder_level_2_txn, folder_level_1_txn] ]); yield ensureItemsRemoved(folder_level_2_info); - // Undo RemoveItem "Folder Level 2" + // Undo Remove "Folder Level 2" yield PT.undo(); ensureUndoState([ [remove_folder_2_txn] , [folder_level_2_txn, folder_level_1_txn] ], 1); @@ -573,7 +653,7 @@ add_task(function* test_remove_folder() { ensureTimestampsUpdated(folder_level_2_info.GUID, true); observer.reset(); - // Redo RemoveItem "Folder Level 2" + // Redo Remove "Folder Level 2" yield PT.redo(); ensureUndoState([ [remove_folder_2_txn] , [folder_level_2_txn, folder_level_1_txn] ]); @@ -604,7 +684,7 @@ add_task(function* test_remove_folder() { ensureTimestampsUpdated(folder_level_2_info.GUID, true); observer.reset(); - // Redo RemoveItem "Folder Level 2" + // Redo Remove "Folder Level 2" yield PT.redo(); ensureUndoState([ [remove_folder_2_txn] , [folder_level_2_txn, folder_level_1_txn] ]); @@ -652,9 +732,9 @@ add_task(function* test_add_and_remove_bookmarks_with_additional_info() { ensureTimestampsUpdated(b1_info.GUID, true); ensureTags([TAG_1]); - // Check if the RemoveItem transaction removes and restores tags of children + // Check if the Remove transaction removes and restores tags of children // correctly. - yield PT.transact(PT.RemoveItem(folder_info.GUID)); + yield PT.transact(PT.Remove(folder_info.GUID)); ensureTags([]); observer.reset(); @@ -700,17 +780,17 @@ add_task(function* test_add_and_remove_bookmarks_with_additional_info() { yield ensureItemsRemoved(b2_info); ensureTags([TAG_1]); - // Check if RemoveItem correctly restores keywords, tags and annotations. + // Check if Remove correctly restores keywords, tags and annotations. observer.reset(); yield PT.redo(); ensureItemsChanged(...b2_post_creation_changes); ensureTags([TAG_1, TAG_2]); - // Test RemoveItem for multiple items. + // Test Remove for multiple items. observer.reset(); - yield PT.transact(PT.RemoveItem(b1_info.GUID)); - yield PT.transact(PT.RemoveItem(b2_info.GUID)); - yield PT.transact(PT.RemoveItem(folder_info.GUID)); + yield PT.transact(PT.Remove(b1_info.GUID)); + yield PT.transact(PT.Remove(b2_info.GUID)); + yield PT.transact(PT.Remove(folder_info.GUID)); yield ensureItemsRemoved(b1_info, b2_info, folder_info); ensureTags([]); @@ -777,7 +857,7 @@ add_task(function* test_creating_and_removing_a_separator() { ensureItemsAdded(folder_info, separator_info); observer.reset(); - let remove_sep_txn = PT.RemoveItem(separator_info); + let remove_sep_txn = PT.Remove(separator_info); yield PT.transact(remove_sep_txn); undoEntries.unshift([remove_sep_txn]); ensureUndoState(undoEntries, 0); @@ -816,6 +896,49 @@ add_task(function* test_creating_and_removing_a_separator() { ensureUndoState(); }); +add_task(function* test_add_and_remove_livemark() { + let createLivemarkTxn = PT.NewLivemark( + { feedURI: NetUtil.newURI("http://test.remove.livemark") + , parentGUID: yield PlacesUtils.promiseItemGUID(root) + , title: "Test Remove Livemark" }); + let guid = yield PlacesTransactions.transact(createLivemarkTxn); + let originalInfo = yield PlacesUtils.promiseBookmarksTree(guid); + Assert.ok(originalInfo); + yield ensureLivemarkCreatedByAddLivemark(guid); + + let removeTxn = PT.Remove(guid); + yield PT.transact(removeTxn); + yield ensureNonExistent(guid); + function* undo() { + ensureUndoState([[removeTxn], [createLivemarkTxn]], 0); + yield PT.undo(); + ensureUndoState([[removeTxn], [createLivemarkTxn]], 1); + yield ensureBookmarksTreeRestoredCorrectly(originalInfo); + yield PT.undo(); + ensureUndoState([[removeTxn], [createLivemarkTxn]], 2); + yield ensureNonExistent(guid); + } + function* redo() { + ensureUndoState([[removeTxn], [createLivemarkTxn]], 2); + yield PT.redo(); + ensureUndoState([[removeTxn], [createLivemarkTxn]], 1); + yield ensureBookmarksTreeRestoredCorrectly(originalInfo); + yield PT.redo(); + ensureUndoState([[removeTxn], [createLivemarkTxn]], 0); + yield ensureNonExistent(guid); + } + + yield undo(); + yield redo(); + yield undo(); + yield redo(); + + // Cleanup + yield undo(); + observer.reset(); + yield PT.clearTransactionsHistory(); +}); + add_task(function* test_edit_title() { let bm_info = { parentGUID: yield PlacesUtils.promiseItemGUID(root) , uri: NetUtil.newURI("http://test_create_item.com") @@ -1157,5 +1280,75 @@ add_task(function* test_livemark_txns() { livemark_info.siteURI = NetUtil.newURI("http://feed.site.uri"); yield* _testDoUndoRedoUndo(); + // Cleanup + observer.reset(); yield PT.clearTransactionsHistory(); }); + +add_task(function* test_copy() { + let rootGUID = yield PlacesUtils.promiseItemGUID(root); + + function* duplicate_and_test(aOriginalGUID) { + yield duplicateGUID = yield PT.transact( + PT.Copy({ GUID: aOriginalGUID, newParentGUID: rootGUID })); + let originalInfo = yield PlacesUtils.promiseBookmarksTree(aOriginalGUID); + let duplicateInfo = yield PlacesUtils.promiseBookmarksTree(duplicateGUID); + yield ensureEqualBookmarksTrees(originalInfo, duplicateInfo, false); + + function* redo() { + yield PT.redo(); + yield ensureBookmarksTreeRestoredCorrectly(originalInfo); + yield PT.redo(); + yield ensureBookmarksTreeRestoredCorrectly(duplicateInfo); + } + function* undo() { + yield PT.undo(); + // also undo the original item addition. + yield PT.undo(); + yield ensureNonExistent(aOriginalGUID, duplicateGUID); + } + + yield undo(); + yield redo(); + yield undo(); + yield redo(); + + // Cleanup. This also remove the original item. + yield PT.undo(); + observer.reset(); + yield PT.clearTransactionsHistory(); + } + + // Test duplicating leafs (bookmark, separator, empty folder) + let bmTxn = PT.NewBookmark({ uri: NetUtil.newURI("http://test.item.duplicate") + , parentGUID: rootGUID + , annos: [{ name: "Anno", value: "AnnoValue"}] }); + let sepTxn = PT.NewSeparator({ parentGUID: rootGUID, index: 1 }); + let livemarkTxn = PT.NewLivemark( + { feedURI: NetUtil.newURI("http://test.feed.uri") + , parentGUID: yield PlacesUtils.promiseItemGUID(root) + , title: "Test Livemark", index: 1 }); + let emptyFolderTxn = PT.NewFolder(yield createTestFolderInfo()); + for (let txn of [livemarkTxn, sepTxn, emptyFolderTxn]) { + let guid = yield PT.transact(txn); + yield duplicate_and_test(guid); + } + + // Test duplicating a folder having some contents. + let filledFolderGUID = yield PT.transact(function *() { + let folderGUID = yield PT.NewFolder(yield createTestFolderInfo()); + let nestedFolderGUID = yield PT.NewFolder({ parentGUID: folderGUID + , title: "Nested Folder" }); + // Insert a bookmark under the nested folder. + yield PT.NewBookmark({ uri: NetUtil.newURI("http://nested.nested.bookmark") + , parentGUID: nestedFolderGUID }); + // Insert a separator below the nested folder + yield PT.NewSeparator({ parentGUID: folderGUID }); + // And another bookmark. + yield PT.NewBookmark({ uri: NetUtil.newURI("http://nested.bookmark") + , parentGUID: folderGUID }); + return folderGUID; + }); + + yield duplicate_and_test(filledFolderGUID); +}); diff --git a/toolkit/components/places/tests/unit/test_promiseBookmarksTree.js b/toolkit/components/places/tests/unit/test_promiseBookmarksTree.js index d930863e025f..ea09161ca90a 100644 --- a/toolkit/components/places/tests/unit/test_promiseBookmarksTree.js +++ b/toolkit/components/places/tests/unit/test_promiseBookmarksTree.js @@ -192,12 +192,16 @@ function* test_promiseBookmarksTreeForEachNode(aNode, aOptions, aExcludedGUIDs) for (let i = 0; i < aNode.childCount; i++) { let child = aNode.getChild(i); if (child.itemId != PlacesUtils.tagsFolderId) - yield test_promiseBookmarksTreeForEachNode(child, {}, aExcludedGUIDs); + yield test_promiseBookmarksTreeForEachNode(child, + { includeItemIds: true }, + aExcludedGUIDs); } return item; } -function* test_promiseBookmarksTreeAgainstResult(aItemGUID = "", aOptions, aExcludedGUIDs) { +function* test_promiseBookmarksTreeAgainstResult(aItemGUID = "", + aOptions = { includeItemIds: true }, + aExcludedGUIDs) { let itemId = aItemGUID ? yield PlacesUtils.promiseItemId(aItemGUID) : PlacesUtils.placesRootId; let node = PlacesUtils.getFolderContents(itemId).root; @@ -246,7 +250,8 @@ add_task(function* () { excludeItemsCallback: aItem => { guidsPassedToExcludeCallback.add(aItem.guid); return aItem.root == "bookmarksMenuFolder"; - } + }, + includeItemIds: true }, [menuGUID]); do_check_eq(guidsPassedToExcludeCallback.size, 4); do_check_eq(placesRootWithoutTheMenu.children.length, 2);