Bug 1428040 - Allow PlacesUtils.isRootItem to take guids as well as ids. r=mak

MozReview-Commit-ID: 9ZMA2A879O8

--HG--
rename : toolkit/components/places/tests/bookmarks/test_protectRoots.js => toolkit/components/places/tests/legacy/test_protectRoots.js
extra : rebase_source : 99f4d3702ed60412baf8b8cbd42fbf4c660d0b5b

browser/components/places/content/treeView.js

@@ -1738,11 +1738,11 @@ PlacesTreeView.prototype = {
       Cu.reportError("isEditable called for an unbuilt row.");
       return false;
     }
-    let itemId = node.itemId;
+    let itemGuid = node.bookmarkGuid;
 
     // Only bookmark-nodes are editable.  Fortunately, this checks also takes
     // care of livemark children.
-    if (itemId == -1)
+    if (itemGuid == "")
       return false;
 
     // The following items are also not editable, even though they are bookmark
@@ -1755,7 +1755,7 @@ PlacesTreeView.prototype = {
     // Note that concrete itemIds aren't used intentionally.  For example, we
     // have no reason to disallow renaming a shortcut to the Bookmarks Toolbar,
     // except for the one under All Bookmarks.
-    if (PlacesUtils.nodeIsSeparator(node) || PlacesUtils.isRootItem(itemId))
+    if (PlacesUtils.nodeIsSeparator(node) || PlacesUtils.isRootItem(itemGuid))
       return false;
 
     let parentId = PlacesUtils.getConcreteItemId(node.parent);

toolkit/components/places/Bookmarks.jsm

@@ -603,6 +603,9 @@ var Bookmarks = Object.freeze({
         async db => {
         let parent;
         if (updateInfo.hasOwnProperty("parentGuid")) {
+          if (PlacesUtils.isRootItem(item.guid)) {
+            throw new Error("It's not possible to move Places root folders.");
+          }
           if (item.type == this.TYPE_FOLDER) {
             // Make sure we are not moving a folder into itself or one of its
             // descendants.
@@ -628,6 +631,9 @@ var Bookmarks = Object.freeze({
         }
 
         if (updateInfo.hasOwnProperty("index")) {
+          if (PlacesUtils.isRootItem(item.guid)) {
+            throw new Error("It's not possible to move Places root folders.");
+          }
           // If at this point we don't have a parent yet, we are moving into
           // the same container.  Thus we know it exists.
           if (!parent)

toolkit/components/places/PlacesUtils.jsm

@@ -1232,19 +1232,26 @@ this.PlacesUtils = {
   },
 
   /**
-   * Checks if aItemId is a root.
+   * Checks if item is a root.
    *
-   *   @param aItemId
-   *          item id to look for.
-   *   @returns true if aItemId is a root, false otherwise.
+   * @param {Number|String} guid The guid or id of the item to look for.
+   * @returns {Boolean} true if guid is a root, false otherwise.
    */
-  isRootItem: function PU_isRootItem(aItemId) {
-    return aItemId == PlacesUtils.bookmarksMenuFolderId ||
-           aItemId == PlacesUtils.toolbarFolderId ||
-           aItemId == PlacesUtils.unfiledBookmarksFolderId ||
-           aItemId == PlacesUtils.tagsFolderId ||
-           aItemId == PlacesUtils.placesRootId ||
-           aItemId == PlacesUtils.mobileFolderId;
+  isRootItem(guid) {
+    if (typeof guid === "string") {
+      return guid == PlacesUtils.bookmarks.menuGuid ||
+             guid == PlacesUtils.bookmarks.toolbarGuid ||
+             guid == PlacesUtils.bookmarks.unfiledGuid ||
+             guid == PlacesUtils.bookmarks.tagsGuid ||
+             guid == PlacesUtils.bookmarks.rootGuid ||
+             guid == PlacesUtils.bookmarks.mobileGuid;
+    }
+    return guid == PlacesUtils.bookmarksMenuFolderId ||
+           guid == PlacesUtils.toolbarFolderId ||
+           guid == PlacesUtils.unfiledBookmarksFolderId ||
+           guid == PlacesUtils.tagsFolderId ||
+           guid == PlacesUtils.placesRootId ||
+           guid == PlacesUtils.mobileFolderId;
   },
 
   /**

toolkit/components/places/tests/bookmarks/test_bookmarks_update.js

@@ -78,6 +78,22 @@ add_task(async function invalid_input_throws() {
                 /The following properties were expected: index/);
 });
 
+add_task(async function move_roots_fail() {
+  let guids = [PlacesUtils.bookmarks.unfiledGuid,
+               PlacesUtils.bookmarks.menuGuid,
+               PlacesUtils.bookmarks.toolbarGuid,
+               PlacesUtils.bookmarks.tagsGuid,
+               PlacesUtils.bookmarks.mobileGuid];
+  for (let guid of guids) {
+    Assert.rejects(PlacesUtils.bookmarks.update({
+      guid,
+      index: -1,
+      parentGuid: PlacesUtils.bookmarks.rootGuid,
+    }), /It's not possible to move Places root folders\./,
+    `Should reject when attempting to move ${guid}`);
+  }
+});
+
 add_task(async function nonexisting_bookmark_throws() {
   try {
     await PlacesUtils.bookmarks.update({ guid: "123456789012",

toolkit/components/places/tests/bookmarks/xpcshell.ini

@@ -40,7 +40,6 @@ skip-if = toolkit == 'android'
 [test_insertTree_fixupOrSkipInvalidEntries.js]
 [test_keywords.js]
 [test_nsINavBookmarkObserver.js]
-[test_protectRoots.js]
 [test_removeFolderTransaction_reinsert.js]
 [test_savedsearches.js]
 [test_sync_fields.js]

toolkit/components/places/tests/legacy/xpcshell.ini

@@ -9,4 +9,5 @@ firefox-appdir = browser
 [test_bookmarks_setNullTitle.js]
 [test_changeBookmarkURI.js]
 [test_placesTxn.js]
+[test_protectRoots.js]
 [test_removeItem.js]

toolkit/components/places/tests/unit/test_PlacesUtils_isRootItem.js

@@ -0,0 +1,15 @@
+"use strict";
+
+const GUIDS = [
+  PlacesUtils.bookmarks.rootGuid,
+  ...PlacesUtils.bookmarks.userContentRoots,
+  PlacesUtils.bookmarks.tagsGuid,
+];
+
+add_task(async function test_isRootItem() {
+  for (let guid of GUIDS) {
+    Assert.ok(PlacesUtils.isRootItem(guid), `Should correctly identify root item ${guid}`);
+  }
+
+  Assert.ok(!PlacesUtils.isRootItem("fakeguid1234"), "Should not identify other items as root.");
+});

toolkit/components/places/tests/unit/xpcshell.ini

@@ -112,6 +112,7 @@ skip-if = true
 [test_frecency_observers.js]
 [test_placeURIs.js]
 [test_PlacesUtils_invalidateCachedGuidFor.js]
+[test_PlacesUtils_isRootItem.js]
 [test_preventive_maintenance.js]
 [test_preventive_maintenance_checkAndFixDatabase.js]
 [test_preventive_maintenance_runTasks.js]