--- a/toolkit/components/places/Bookmarks.jsm
+++ b/toolkit/components/places/Bookmarks.jsm
@@ -451,93 +451,25 @@ let Bookmarks = Object.freeze({
*
* Note that roots are preserved, only their children will be removed.
*
* @return {Promise} resolved when the removal is complete.
* @resolves once the removal is complete.
*/
eraseEverything: Task.async(function* () {
let db = yield DBConnPromised;
-
yield db.executeTransaction(function* () {
- let rows = yield db.executeCached(
- `WITH RECURSIVE
- descendants(did) AS (
- SELECT b.id FROM moz_bookmarks b
- JOIN moz_bookmarks p ON b.parent = p.id
- WHERE p.guid IN ( :toolbarGuid, :menuGuid, :unfiledGuid )
- UNION ALL
- SELECT id FROM moz_bookmarks
- JOIN descendants ON parent = did
- )
- SELECT b.id AS _id, b.parent AS _parentId, b.position AS 'index',
- b.type, url, b.guid, p.guid AS parentGuid, b.dateAdded,
- b.lastModified, b.title, p.parent AS _grandParentId,
- NULL AS _childCount, NULL AS keyword
- FROM moz_bookmarks b
- JOIN moz_bookmarks p ON p.id = b.parent
- LEFT JOIN moz_places h ON b.fk = h.id
- WHERE b.id IN descendants
- `, { menuGuid: this.menuGuid, toolbarGuid: this.toolbarGuid,
- unfiledGuid: this.unfiledGuid });
- let items = rowsToItemsArray(rows);
-
- yield db.executeCached(
- `WITH RECURSIVE
- descendants(did) AS (
- SELECT b.id FROM moz_bookmarks b
- JOIN moz_bookmarks p ON b.parent = p.id
- WHERE p.guid IN ( :toolbarGuid, :menuGuid, :unfiledGuid )
- UNION ALL
- SELECT id FROM moz_bookmarks
- JOIN descendants ON parent = did
- )
- DELETE FROM moz_bookmarks WHERE id IN descendants
- `, { menuGuid: this.menuGuid, toolbarGuid: this.toolbarGuid,
- unfiledGuid: this.unfiledGuid });
-
- // Clenup orphans.
- yield removeOrphanAnnotations(db);
- yield removeOrphanKeywords(db);
-
- // TODO (Bug 1087576): this may leave orphan tags behind.
-
- // Update roots' lastModified.
- yield db.executeCached(
- `UPDATE moz_bookmarks SET lastModified = :time
- WHERE id IN (SELECT id FROM moz_bookmarks
- WHERE guid IN ( :rootGuid, :toolbarGuid, :menuGuid, :unfiledGuid ))
- `, { time: toPRTime(new Date()), rootGuid: this.rootGuid,
- menuGuid: this.menuGuid, toolbarGuid: this.toolbarGuid,
- unfiledGuid: this.unfiledGuid });
-
- let urls = [for (item of items) if (item.url) item.url];
- updateFrecency(db, urls).then(null, Cu.reportError);
-
- // Send onItemRemoved notifications to listeners.
- // TODO (Bug 1087580): this should send a single clear bookmarks
- // notification rather than notifying for each bookmark.
-
- // Notify listeners in reverse order to serve children before parents.
- let observers = PlacesUtils.bookmarks.getObservers();
- for (let item of items.reverse()) {
- let uri = item.hasOwnProperty("url") ? toURI(item.url) : null;
- notify(observers, "onItemRemoved", [ item._id, item._parentId,
- item.index, item.type, uri,
- item.guid, item.parentGuid ]);
-
- let isUntagging = item._grandParentId == PlacesUtils.tagsFolderId;
- if (isUntagging) {
- for (let entry of (yield fetchBookmarksByURL(item))) {
- notify(observers, "onItemChanged", [ entry._id, "tags", false, "",
- toPRTime(entry.lastModified),
- entry.type, entry._parentId,
- entry.guid, entry.parentGuid ]);
- }
- }
+ const folderGuids = [this.toolbarGuid, this.menuGuid, this.unfiledGuid];
+ yield removeFoldersContents(db, folderGuids);
+ const time = toPRTime(new Date());
+ for (let folderGuid of folderGuids) {
+ yield db.executeCached(
+ `UPDATE moz_bookmarks SET lastModified = :time
+ WHERE id IN (SELECT id FROM moz_bookmarks WHERE guid = :folderGuid )
+ `, { folderGuid, time });
}
}.bind(this));
}),
/**
* Fetches information about a bookmark-item.
*
* REMARK: any successful call to this method resolves to a single
@@ -1021,16 +953,20 @@ function* fetchBookmarksByKeyword(info)
// Remove implementation.
function* removeBookmark(item) {
let db = yield DBConnPromised;
let isUntagging = item._grandParentId == PlacesUtils.tagsFolderId;
yield db.executeTransaction(function* transaction() {
+ // If it's a folder, remove its contents first.
+ if (item.type == Bookmarks.TYPE_FOLDER)
+ yield removeFoldersContents(db, [item.guid]);
+
// Remove annotations first. If it's a tag, we can avoid paying that cost.
if (!isUntagging) {
// 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.
yield removeAnnotationsForItem(db, item._id);
}
@@ -1409,8 +1345,88 @@ let setAncestorsLastModified = Task.asyn
JOIN ancestors ON id = aid
WHERE type = :type
)
UPDATE moz_bookmarks SET lastModified = :time
WHERE id IN ancestors
`, { guid: folderGuid, type: Bookmarks.TYPE_FOLDER,
time: toPRTime(time) });
});
+
+/**
+ * Remove all descendants of one or more bookmark folders.
+ *
+ * @param db
+ * the Sqlite.jsm connection handle.
+ * @param folderGuids
+ * array of folder guids.
+ */
+let removeFoldersContents =
+Task.async(function* (db, folderGuids) {
+ let itemsRemoved = [];
+ for (let folderGuid of folderGuids) {
+ let rows = yield db.executeCached(
+ `WITH RECURSIVE
+ descendants(did) AS (
+ SELECT b.id FROM moz_bookmarks b
+ JOIN moz_bookmarks p ON b.parent = p.id
+ WHERE p.guid = :folderGuid
+ UNION ALL
+ SELECT id FROM moz_bookmarks
+ JOIN descendants ON parent = did
+ )
+ SELECT b.id AS _id, b.parent AS _parentId, b.position AS 'index',
+ b.type, url, b.guid, p.guid AS parentGuid, b.dateAdded,
+ b.lastModified, b.title, p.parent AS _grandParentId,
+ NULL AS _childCount, NULL AS keyword
+ FROM moz_bookmarks b
+ JOIN moz_bookmarks p ON p.id = b.parent
+ LEFT JOIN moz_places h ON b.fk = h.id
+ WHERE b.id IN descendants`, { folderGuid });
+
+ itemsRemoved = itemsRemoved.concat(rowsToItemsArray(rows));
+
+ yield db.executeCached(
+ `WITH RECURSIVE
+ descendants(did) AS (
+ SELECT b.id FROM moz_bookmarks b
+ JOIN moz_bookmarks p ON b.parent = p.id
+ WHERE p.guid = :folderGuid
+ UNION ALL
+ SELECT id FROM moz_bookmarks
+ JOIN descendants ON parent = did
+ )
+ DELETE FROM moz_bookmarks WHERE id IN descendants`, { folderGuid });
+ }
+
+ // Cleanup orphans.
+ yield removeOrphanAnnotations(db);
+ yield removeOrphanKeywords(db);
+
+ // TODO (Bug 1087576): this may leave orphan tags behind.
+
+ let urls = [for (item of itemsRemoved) if (item.url) item.url];
+ updateFrecency(db, urls).then(null, Cu.reportError);
+
+ // Send onItemRemoved notifications to listeners.
+ // TODO (Bug 1087580): for the case of eraseEverything, this should send a
+ // single clear bookmarks notification rather than notifying for each
+ // bookmark.
+
+ // Notify listeners in reverse order to serve children before parents.
+ let observers = PlacesUtils.bookmarks.getObservers();
+ for (let item of itemsRemoved.reverse()) {
+ let uri = item.hasOwnProperty("url") ? toURI(item.url) : null;
+ notify(observers, "onItemRemoved", [ item._id, item._parentId,
+ item.index, item.type, uri,
+ item.guid, item.parentGuid ]);
+
+ let isUntagging = item._grandParentId == PlacesUtils.tagsFolderId;
+ if (isUntagging) {
+ for (let entry of (yield fetchBookmarksByURL(item))) {
+ notify(observers, "onItemChanged", [ entry._id, "tags", false, "",
+ toPRTime(entry.lastModified),
+ entry.type, entry._parentId,
+ entry.guid, entry.parentGuid ]);
+ }
+ }
+ }
+});
--- a/toolkit/components/places/tests/bookmarks/test_bookmarks_notifications.js
+++ b/toolkit/components/places/tests/bookmarks/test_bookmarks_notifications.js
@@ -310,16 +310,56 @@ add_task(function* remove_bookmark_tag_n
tag.url, tag.guid, tag.parentGuid ] },
{ name: "onItemChanged",
arguments: [ itemId, "tags", false, "",
bm.lastModified, bm.type, parentId,
bm.guid, bm.parentGuid ] }
]);
});
+add_task(function* remove_folder_notification() {
+ let folder1 = yield PlacesUtils.bookmarks.insert({ type: PlacesUtils.bookmarks.TYPE_FOLDER,
+ parentGuid: PlacesUtils.bookmarks.unfiledGuid });
+ let folder1Id = yield PlacesUtils.promiseItemId(folder1.guid);
+ let folder1ParentId = yield PlacesUtils.promiseItemId(folder1.parentGuid);
+
+ let bm = yield PlacesUtils.bookmarks.insert({ type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
+ parentGuid: folder1.guid,
+ url: new URL("http://example.com/") });
+ let bmItemId = yield PlacesUtils.promiseItemId(bm.guid);
+
+ let folder2 = yield PlacesUtils.bookmarks.insert({ type: PlacesUtils.bookmarks.TYPE_FOLDER,
+ parentGuid: folder1.guid });
+ let folder2Id = yield PlacesUtils.promiseItemId(folder2.guid);
+
+ let bm2 = yield PlacesUtils.bookmarks.insert({ type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
+ parentGuid: folder2.guid,
+ url: new URL("http://example.com/") });
+ let bm2ItemId = yield PlacesUtils.promiseItemId(bm2.guid);
+
+ let observer = expectNotifications();
+ yield PlacesUtils.bookmarks.remove(folder1.guid);
+
+ observer.check([ { name: "onItemRemoved",
+ arguments: [ bm2ItemId, folder2Id, bm2.index, bm2.type,
+ bm2.url, bm2.guid, bm2.parentGuid ] },
+ { name: "onItemRemoved",
+ arguments: [ folder2Id, folder1Id, folder2.index,
+ folder2.type, null, folder2.guid,
+ folder2.parentGuid ] },
+ { name: "onItemRemoved",
+ arguments: [ bmItemId, folder1Id, bm.index, bm.type,
+ bm.url, bm.guid, bm.parentGuid ] },
+ { name: "onItemRemoved",
+ arguments: [ folder1Id, folder1ParentId, folder1.index,
+ folder1.type, null, folder1.guid,
+ folder1.parentGuid ] }
+ ]);
+});
+
add_task(function* eraseEverything_notification() {
// Let's start from a clean situation.
yield PlacesUtils.bookmarks.eraseEverything();
let folder1 = yield PlacesUtils.bookmarks.insert({ type: PlacesUtils.bookmarks.TYPE_FOLDER,
parentGuid: PlacesUtils.bookmarks.unfiledGuid });
let folder1Id = yield PlacesUtils.promiseItemId(folder1.guid);
let folder1ParentId = yield PlacesUtils.promiseItemId(folder1.parentGuid);
@@ -343,39 +383,39 @@ add_task(function* eraseEverything_notif
let menuBm = yield PlacesUtils.bookmarks.insert({ type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
parentGuid: PlacesUtils.bookmarks.menuGuid,
url: new URL("http://example.com/") });
let menuBmId = yield PlacesUtils.promiseItemId(menuBm.guid);
let menuBmParentId = yield PlacesUtils.promiseItemId(menuBm.parentGuid);
let observer = expectNotifications();
- let removed = yield PlacesUtils.bookmarks.eraseEverything();
+ yield PlacesUtils.bookmarks.eraseEverything();
observer.check([ { name: "onItemRemoved",
- arguments: [ menuBmId, menuBmParentId,
- menuBm.index, menuBm.type,
- menuBm.url, menuBm.guid,
- menuBm.parentGuid ] },
- { name: "onItemRemoved",
- arguments: [ toolbarBmId, toolbarBmParentId,
- toolbarBm.index, toolbarBm.type,
- toolbarBm.url, toolbarBm.guid,
- toolbarBm.parentGuid ] },
- { name: "onItemRemoved",
arguments: [ folder2Id, folder2ParentId, folder2.index,
folder2.type, null, folder2.guid,
folder2.parentGuid ] },
{ name: "onItemRemoved",
arguments: [ itemId, parentId, bm.index, bm.type,
bm.url, bm.guid, bm.parentGuid ] },
{ name: "onItemRemoved",
arguments: [ folder1Id, folder1ParentId, folder1.index,
folder1.type, null, folder1.guid,
- folder1.parentGuid ] }
+ folder1.parentGuid ] },
+ { name: "onItemRemoved",
+ arguments: [ menuBmId, menuBmParentId,
+ menuBm.index, menuBm.type,
+ menuBm.url, menuBm.guid,
+ menuBm.parentGuid ] },
+ { name: "onItemRemoved",
+ arguments: [ toolbarBmId, toolbarBmParentId,
+ toolbarBm.index, toolbarBm.type,
+ toolbarBm.url, toolbarBm.guid,
+ toolbarBm.parentGuid ] }
]);
});
function expectNotifications() {
let notifications = [];
let observer = new Proxy(NavBookmarkObserver, {
get(target, name) {
if (name == "check") {
--- a/toolkit/components/places/tests/bookmarks/test_bookmarks_remove.js
+++ b/toolkit/components/places/tests/bookmarks/test_bookmarks_remove.js
@@ -142,16 +142,30 @@ add_task(function* remove_folder() {
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));
Assert.ok(!("keyword" in bm2));
});
+add_task(function* test_nested_contents_removed() {
+ let folder1 = yield PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
+ type: PlacesUtils.bookmarks.TYPE_FOLDER,
+ title: "a folder" });
+ let folder2 = yield PlacesUtils.bookmarks.insert({ parentGuid: folder1.guid,
+ type: PlacesUtils.bookmarks.TYPE_FOLDER,
+ title: "a folder" });
+ let sep = yield PlacesUtils.bookmarks.insert({ parentGuid: folder2.guid,
+ type: PlacesUtils.bookmarks.TYPE_SEPARATOR });
+ yield PlacesUtils.bookmarks.remove(folder1);
+ Assert.strictEqual((yield PlacesUtils.bookmarks.fetch(folder1.guid)), null);
+ Assert.strictEqual((yield PlacesUtils.bookmarks.fetch(folder2.guid)), null);
+ Assert.strictEqual((yield PlacesUtils.bookmarks.fetch(sep.guid)), null);
+});
add_task(function* remove_folder_empty_title() {
let bm1 = yield PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
type: PlacesUtils.bookmarks.TYPE_FOLDER,
title: "" });
checkBookmarkObject(bm1);
let bm2 = yield PlacesUtils.bookmarks.remove(bm1.guid);
checkBookmarkObject(bm2);