Bug 1104796 - Bookmarks.remove disallows removing any direct child of the places root. r=mak
--- a/toolkit/components/places/Bookmarks.jsm
+++ b/toolkit/components/places/Bookmarks.jsm
@@ -399,33 +399,34 @@ let Bookmarks = Object.freeze({
* @resolves to an object representing the removed bookmark.
* @rejects if the provided guid doesn't match any existing bookmark.
* @throws if the arguments are invalid.
*/
remove(guidOrInfo) {
let info = guidOrInfo;
if (!info)
throw new Error("Input should be a valid object");
- if (typeof(guidOrInfo) != "object") {
+ if (typeof(guidOrInfo) != "object")
info = { guid: guidOrInfo };
+
+ // Disallow removing the root folders.
+ if ([this.rootGuid, this.menuGuid, this.toolbarGuid, this.unfiledGuid,
+ this.tagsGuid].indexOf(info.guid) != -1) {
+ 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(info);
return Task.spawn(function* () {
let item = yield fetchBookmark(removeInfo);
if (!item)
throw new Error("No bookmarks found for the provided GUID.");
- // Disallow removing the root folders.
- if (!item._parentId || item._parentId == PlacesUtils.placesRootId)
- throw new Error("It's not possible to remove Places root folders.");
-
item = yield removeBookmark(item);
// Notify onItemRemoved to listeners.
let observers = PlacesUtils.bookmarks.getObservers();
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 ]);
--- a/toolkit/components/places/tests/bookmarks/test_bookmarks_remove.js
+++ b/toolkit/components/places/tests/bookmarks/test_bookmarks_remove.js
@@ -39,29 +39,34 @@ add_task(function* remove_nonexistent_gu
yield PlacesUtils.bookmarks.remove({ guid: "123456789012"});
Assert.ok(false, "Should have thrown");
} catch (ex) {
Assert.ok(/No bookmarks found for the provided GUID/.test(ex));
}
});
add_task(function* remove_roots_fail() {
- try {
- yield PlacesUtils.bookmarks.remove(PlacesUtils.bookmarks.unfiledGuid);
- Assert.ok(false, "Should have thrown");
- } catch (ex) {
- Assert.ok(/It's not possible to remove Places root folders/.test(ex));
+ let guids = [PlacesUtils.bookmarks.rootGuid,
+ PlacesUtils.bookmarks.unfiledGuid,
+ PlacesUtils.bookmarks.menuGuid,
+ PlacesUtils.bookmarks.toolbarGuid,
+ PlacesUtils.bookmarks.tagsGuid];
+ for (let guid of guids) {
+ Assert.throws(() => PlacesUtils.bookmarks.remove(guid),
+ /It's not possible to remove Places root folders/);
}
+});
- try {
- yield PlacesUtils.bookmarks.remove(PlacesUtils.bookmarks.rootGuid);
- Assert.ok(false, "Should have thrown");
- } catch (ex) {
- Assert.ok(/It's not possible to remove Places root folders/.test(ex));
- }
+add_task(function* remove_normal_folder_undes_root_succeeds() {
+ let folder = yield PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.rootGuid,
+ type: PlacesUtils.bookmarks.TYPE_FOLDER });
+ checkBookmarkObject(folder);
+ let removed_folder = yield PlacesUtils.bookmarks.remove(folder);
+ Assert.deepEqual(folder, removed_folder);
+ Assert.strictEqual((yield PlacesUtils.bookmarks.fetch(folder.guid)), null);
});
add_task(function* remove_bookmark() {
let bm1 = yield PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
url: "http://example.com/",
title: "a bookmark" });
checkBookmarkObject(bm1);