Bug 1364546 - Set the children property for BookmarkTreeNodes without children, r?mixedpuppy draft
authorBob Silverberg <bsilverberg@mozilla.com>
Tue, 23 May 2017 15:16:10 -0400
changeset 583182 fa44dd672af5670f23251468362575a872ea34ae
parent 582730 5bc1c758ab57c1885dceab4e7837e58af27b998c
child 629990 2195419b5dd51a3b1b6eee9aa49657ef280d8e17
push id60321
push userbmo:bob.silverberg@gmail.com
push dateTue, 23 May 2017 19:18:50 +0000
reviewersmixedpuppy
bugs1364546
milestone55.0a1
Bug 1364546 - Set the children property for BookmarkTreeNodes without children, r?mixedpuppy BookmarkTreeNodes without children do not contain a children property, but they should actually contain a children property which is an empty array. This is what Chrome does as well. This patch fixes that, and includes a test for a case of getChildren that was previously untested, which I noticed when working on this code. MozReview-Commit-ID: CjUwExma760
browser/components/extensions/ext-bookmarks.js
browser/components/extensions/test/xpcshell/test_ext_bookmarks.js
--- a/browser/components/extensions/ext-bookmarks.js
+++ b/browser/components/extensions/ext-bookmarks.js
@@ -21,18 +21,20 @@ function getTree(rootGuid, onlyChildren)
     }
 
     if (node.type == PlacesUtils.TYPE_X_MOZ_PLACE) {
       // This isn't quite correct. Recently Bookmarked ends up here ...
       treenode.url = node.uri;
     } else {
       treenode.dateGroupModified = node.lastModified / 1000;
 
-      if (node.children && !onlyChildren) {
-        treenode.children = node.children.map(child => convert(child, node));
+      if (!onlyChildren) {
+        treenode.children = node.children
+          ? node.children.map(child => convert(child, node))
+          : [];
       }
     }
 
     return treenode;
   }
 
   return PlacesUtils.promiseBookmarksTree(rootGuid, {
     excludeItemsCallback: item => {
--- a/browser/components/extensions/test/xpcshell/test_ext_bookmarks.js
+++ b/browser/components/extensions/test/xpcshell/test_ext_bookmarks.js
@@ -652,8 +652,50 @@ add_task(async function test_get_recent_
     let expected = createdBookmarks[i];
     equal(actual.url, expected.url, "Bookmark has the expected url.");
     equal(actual.title, expected.title, "Bookmark has the expected title.");
     equal(actual.parentId, expected.parentGuid, "Bookmark has the expected parentId.");
   }
 
   await extension.unload();
 });
+
+add_task(async function test_tree_with_empty_folder() {
+  async function background() {
+    await browser.bookmarks.create({title: "Empty Folder"});
+    let nonEmptyFolder = await browser.bookmarks.create({title: "Non-Empty Folder"});
+    await browser.bookmarks.create({title: "A bookmark", url: "http://example.com", parentId: nonEmptyFolder.id});
+
+    let tree = await browser.bookmarks.getSubTree(nonEmptyFolder.parentId);
+    browser.test.assertEq(0,
+      tree[0].children[0].children.length,
+      "The empty folder returns an empty array for children.");
+    browser.test.assertEq(1,
+      tree[0].children[1].children.length,
+      "The non-empty folder returns a single item array for children.");
+
+    let children = await browser.bookmarks.getChildren(nonEmptyFolder.parentId);
+    // getChildren should only return immediate children. This is not tested in the
+    // monster test above.
+    for (let child of children) {
+      browser.test.assertEq(undefined,
+        child.children,
+        "Child from getChildren does not contain any children.");
+    }
+
+    browser.test.sendMessage("done");
+  }
+
+  let extension = ExtensionTestUtils.loadExtension({
+    background,
+    manifest: {
+      permissions: ["bookmarks"],
+    },
+  });
+
+  // Start with an empty bookmarks database.
+  await PlacesUtils.bookmarks.eraseEverything();
+
+  await extension.startup();
+  await extension.awaitMessage("done");
+
+  await extension.unload();
+});