Bug 1379798 - Ensure `insertTree` notifies observers with the correct parent ID. r=standard8 draft
authorKit Cambridge <kit@yakshaving.ninja>
Mon, 10 Jul 2017 14:38:21 -0700
changeset 607644 f2860ccd4c45167e53140a819998bc2019963d9d
parent 606124 91c943f7373722ad4e122d98a2ddd6c79708b732
child 637105 490f7b8d418603ebe20f8a45cfc7ca7126582bd5
push id68065
push userbmo:kit@mozilla.com
push dateWed, 12 Jul 2017 16:54:27 +0000
reviewersstandard8
bugs1379798
milestone56.0a1
Bug 1379798 - Ensure `insertTree` notifies observers with the correct parent ID. r=standard8 MozReview-Commit-ID: LBm8VddumPJ
toolkit/components/places/Bookmarks.jsm
toolkit/components/places/tests/bookmarks/test_bookmarks_insertTree.js
--- a/toolkit/components/places/Bookmarks.jsm
+++ b/toolkit/components/places/Bookmarks.jsm
@@ -420,57 +420,56 @@ var Bookmarks = Object.freeze({
     }
 
     // We want to validate synchronously, but we can't know the index at which
     // we're inserting into the parent. We just use NULL instead,
     // and the SQL query with which we insert will update it as necessary.
     let lastAddedForParent = appendInsertionInfoForInfoArray(tree.children, null, tree.guid);
 
     return (async function() {
-      let parent = await fetchBookmark({ guid: tree.guid });
-      if (!parent) {
+      let treeParent = await fetchBookmark({ guid: tree.guid });
+      if (!treeParent) {
         throw new Error("The parent you specified doesn't exist.");
       }
 
-      if (parent._parentId == PlacesUtils.tagsFolderId) {
+      if (treeParent._parentId == PlacesUtils.tagsFolderId) {
         throw new Error("Can't use insertTree to insert tags.");
       }
 
-      await insertBookmarkTree(insertInfos, source, parent,
+      await insertBookmarkTree(insertInfos, source, treeParent,
                                urlsThatMightNeedPlaces, lastAddedForParent);
 
       await insertLivemarkData(insertLivemarkInfos);
 
       // Now update the indices of root items in the objects we return.
       // These may be wrong if someone else modified the table between
       // when we fetched the parent and inserted our items, but the actual
       // inserts will have been correct, and we don't want to query the DB
       // again if we don't have to. bug 1347230 covers improving this.
-      let rootIndex = parent._childCount;
+      let rootIndex = treeParent._childCount;
       for (let insertInfo of insertInfos) {
         if (insertInfo.parentGuid == tree.guid) {
           insertInfo.index += rootIndex++;
         }
       }
       // We need the itemIds to notify, though once the switch to guids is
       // complete we may stop using them.
       let itemIdMap = await PlacesUtils.promiseManyItemIds(insertInfos.map(info => info.guid));
       // Notify onItemAdded to listeners.
       let observers = PlacesUtils.bookmarks.getObservers();
       for (let i = 0; i < insertInfos.length; i++) {
         let item = insertInfos[i];
         let itemId = itemIdMap.get(item.guid);
         let uri = item.hasOwnProperty("url") ? PlacesUtils.toURI(item.url) : null;
         // For sub-folders, we need to make sure their children have the correct parent ids.
         let parentId;
-        if (item.guid === parent.guid ||
-            Bookmarks.userContentRoots.includes(item.parentGuid)) {
-          // We're the item being inserted at the top-level, or we're a top-level
-          // folder, so the parent id won't have changed.
-          parentId = parent._id;
+        if (item.parentGuid === treeParent.guid) {
+          // This is a direct child of the tree parent, so we can use the
+          // existing parent's id.
+          parentId = treeParent._id;
         } else {
           // This is a parent folder that's been updated, so we need to
           // use the new item id.
           parentId = itemIdMap.get(item.parentGuid);
         }
 
         notify(observers, "onItemAdded", [ itemId, parentId, item.index,
                                            item.type, uri, item.title,
--- a/toolkit/components/places/tests/bookmarks/test_bookmarks_insertTree.js
+++ b/toolkit/components/places/tests/bookmarks/test_bookmarks_insertTree.js
@@ -322,8 +322,77 @@ add_task(async function insert_many_non_
     if (bm.type == PlacesUtils.bookmarks.TYPE_BOOKMARK) {
       Assert.greater(frecencyForUrl(bm.url), 0, "Check frecency has been updated for bookmark " + bm.url);
     }
     Assert.equal(bm.parentGuid, PlacesUtils.bookmarks.unfiledGuid);
   }
   Assert.equal(obsInvoked, bms.length);
   Assert.equal(obsInvoked, 6);
 });
+
+add_task(async function create_in_folder() {
+  let mozFolder = await PlacesUtils.bookmarks.insert({
+    parentGuid: PlacesUtils.bookmarks.menuGuid,
+    type: PlacesUtils.bookmarks.TYPE_FOLDER,
+    title: "Mozilla",
+  });
+
+  let notifications = [];
+  let obs = {
+    onItemAdded(itemId, parentId, index, type, uri, title, dateAdded, guid, parentGuid) {
+      notifications.push({ itemId, parentId, index, title, guid, parentGuid });
+    },
+  };
+  PlacesUtils.bookmarks.addObserver(obs);
+
+  let bms = await PlacesUtils.bookmarks.insertTree({children: [{
+    url: "http://getfirefox.com",
+    title: "Get Firefox!",
+  }, {
+    type: PlacesUtils.bookmarks.TYPE_FOLDER,
+    title: "Community",
+    children: [
+      {
+        url: "http://getthunderbird.com",
+        title: "Get Thunderbird!",
+      },
+      {
+        url: "https://www.seamonkey-project.org",
+        title: "SeaMonkey",
+      },
+    ],
+  }], guid: mozFolder.guid});
+  await PlacesTestUtils.promiseAsyncUpdates();
+
+  PlacesUtils.bookmarks.removeObserver(obs);
+
+  let mozFolderId = await PlacesUtils.promiseItemId(mozFolder.guid);
+  let commFolderId = await PlacesUtils.promiseItemId(bms[1].guid);
+  deepEqual(notifications, [{
+    itemId: await PlacesUtils.promiseItemId(bms[0].guid),
+    parentId: mozFolderId,
+    index: 0,
+    title: "Get Firefox!",
+    guid: bms[0].guid,
+    parentGuid: mozFolder.guid,
+  }, {
+    itemId: commFolderId,
+    parentId: mozFolderId,
+    index: 1,
+    title: "Community",
+    guid: bms[1].guid,
+    parentGuid: mozFolder.guid,
+  }, {
+    itemId: await PlacesUtils.promiseItemId(bms[2].guid),
+    parentId: commFolderId,
+    index: 0,
+    title: "Get Thunderbird!",
+    guid: bms[2].guid,
+    parentGuid: bms[1].guid,
+  }, {
+    itemId: await PlacesUtils.promiseItemId(bms[3].guid),
+    parentId: commFolderId,
+    index: 1,
+    title: "SeaMonkey",
+    guid: bms[3].guid,
+    parentGuid: bms[1].guid,
+  }]);
+});