--- a/browser/components/extensions/ext-bookmarks.js
+++ b/browser/components/extensions/ext-bookmarks.js
@@ -313,30 +313,30 @@ this.bookmarks = class extends Extension
remove: function(id) {
let info = {
guid: id,
};
// The API doesn't give you the old bookmark at the moment
try {
- return PlacesUtils.bookmarks.remove(info, {preventRemovalOfNonEmptyFolders: true}).then(result => {})
+ return PlacesUtils.bookmarks.remove(info, {preventRemovalOfNonEmptyFolders: true})
.catch(error => Promise.reject({message: error.message}));
} catch (e) {
return Promise.reject({message: `Invalid bookmark: ${JSON.stringify(info)}`});
}
},
removeTree: function(id) {
let info = {
guid: id,
};
try {
- return PlacesUtils.bookmarks.remove(info).then(result => {})
+ return PlacesUtils.bookmarks.remove(info)
.catch(error => Promise.reject({message: error.message}));
} catch (e) {
return Promise.reject({message: `Invalid bookmark: ${JSON.stringify(info)}`});
}
},
onCreated: new EventManager(context, "bookmarks.onCreated", fire => {
let listener = (event, bookmark) => {
--- a/browser/components/places/content/controller.js
+++ b/browser/components/places/content/controller.js
@@ -868,27 +868,33 @@ PlacesController.prototype = {
* Creates a set of transactions for the removal of a range of items.
* A range is an array of adjacent nodes in a view.
* @param [in] range
* An array of nodes to remove. Should all be adjacent.
* @param [out] transactions
* An array of transactions.
* @param [optional] removedFolders
* An array of folder nodes that have already been removed.
+ * @return {Integer} The total number of items affected.
*/
async _removeRange(range, transactions, removedFolders) {
NS_ASSERT(transactions instanceof Array, "Must pass a transactions array");
if (!removedFolders)
removedFolders = [];
+ let bmGuidsToRemove = [];
+ let totalItems = 0;
+
for (var i = 0; i < range.length; ++i) {
var node = range[i];
if (this._shouldSkipNode(node, removedFolders))
continue;
+ totalItems++;
+
if (PlacesUtils.nodeIsTagQuery(node.parent)) {
// This is a uri node inside a tag container. It needs a special
// untag transaction.
var tagItemId = PlacesUtils.getConcreteItemId(node.parent);
var uri = NetUtil.newURI(node.uri);
if (PlacesUIUtils.useAsyncTransactions) {
let tag = node.parent.title;
if (!tag) {
@@ -937,43 +943,47 @@ PlacesController.prototype = {
} else {
// This is a common bookmark item.
if (PlacesUtils.nodeIsFolder(node)) {
// If this is a folder we add it to our array of folders, used
// to skip nodes that are children of an already removed folder.
removedFolders.push(node);
}
if (PlacesUIUtils.useAsyncTransactions) {
- transactions.push(
- PlacesTransactions.Remove({ guid: node.bookmarkGuid }));
+ bmGuidsToRemove.push(node.bookmarkGuid);
} else {
let txn = new PlacesRemoveItemTransaction(node.itemId);
transactions.push(txn);
}
}
}
+ if (bmGuidsToRemove.length) {
+ transactions.push(PlacesTransactions.Remove({ guids: bmGuidsToRemove }));
+ }
+ return totalItems;
},
/**
* Removes the set of selected ranges from bookmarks.
* @param txnName
* See |remove|.
*/
async _removeRowsFromBookmarks(txnName) {
- var ranges = this._view.removableSelectionRanges;
- var transactions = [];
- var removedFolders = [];
+ let ranges = this._view.removableSelectionRanges;
+ let transactions = [];
+ let removedFolders = [];
+ let totalItems = 0;
for (let range of ranges) {
- await this._removeRange(range, transactions, removedFolders);
+ totalItems += await this._removeRange(range, transactions, removedFolders);
}
if (transactions.length > 0) {
if (PlacesUIUtils.useAsyncTransactions) {
- await PlacesUIUtils.batchUpdatesForNode(this._view.result, transactions.length, async () => {
+ await PlacesUIUtils.batchUpdatesForNode(this._view.result, totalItems, async () => {
await PlacesTransactions.batch(transactions);
});
} else {
var txn = new PlacesAggregatedTransaction(txnName, transactions);
PlacesUtils.transactionManager.doTransaction(txn);
}
}
},
--- a/toolkit/components/places/Bookmarks.jsm
+++ b/toolkit/components/places/Bookmarks.jsm
@@ -733,84 +733,100 @@ var Bookmarks = Object.freeze({
// Remove non-enumerable properties.
delete updatedItem.source;
return Object.assign({}, updatedItem);
});
})();
},
/**
- * Removes a bookmark-item.
+ * Removes one or more bookmark-items.
*
- * @param guidOrInfo
- * The globally unique identifier of the item to remove, or an
- * object representing it, as defined above.
+ * @param guidOrInfo This may be:
+ * - The globally unique identifier of the item to remove
+ * - an object representing the item, as defined above
+ * - an array of objects representing the items to be removed
* @param {Object} [options={}]
* Additional options that can be passed to the function.
* Currently supports the following properties:
* - preventRemovalOfNonEmptyFolders: Causes an exception to be
* thrown when attempting to remove a folder that is not empty.
* - source: The change source, forwarded to all bookmark observers.
* Defaults to nsINavBookmarksService::SOURCE_DEFAULT.
*
- * @return {Promise} resolved when the removal is complete.
- * @resolves to an object representing the removed bookmark.
+ * @return {Promise}
+ * @resolves when the removal is complete
* @rejects if the provided guid doesn't match any existing bookmark.
* @throws if the arguments are invalid.
*/
remove(guidOrInfo, options = {}) {
- let info = guidOrInfo;
- if (!info)
+ let infos = guidOrInfo;
+ if (!infos)
throw new Error("Input should be a valid object");
- if (typeof(guidOrInfo) != "object")
- info = { guid: guidOrInfo };
-
- // Disallow removing the root folders.
- if ([this.rootGuid, this.menuGuid, this.toolbarGuid, this.unfiledGuid,
- this.tagsGuid, this.mobileGuid].includes(info.guid)) {
- throw new Error("It's not possible to remove Places root folders.");
+ if (!Array.isArray(guidOrInfo)) {
+ if (typeof(guidOrInfo) != "object") {
+ infos = [{ guid: guidOrInfo }];
+ } else {
+ infos = [guidOrInfo];
+ }
}
if (!("source" in options)) {
options.source = Bookmarks.SOURCES.DEFAULT;
}
- // Even if we ignore any other unneeded property, we still validate any
- // known property to reduce likelihood of hidden bugs.
- let removeInfo = validateBookmarkObject("Bookmarks.jsm: remove", info);
+ let removeInfos = [];
+ for (let info of infos) {
+ // Disallow removing the root folders.
+ if ([
+ Bookmarks.rootGuid, Bookmarks.menuGuid, Bookmarks.toolbarGuid,
+ Bookmarks.unfiledGuid, Bookmarks.tagsGuid, Bookmarks.mobileGuid
+ ].includes(info.guid)) {
+ throw new Error("It's not possible to remove Places root folders.");
+ }
+
+ // Even if we ignore any other unneeded property, we still validate any
+ // known property to reduce likelihood of hidden bugs.
+ let removeInfo = validateBookmarkObject("Bookmarks.jsm: remove", info);
+ removeInfos.push(removeInfo);
+ }
return (async function() {
- let item = await fetchBookmark(removeInfo);
- if (!item)
- throw new Error("No bookmarks found for the provided GUID.");
+ let removeItems = [];
+ for (let info of removeInfos) {
+ let item = await fetchBookmark(info);
+ if (!item)
+ throw new Error("No bookmarks found for the provided GUID.");
- item = await removeBookmark(item, options);
+ removeItems.push(item);
+ }
+
+ await removeBookmarks(removeItems, options);
// Notify onItemRemoved to listeners.
- let observers = PlacesUtils.bookmarks.getObservers();
- let uri = item.hasOwnProperty("url") ? PlacesUtils.toURI(item.url) : null;
- let isUntagging = item._grandParentId == PlacesUtils.tagsFolderId;
- notify(observers, "onItemRemoved", [ item._id, item._parentId, item.index,
- item.type, uri, item.guid,
- item.parentGuid,
- options.source ],
- { isTagging: isUntagging });
+ for (let item of removeItems) {
+ let observers = PlacesUtils.bookmarks.getObservers();
+ let uri = item.hasOwnProperty("url") ? PlacesUtils.toURI(item.url) : null;
+ let isUntagging = item._grandParentId == PlacesUtils.tagsFolderId;
+ notify(observers, "onItemRemoved", [ item._id, item._parentId, item.index,
+ item.type, uri, item.guid,
+ item.parentGuid,
+ options.source ],
+ { isTagging: isUntagging });
- if (isUntagging) {
- for (let entry of (await fetchBookmarksByURL(item, true))) {
- notify(observers, "onItemChanged", [ entry._id, "tags", false, "",
- PlacesUtils.toPRTime(entry.lastModified),
- entry.type, entry._parentId,
- entry.guid, entry.parentGuid,
- "", options.source ]);
+ if (isUntagging) {
+ for (let entry of (await fetchBookmarksByURL(item, true))) {
+ notify(observers, "onItemChanged", [ entry._id, "tags", false, "",
+ PlacesUtils.toPRTime(entry.lastModified),
+ entry.type, entry._parentId,
+ entry.guid, entry.parentGuid,
+ "", options.source ]);
+ }
}
}
-
- // Remove non-enumerable properties.
- return Object.assign({}, item);
})();
},
/**
* Removes ALL bookmarks, resetting the bookmarks storage to an empty tree.
*
* Note that roots are preserved, only their children will be removed.
*
@@ -1505,17 +1521,17 @@ function insertBookmarkTree(items, sourc
*
* @param {Object} item Livemark item that need to be added.
*/
async function insertLivemarkData(item) {
// Delete the placeholder but note the index of it, so that we can insert the
// livemark item at the right place.
let placeholder = await Bookmarks.fetch(item.guid);
let index = placeholder.index;
- await removeBookmark(item, {source: item.source});
+ await removeBookmarks([item], {source: item.source});
let feedURI = null;
let siteURI = null;
item.annos = item.annos.filter(function(aAnno) {
switch (aAnno.name) {
case PlacesUtils.LMANNO_FEEDURI:
feedURI = NetUtil.newURI(aAnno.value);
return false;
@@ -1782,80 +1798,93 @@ async function fetchBookmarksByParent(db
ORDER BY b.position ASC
`, { parentGuid: info.parentGuid });
return rowsToItemsArray(rows);
}
// Remove implementation.
-function removeBookmark(item, options) {
- return PlacesUtils.withConnectionWrapper("Bookmarks.jsm: removeBookmark",
+function removeBookmarks(items, options) {
+ return PlacesUtils.withConnectionWrapper("Bookmarks.jsm: removeBookmarks",
async function(db) {
- let urls;
- let isUntagging = item._grandParentId == PlacesUtils.tagsFolderId;
+ let urls = [];
await db.executeTransaction(async function transaction() {
- // If it's a folder, remove its contents first.
- if (item.type == Bookmarks.TYPE_FOLDER) {
- if (options.preventRemovalOfNonEmptyFolders && item._childCount > 0) {
- throw new Error("Cannot remove a non-empty folder.");
+ let parentGuids = new Set();
+ let syncChangeDelta =
+ PlacesSyncUtils.bookmarks.determineSyncChangeDelta(options.source);
+
+ for (let item of items) {
+ parentGuids.add(item.parentGuid);
+
+ // If it's a folder, remove its contents first.
+ if (item.type == Bookmarks.TYPE_FOLDER) {
+ if (options.preventRemovalOfNonEmptyFolders && item._childCount > 0) {
+ throw new Error("Cannot remove a non-empty folder.");
+ }
+ urls = urls.concat(await removeFoldersContents(db, [item.guid], options));
}
- urls = await removeFoldersContents(db, [item.guid], options);
}
- // Remove annotations first. If it's a tag, we can avoid paying that cost.
- if (!isUntagging) {
+ for (let chunk of chunkArray(items, SQLITE_MAX_VARIABLE_NUMBER)) {
// We don't go through the annotations service for this cause otherwise
// we'd get a pointless onItemChanged notification and it would also
// set lastModified to an unexpected value.
- await removeAnnotationsForItem(db, item._id);
+ await removeAnnotationsForItems(db, chunk);
+
+ // Remove the bookmarks.
+ await db.executeCached(
+ `DELETE FROM moz_bookmarks WHERE guid IN (${
+ new Array(chunk.length).fill("?").join(",")})`,
+ chunk.map(item => item.guid)
+ );
}
- // Remove the bookmark from the database.
- await db.executeCached(
- `DELETE FROM moz_bookmarks WHERE guid = :guid`, { guid: item.guid });
-
- // Fix indices in the parent.
- await db.executeCached(
- `UPDATE moz_bookmarks SET position = position - 1 WHERE
- parent = :parentId AND position > :index
- `, { parentId: item._parentId, index: item.index });
+ for (let item of items) {
+ // Fix indices in the parent.
+ await db.executeCached(
+ `UPDATE moz_bookmarks SET position = position - 1 WHERE
+ parent = :parentId AND position > :index
+ `, { parentId: item._parentId, index: item.index });
- let syncChangeDelta =
- PlacesSyncUtils.bookmarks.determineSyncChangeDelta(options.source);
-
- // Mark all affected separators as changed
- await adjustSeparatorsSyncCounter(db, item._parentId, item.index, syncChangeDelta);
+ if (item._grandParentId == PlacesUtils.tagsFolderId) {
+ // If we're removing a tag entry, increment the change counter for all
+ // bookmarks with the tagged URL.
+ await PlacesSyncUtils.bookmarks.addSyncChangesForBookmarksWithURL(
+ db, item.url, syncChangeDelta);
+ }
- if (isUntagging) {
- // If we're removing a tag entry, increment the change counter for all
- // bookmarks with the tagged URL.
- await PlacesSyncUtils.bookmarks.addSyncChangesForBookmarksWithURL(
- db, item.url, syncChangeDelta);
+ await adjustSeparatorsSyncCounter(db, item._parentId, item.index, syncChangeDelta);
+ }
+
+ for (let guid of parentGuids) {
+ // Mark all affected parents as changed.
+ await setAncestorsLastModified(db, guid, new Date(), syncChangeDelta);
}
// Write a tombstone for the removed item.
- await insertTombstone(db, item, syncChangeDelta);
-
- await setAncestorsLastModified(db, item.parentGuid, new Date(),
- syncChangeDelta);
+ await insertTombstones(db, items, syncChangeDelta);
});
- // If not a tag recalculate frecency...
- if (item.type == Bookmarks.TYPE_BOOKMARK && !isUntagging) {
- // ...though we don't wait for the calculation.
- updateFrecency(db, [item.url]).catch(Cu.reportError);
+ // Update the frecencies outside of the transaction, so that the updates
+ // can progress in the background.
+ for (let item of items) {
+ let isUntagging = item._grandParentId == PlacesUtils.tagsFolderId;
+
+ // If not a tag recalculate frecency...
+ if (item.type == Bookmarks.TYPE_BOOKMARK && !isUntagging) {
+ // ...though we don't wait for the calculation.
+ updateFrecency(db, [item.url]).catch(Cu.reportError);
+ }
}
- if (urls && urls.length && item.type == Bookmarks.TYPE_FOLDER) {
+ if (urls.length) {
updateFrecency(db, urls, true).catch(Cu.reportError);
}
-
- return item;
});
}
// Reorder implementation.
function reorderChildren(parent, orderedChildrenGuids, options) {
return PlacesUtils.withConnectionWrapper("Bookmarks.jsm: reorderChildren",
db => db.executeTransaction(async function() {
@@ -2148,24 +2177,25 @@ var removeOrphanAnnotations = async func
`);
};
/**
* Removes annotations for a given item.
*
* @param db
* the Sqlite.jsm connection handle.
- * @param itemId
- * internal id of the item for which to remove annotations.
+ * @param items
+ * The items for which to remove annotations.
*/
-var removeAnnotationsForItem = async function(db, itemId) {
+var removeAnnotationsForItems = async function(db, items) {
+ // Remove the annotations.
+ let ids = sqlList(items.map(item => item._id));
await db.executeCached(
- `DELETE FROM moz_items_annos
- WHERE item_id = :id
- `, { id: itemId });
+ `DELETE FROM moz_items_annos WHERE item_id IN (${ids})`,
+ );
await db.executeCached(
`DELETE FROM moz_anno_attributes
WHERE id IN (SELECT n.id from moz_anno_attributes n
LEFT JOIN moz_annos a1 ON a1.anno_attribute_id = n.id
LEFT JOIN moz_items_annos a2 ON a2.anno_attribute_id = n.id
WHERE a1.id ISNULL AND a2.id ISNULL)
`);
};
@@ -2447,8 +2477,16 @@ function* chunkArray(array, chunkLength)
yield array;
return;
}
let startIndex = 0;
while (startIndex < array.length) {
yield array.slice(startIndex, startIndex += chunkLength);
}
}
+
+/**
+ * Convert a list of strings or numbers to its SQL
+ * representation as a string.
+ */
+function sqlList(list) {
+ return list.map(JSON.stringify).join();
+}
--- a/toolkit/components/places/PlacesTransactions.jsm
+++ b/toolkit/components/places/PlacesTransactions.jsm
@@ -1484,39 +1484,43 @@ PT.SortByName.prototype = {
/**
* Transaction for removing an item (any type).
*
* Required Input Properties: guids.
*/
PT.Remove = DefineTransaction(["guids"]);
PT.Remove.prototype = {
async execute({ guids }) {
- let promiseBookmarksTree = async function(guid) {
- let tree;
+ let removedItems = [];
+
+ for (let guid of guids) {
try {
- tree = await PlacesUtils.promiseBookmarksTree(guid);
+ // Although we don't strictly need to get this information for the remove,
+ // we do need it for the possibility of undo().
+ removedItems.push(await PlacesUtils.promiseBookmarksTree(guid));
} catch (ex) {
throw new Error("Failed to get info for the specified item (guid: " +
guid + "): " + ex);
}
- return tree;
- };
- let removedItems = [];
- for (let guid of guids) {
- removedItems.push(await promiseBookmarksTree(guid));
}
+
let removeThem = async function() {
+ let bmsToRemove = [];
for (let info of removedItems) {
if (info.annos &&
info.annos.some(anno => anno.name == PlacesUtils.LMANNO_FEEDURI)) {
await PlacesUtils.livemarks.removeLivemark({ guid: info.guid });
} else {
- await PlacesUtils.bookmarks.remove({ guid: info.guid });
+ bmsToRemove.push({guid: info.guid});
}
}
+
+ if (bmsToRemove.length) {
+ await PlacesUtils.bookmarks.remove(bmsToRemove);
+ }
};
await removeThem();
this.undo = async function() {
for (let info of removedItems) {
await createItemsFromBookmarksTree(info, true);
}
};
--- a/toolkit/components/places/tests/bookmarks/test_bookmarks_notifications.js
+++ b/toolkit/components/places/tests/bookmarks/test_bookmarks_notifications.js
@@ -225,34 +225,61 @@ add_task(async function update_move_diff
add_task(async function remove_bookmark() {
let bm = await PlacesUtils.bookmarks.insert({ type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
parentGuid: PlacesUtils.bookmarks.unfiledGuid,
url: new URL("http://remove.example.com/") });
let itemId = await PlacesUtils.promiseItemId(bm.guid);
let parentId = await PlacesUtils.promiseItemId(bm.parentGuid);
let observer = expectNotifications();
- bm = await PlacesUtils.bookmarks.remove(bm.guid);
+ await PlacesUtils.bookmarks.remove(bm.guid);
// TODO (Bug 653910): onItemAnnotationRemoved notified even if there were no
// annotations.
observer.check([ { name: "onItemRemoved",
arguments: [ itemId, parentId, bm.index, bm.type, bm.url,
bm.guid, bm.parentGuid,
Ci.nsINavBookmarksService.SOURCE_DEFAULT ] }
]);
});
+add_task(async function remove_multiple_bookmarks() {
+ let bm1 = await PlacesUtils.bookmarks.insert({ type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
+ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
+ url: "http://remove.example.com/" });
+ let bm2 = await PlacesUtils.bookmarks.insert({ type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
+ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
+ url: "http://remove1.example.com/" });
+ let itemId1 = await PlacesUtils.promiseItemId(bm1.guid);
+ let parentId1 = await PlacesUtils.promiseItemId(bm1.parentGuid);
+ let itemId2 = await PlacesUtils.promiseItemId(bm2.guid);
+ let parentId2 = await PlacesUtils.promiseItemId(bm2.parentGuid);
+
+ let observer = expectNotifications();
+ await PlacesUtils.bookmarks.remove([bm1, bm2]);
+ // TODO (Bug 653910): onItemAnnotationRemoved notified even if there were no
+ // annotations.
+ observer.check([ { name: "onItemRemoved",
+ arguments: [ itemId1, parentId1, bm1.index, bm1.type, bm1.url,
+ bm1.guid, bm1.parentGuid,
+ Ci.nsINavBookmarksService.SOURCE_DEFAULT ] },
+ { name: "onItemRemoved",
+ arguments: [ itemId2, parentId2, bm2.index, bm2.type, bm2.url,
+ bm2.guid, bm2.parentGuid,
+ Ci.nsINavBookmarksService.SOURCE_DEFAULT ] }
+ ]);
+});
+
add_task(async function remove_folder() {
let bm = await PlacesUtils.bookmarks.insert({ type: PlacesUtils.bookmarks.TYPE_FOLDER,
parentGuid: PlacesUtils.bookmarks.unfiledGuid });
let itemId = await PlacesUtils.promiseItemId(bm.guid);
let parentId = await PlacesUtils.promiseItemId(bm.parentGuid);
let observer = expectNotifications();
- bm = await PlacesUtils.bookmarks.remove(bm.guid);
+ await PlacesUtils.bookmarks.remove(bm.guid);
observer.check([ { name: "onItemRemoved",
arguments: [ itemId, parentId, bm.index, bm.type, null,
bm.guid, bm.parentGuid,
Ci.nsINavBookmarksService.SOURCE_DEFAULT ] }
]);
});
add_task(async function remove_bookmark_tag_notification() {
--- a/toolkit/components/places/tests/bookmarks/test_bookmarks_remove.js
+++ b/toolkit/components/places/tests/bookmarks/test_bookmarks_remove.js
@@ -62,26 +62,25 @@ add_task(async function remove_roots_fai
let guids = [PlacesUtils.bookmarks.rootGuid,
PlacesUtils.bookmarks.unfiledGuid,
PlacesUtils.bookmarks.menuGuid,
PlacesUtils.bookmarks.toolbarGuid,
PlacesUtils.bookmarks.tagsGuid,
PlacesUtils.bookmarks.mobileGuid];
for (let guid of guids) {
Assert.throws(() => PlacesUtils.bookmarks.remove(guid),
- /It's not possible to remove Places root folders/);
+ /It's not possible to remove Places root folders\./);
}
});
add_task(async function remove_normal_folder_under_root_succeeds() {
let folder = await PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.rootGuid,
type: PlacesUtils.bookmarks.TYPE_FOLDER });
checkBookmarkObject(folder);
- let removed_folder = await PlacesUtils.bookmarks.remove(folder);
- Assert.deepEqual(folder, removed_folder);
+ await PlacesUtils.bookmarks.remove(folder);
Assert.strictEqual((await PlacesUtils.bookmarks.fetch(folder.guid)), null);
});
add_task(async function remove_bookmark() {
// When removing a bookmark we need to check the frecency. First we confirm
// that there is a normal update when it is inserted.
let frecencyChangedPromise = promiseFrecencyChanged("http://example.com/",
UNVISITED_BOOKMARK_BONUS);
@@ -91,42 +90,61 @@ add_task(async function remove_bookmark(
title: "a bookmark" });
checkBookmarkObject(bm1);
await frecencyChangedPromise;
// This second one checks the frecency is changed when we remove the bookmark.
frecencyChangedPromise = promiseFrecencyChanged("http://example.com/", 0);
- let bm2 = await PlacesUtils.bookmarks.remove(bm1.guid);
- checkBookmarkObject(bm2);
-
- Assert.deepEqual(bm1, bm2);
- Assert.equal(bm2.parentGuid, PlacesUtils.bookmarks.unfiledGuid);
- Assert.equal(bm2.index, 0);
- Assert.deepEqual(bm2.dateAdded, bm2.lastModified);
- Assert.equal(bm2.type, PlacesUtils.bookmarks.TYPE_BOOKMARK);
- Assert.equal(bm2.url.href, "http://example.com/");
- Assert.equal(bm2.title, "a bookmark");
+ await PlacesUtils.bookmarks.remove(bm1.guid);
await frecencyChangedPromise;
});
+add_task(async function remove_multiple_bookmarks() {
+ // When removing a bookmark we need to check the frecency. First we confirm
+ // that there is a normal update when it is inserted.
+ let frecencyChangedPromise = promiseFrecencyChanged("http://example.com/",
+ UNVISITED_BOOKMARK_BONUS);
+ let bm1 = await PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
+ type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
+ url: "http://example.com/",
+ title: "a bookmark" });
+ checkBookmarkObject(bm1);
+
+ let frecencyChangedPromise1 = promiseFrecencyChanged("http://example1.com/",
+ UNVISITED_BOOKMARK_BONUS);
+ let bm2 = await PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
+ type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
+ url: "http://example1.com/",
+ title: "a bookmark" });
+ checkBookmarkObject(bm2);
+
+ await Promise.all([frecencyChangedPromise, frecencyChangedPromise1]);
+
+ // This second one checks the frecency is changed when we remove the bookmark.
+ frecencyChangedPromise = promiseFrecencyChanged("http://example.com/", 0);
+ frecencyChangedPromise1 = promiseFrecencyChanged("http://example1.com/", 0);
+
+ await PlacesUtils.bookmarks.remove([bm1, bm2]);
+
+ await Promise.all([frecencyChangedPromise, frecencyChangedPromise1]);
+});
add_task(async function remove_bookmark_orphans() {
let bm1 = await PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
url: "http://example.com/",
title: "a bookmark" });
checkBookmarkObject(bm1);
PlacesUtils.annotations.setItemAnnotation((await PlacesUtils.promiseItemId(bm1.guid)),
"testanno", "testvalue", 0, 0);
- let bm2 = await PlacesUtils.bookmarks.remove(bm1.guid);
- checkBookmarkObject(bm2);
+ await PlacesUtils.bookmarks.remove(bm1.guid);
// Check there are no orphan annotations.
let conn = await PlacesUtils.promiseDBConnection();
let annoAttrs = await conn.execute(`SELECT id, name FROM moz_anno_attributes`);
// Bug 1306445 will eventually remove the mobile root anno.
Assert.equal(annoAttrs.length, 1);
Assert.equal(annoAttrs[0].getResultByName("name"), PlacesUtils.MOBILE_ROOT_ANNO);
let annos = await conn.execute(`SELECT item_id, anno_attribute_id FROM moz_items_annos`);
@@ -137,40 +155,28 @@ add_task(async function remove_bookmark_
add_task(async function remove_bookmark_empty_title() {
let bm1 = await PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
url: "http://example.com/",
title: "" });
checkBookmarkObject(bm1);
- let bm2 = await PlacesUtils.bookmarks.remove(bm1.guid);
- checkBookmarkObject(bm2);
-
- Assert.deepEqual(bm1, bm2);
- Assert.equal(bm2.index, 0);
- Assert.strictEqual(bm2.title, "");
+ await PlacesUtils.bookmarks.remove(bm1.guid);
+ Assert.strictEqual((await PlacesUtils.bookmarks.fetch(bm1.guid)), null);
});
add_task(async function remove_folder() {
let bm1 = await PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
type: PlacesUtils.bookmarks.TYPE_FOLDER,
title: "a folder" });
checkBookmarkObject(bm1);
- let bm2 = await PlacesUtils.bookmarks.remove(bm1.guid);
- checkBookmarkObject(bm2);
-
- Assert.deepEqual(bm1, bm2);
- Assert.equal(bm2.parentGuid, PlacesUtils.bookmarks.unfiledGuid);
- Assert.equal(bm2.index, 0);
- Assert.deepEqual(bm2.dateAdded, bm2.lastModified);
- Assert.equal(bm2.type, PlacesUtils.bookmarks.TYPE_FOLDER);
- Assert.equal(bm2.title, "a folder");
- Assert.ok(!("url" in bm2));
+ await PlacesUtils.bookmarks.remove(bm1.guid);
+ Assert.strictEqual((await PlacesUtils.bookmarks.fetch(bm1.guid)), null);
// No wait for onManyFrecenciesChanged in this test as the folder doesn't have
// any children that would need updating.
});
add_task(async function test_contents_removed() {
let folder1 = await PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
type: PlacesUtils.bookmarks.TYPE_FOLDER,
@@ -241,39 +247,27 @@ add_task(async function test_nested_cont
});
add_task(async function remove_folder_empty_title() {
let bm1 = await PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
type: PlacesUtils.bookmarks.TYPE_FOLDER,
title: "" });
checkBookmarkObject(bm1);
- let bm2 = await PlacesUtils.bookmarks.remove(bm1.guid);
- checkBookmarkObject(bm2);
-
- Assert.deepEqual(bm1, bm2);
- Assert.equal(bm2.index, 0);
- Assert.strictEqual(bm2.title, "");
+ await PlacesUtils.bookmarks.remove(bm1.guid);
+ Assert.strictEqual((await PlacesUtils.bookmarks.fetch(bm1.guid)), null);
});
add_task(async function remove_separator() {
let bm1 = await PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
type: PlacesUtils.bookmarks.TYPE_SEPARATOR });
checkBookmarkObject(bm1);
- let bm2 = await PlacesUtils.bookmarks.remove(bm1.guid);
- checkBookmarkObject(bm2);
-
- Assert.deepEqual(bm1, bm2);
- Assert.equal(bm2.parentGuid, PlacesUtils.bookmarks.unfiledGuid);
- Assert.equal(bm2.index, 0);
- Assert.deepEqual(bm2.dateAdded, bm2.lastModified);
- Assert.equal(bm2.type, PlacesUtils.bookmarks.TYPE_SEPARATOR);
- Assert.ok(!("url" in bm2));
- Assert.strictEqual(bm2.title, "");
+ await PlacesUtils.bookmarks.remove(bm1.guid);
+ Assert.strictEqual((await PlacesUtils.bookmarks.fetch(bm1.guid)), null);
});
add_task(async function test_nested_content_fails_when_not_allowed() {
let folder1 = await PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
type: PlacesUtils.bookmarks.TYPE_FOLDER,
title: "a folder" });
await PlacesUtils.bookmarks.insert({ parentGuid: folder1.guid,
type: PlacesUtils.bookmarks.TYPE_FOLDER,
--- a/toolkit/modules/NewTabUtils.jsm
+++ b/toolkit/modules/NewTabUtils.jsm
@@ -1263,18 +1263,17 @@ var ActivityStreamLinks = {
},
/**
* Removes a bookmark
*
* @param {String} aBookmarkGuid
* The bookmark guid associated with the bookmark to remove
*
- * @returns {Promise} Returns a promise set to an object representing the
- * removed bookmark
+ * @returns {Promise} Returns a promise at completion.
*/
deleteBookmark(aBookmarkGuid) {
return PlacesUtils.bookmarks.remove(aBookmarkGuid);
},
/**
* Removes a history link and unpins the URL if previously pinned
*
--- a/toolkit/modules/tests/xpcshell/test_NewTabUtils.js
+++ b/toolkit/modules/tests/xpcshell/test_NewTabUtils.js
@@ -730,19 +730,19 @@ add_task(async function activityStream_d
await PlacesUtils.bookmarks.insert(placeInfo);
}
bookmarksSize = await getBookmarksSize();
Assert.equal(bookmarksSize, 2, "size 2 for 2 bookmarks added");
let bookmarkGuid = await new Promise(resolve => PlacesUtils.bookmarks.fetch(
{url: bookmarks[0].url}, bookmark => resolve(bookmark.guid)));
- let deleted = await provider.deleteBookmark(bookmarkGuid);
- Assert.equal(deleted.guid, bookmarkGuid, "the correct bookmark was deleted");
-
+ await provider.deleteBookmark(bookmarkGuid);
+ Assert.strictEqual((await PlacesUtils.bookmarks.fetch(bookmarkGuid)), null,
+ "the bookmark should no longer be found");
bookmarksSize = await getBookmarksSize();
Assert.equal(bookmarksSize, 1, "size 1 after deleting");
});
add_task(async function activityStream_blockedURLs() {
await setUpActivityStreamTest();
let provider = NewTabUtils.activityStreamLinks;
@@ -789,9 +789,8 @@ TestProvider.prototype = {
let args = Array.prototype.slice.call(arguments, 1);
args.unshift(this);
for (let obs of this._observers) {
if (obs[observerMethodName])
obs[observerMethodName].apply(NewTabUtils.links, args);
}
},
};
-