mirror of
https://github.com/mozilla/gecko-dev.git
synced 2024-10-28 12:45:27 +00:00
Bug 1110101 - Bookmarks.remove doesn't remove folder contents properly. r=mak.
This commit is contained in:
Asaf Romano
parent
e1e7bf30ee
commit
908c8f81e1
@ -456,83 +456,15 @@ let Bookmarks = Object.freeze({
|
||||
*/
|
||||
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));
|
||||
}),
|
||||
@ -1026,6 +958,10 @@ function* removeBookmark(item) {
|
||||
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
|
||||
@ -1414,3 +1350,83 @@ let setAncestorsLastModified = Task.async(function* (db, folderGuid, time) {
|
||||
`, { 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 ]);
|
||||
}
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
@ -315,6 +315,46 @@ add_task(function* remove_bookmark_tag_notification() {
|
||||
]);
|
||||
});
|
||||
|
||||
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();
|
||||
@ -348,19 +388,9 @@ add_task(function* eraseEverything_notification() {
|
||||
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 ] },
|
||||
@ -370,7 +400,17 @@ add_task(function* eraseEverything_notification() {
|
||||
{ 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 ] }
|
||||
]);
|
||||
});
|
||||
|
||||
|
||||
@ -147,6 +147,20 @@ add_task(function* remove_folder() {
|
||||
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,
|
||||
|
||||
Loading…
Reference in New Issue
Block a user