Bug 1293853 - Part 3: Add support for separators to bookmarks API, r?mixedpuppy draft
authorBob Silverberg <bsilverberg@mozilla.com>
Mon, 28 Aug 2017 17:05:55 -0400
changeset 657528 e4df313ec3911353a6f0e585ece99e9abcc38578
parent 657527 ea907a4e38848ec7bb15c2ab39046c660eb07ad9
child 729454 7093190dc6a0be4c70e2fea5a3385b79925ecaff
push id77550
push userbmo:bob.silverberg@gmail.com
push dateFri, 01 Sep 2017 16:04:30 +0000
reviewersmixedpuppy
bugs1293853
milestone57.0a1
Bug 1293853 - Part 3: Add support for separators to bookmarks API, r?mixedpuppy This adds support for separators to the bookmarks API. Separators can now be created and will be returned by any method that returns BookmarkTreeNodes. They will also be included in data for the onCreated and onRemoved events. BookmarkTreeNodes will now contain a `type` property which will be one of bookmark, folder or separator. When creating a bookmark object, one can specify the type, or one can rely on the Chrome-compatible behaviour which treats any bookmarks without a URL as a folder. To create a separator one must specify a type as part of the CreateDetails object. MozReview-Commit-ID: BoyGgx8lMAZ
browser/components/extensions/ext-bookmarks.js
browser/components/extensions/schemas/bookmarks.json
browser/components/extensions/test/xpcshell/test_ext_bookmarks.js
--- a/browser/components/extensions/ext-bookmarks.js
+++ b/browser/components/extensions/ext-bookmarks.js
@@ -3,52 +3,82 @@
 "use strict";
 
 // The ext-* files are imported into the same scopes.
 /* import-globals-from ext-browserAction.js */
 
 XPCOMUtils.defineLazyModuleGetter(this, "PlacesUtils",
                                   "resource://gre/modules/PlacesUtils.jsm");
 
+const {
+  TYPE_BOOKMARK,
+  TYPE_FOLDER,
+  TYPE_SEPARATOR,
+} = PlacesUtils.bookmarks;
+
+const BOOKMARKS_TYPES_TO_API_TYPES_MAP = new Map([
+  [TYPE_BOOKMARK, "bookmark"],
+  [TYPE_FOLDER, "folder"],
+  [TYPE_SEPARATOR, "separator"],
+]);
+
+const BOOKMARK_SEPERATOR_URL = "data:";
+
+XPCOMUtils.defineLazyGetter(this, "API_TYPES_TO_BOOKMARKS_TYPES_MAP", () => {
+  let theMap = new Map();
+
+  for (let [code, name] of BOOKMARKS_TYPES_TO_API_TYPES_MAP) {
+    theMap.set(name, code);
+  }
+  return theMap;
+});
+
 let listenerCount = 0;
 
+function getUrl(type, url) {
+  switch (type) {
+    case TYPE_BOOKMARK:
+      return url;
+    case TYPE_SEPARATOR:
+      return BOOKMARK_SEPERATOR_URL;
+    default:
+      return undefined;
+  }
+}
+
 const getTree = (rootGuid, onlyChildren) => {
   function convert(node, parent) {
     let treenode = {
       id: node.guid,
       title: node.title || "",
       index: node.index,
       dateAdded: node.dateAdded / 1000,
+      type: BOOKMARKS_TYPES_TO_API_TYPES_MAP.get(node.typeCode),
+      url: getUrl(node.typeCode, node.uri),
     };
 
     if (parent && node.guid != PlacesUtils.bookmarks.rootGuid) {
       treenode.parentId = parent.guid;
     }
 
-    if (node.type == PlacesUtils.TYPE_X_MOZ_PLACE) {
-      // This isn't quite correct. Recently Bookmarked ends up here ...
-      treenode.url = node.uri;
-    } else {
+    if (node.typeCode == TYPE_FOLDER) {
       treenode.dateGroupModified = node.lastModified / 1000;
 
       if (!onlyChildren) {
         treenode.children = node.children
           ? node.children.map(child => convert(child, node))
           : [];
       }
     }
 
     return treenode;
   }
 
   return PlacesUtils.promiseBookmarksTree(rootGuid, {
     excludeItemsCallback: item => {
-      if (item.type == PlacesUtils.TYPE_X_MOZ_PLACE_SEPARATOR) {
-        return true;
-      }
       return item.annos &&
              item.annos.find(a => a.name == PlacesUtils.EXCLUDE_FROM_BACKUP_ANNO);
     },
   }).then(root => {
     if (onlyChildren) {
       let children = root.children || [];
       return children.map(child => convert(child, root));
     }
@@ -60,25 +90,25 @@ const getTree = (rootGuid, onlyChildren)
 };
 
 const convertBookmarks = result => {
   let node = {
     id: result.guid,
     title: result.title || "",
     index: result.index,
     dateAdded: result.dateAdded.getTime(),
+    type: BOOKMARKS_TYPES_TO_API_TYPES_MAP.get(result.type),
+    url: getUrl(result.type, result.url && result.url.href),
   };
 
   if (result.guid != PlacesUtils.bookmarks.rootGuid) {
     node.parentId = result.parentGuid;
   }
 
-  if (result.type == PlacesUtils.bookmarks.TYPE_BOOKMARK) {
-    node.url = result.url.href; // Output is always URL object.
-  } else {
+  if (result.type == TYPE_FOLDER) {
     node.dateGroupModified = result.lastModified.getTime();
   }
 
   return node;
 };
 
 let observer = new class extends EventEmitter {
   constructor() {
@@ -87,76 +117,58 @@ let observer = new class extends EventEm
     this.skipTags = true;
     this.skipDescendantsOnItemRemoval = true;
   }
 
   onBeginUpdateBatch() {}
   onEndUpdateBatch() {}
 
   onItemAdded(id, parentId, index, itemType, uri, title, dateAdded, guid, parentGuid, source) {
-    if (itemType == PlacesUtils.bookmarks.TYPE_SEPARATOR) {
-      return;
-    }
-
     let bookmark = {
       id: guid,
       parentId: parentGuid,
       index,
       title,
       dateAdded: dateAdded / 1000,
+      type: BOOKMARKS_TYPES_TO_API_TYPES_MAP.get(itemType),
+      url: getUrl(itemType, uri && uri.spec),
     };
 
-    if (itemType == PlacesUtils.bookmarks.TYPE_BOOKMARK) {
-      bookmark.url = uri.spec;
-    } else {
+    if (itemType == TYPE_FOLDER) {
       bookmark.dateGroupModified = bookmark.dateAdded;
     }
 
     this.emit("created", bookmark);
   }
 
   onItemVisited() {}
 
   onItemMoved(id, oldParentId, oldIndex, newParentId, newIndex, itemType, guid, oldParentGuid, newParentGuid, source) {
-    if (itemType == PlacesUtils.bookmarks.TYPE_SEPARATOR) {
-      return;
-    }
-
     let info = {
       parentId: newParentGuid,
       index: newIndex,
       oldParentId: oldParentGuid,
       oldIndex,
     };
     this.emit("moved", {guid, info});
   }
 
   onItemRemoved(id, parentId, index, itemType, uri, guid, parentGuid, source) {
-    if (itemType == PlacesUtils.bookmarks.TYPE_SEPARATOR) {
-      return;
-    }
-
     let node = {
       id: guid,
       parentId: parentGuid,
       index,
+      type: BOOKMARKS_TYPES_TO_API_TYPES_MAP.get(itemType),
+      url: getUrl(itemType, uri && uri.spec),
     };
 
-    if (itemType == PlacesUtils.bookmarks.TYPE_BOOKMARK) {
-      node.url = uri.spec;
-    }
-
     this.emit("removed", {guid, info: {parentId: parentGuid, index, node}});
   }
 
   onItemChanged(id, prop, isAnno, val, lastMod, itemType, parentId, guid, parentGuid, oldVal, source) {
-    if (itemType == PlacesUtils.bookmarks.TYPE_SEPARATOR) {
-      return;
-    }
-
     let info = {};
     if (prop == "title") {
       info.title = val;
     } else if (prop == "uri") {
       info.url = val;
     } else {
       // Not defined yet.
       return;
@@ -223,22 +235,28 @@ this.bookmarks = class extends Extension
           return PlacesUtils.bookmarks.getRecent(numberOfItems).then(result => result.map(convertBookmarks));
         },
 
         create: function(bookmark) {
           let info = {
             title: bookmark.title || "",
           };
 
-          // If url is NULL or missing, it will be a folder.
-          if (bookmark.url !== null) {
-            info.type = PlacesUtils.bookmarks.TYPE_BOOKMARK;
+          info.type = API_TYPES_TO_BOOKMARKS_TYPES_MAP.get(bookmark.type);
+          if (!info.type) {
+            // If url is NULL or missing, it will be a folder.
+            if (bookmark.url !== null) {
+              info.type = TYPE_BOOKMARK;
+            } else {
+              info.type = TYPE_FOLDER;
+            }
+          }
+
+          if (info.type === TYPE_BOOKMARK) {
             info.url = bookmark.url || "";
-          } else {
-            info.type = PlacesUtils.bookmarks.TYPE_FOLDER;
           }
 
           if (bookmark.index !== null) {
             info.index = bookmark.index;
           }
 
           if (bookmark.parentId !== null) {
             info.parentGuid = bookmark.parentId;
--- a/browser/components/extensions/schemas/bookmarks.json
+++ b/browser/components/extensions/schemas/bookmarks.json
@@ -24,16 +24,22 @@
     "types": [
       {
         "id": "BookmarkTreeNodeUnmodifiable",
         "type": "string",
         "enum": ["managed"],
         "description": "Indicates the reason why this node is unmodifiable. The <var>managed</var> value indicates that this node was configured by the system administrator or by the custodian of a supervised user. Omitted if the node can be modified by the user and the extension (default)."
       },
       {
+        "id": "BookmarkTreeNodeType",
+        "type": "string",
+        "enum": ["bookmark", "folder", "separator"],
+        "description": "Indicates the type of a BookmarkTreeNode, which can be one of bookmark, folder or separator."
+      },
+      {
         "id": "BookmarkTreeNode",
         "type": "object",
         "description": "A node (either a bookmark or a folder) in the bookmark tree.  Child nodes are ordered within their parent folder.",
         "properties": {
           "id": {
             "type": "string",
             "description": "The unique identifier for the node. IDs are unique within the current profile, and they remain valid even after the browser is restarted."
           },
@@ -66,16 +72,21 @@
             "optional": true,
             "description": "When the contents of this folder last changed, in milliseconds since the epoch."
           },
           "unmodifiable": {
             "$ref": "BookmarkTreeNodeUnmodifiable",
             "optional": true,
             "description": "Indicates the reason why this node is unmodifiable. The <var>managed</var> value indicates that this node was configured by the system administrator or by the custodian of a supervised user. Omitted if the node can be modified by the user and the extension (default)."
           },
+          "type": {
+            "$ref": "BookmarkTreeNodeType",
+            "optional": true,
+            "description": "Indicates the type of the BookmarkTreeNode, which can be one of bookmark, folder or separator."
+          },
           "children": {
             "type": "array",
             "optional": true,
             "items": { "$ref": "BookmarkTreeNode" },
             "description": "An ordered list of children of this node."
           }
         }
       },
@@ -96,16 +107,21 @@
           },
           "title": {
             "type": "string",
             "optional": true
           },
           "url": {
             "type": "string",
             "optional": true
+          },
+          "type": {
+            "$ref": "BookmarkTreeNodeType",
+            "optional": true,
+            "description": "Indicates the type of BookmarkTreeNode to create, which can be one of bookmark, folder or separator."
           }
         }
       }
     ],
     "functions": [
       {
         "name": "get",
         "type": "function",
--- a/browser/components/extensions/test/xpcshell/test_ext_bookmarks.js
+++ b/browser/components/extensions/test/xpcshell/test_ext_bookmarks.js
@@ -6,16 +6,17 @@ XPCOMUtils.defineLazyModuleGetter(this, 
                                   "resource://gre/modules/PlacesUtils.jsm");
 
 add_task(async function test_bookmarks() {
   function background() {
     let unsortedId, ourId;
     let initialBookmarkCount = 0;
     let createdBookmarks = new Set();
     let createdFolderId;
+    let createdSeparatorId;
     let collectedEvents = [];
     const nonExistentId = "000000000000";
     const bookmarkGuids = {
       menuGuid:    "menu________",
       toolbarGuid: "toolbar_____",
       unfiledGuid: "unfiled_____",
     };
 
@@ -23,42 +24,45 @@ add_task(async function test_bookmarks()
       browser.test.assertEq(ourId, bookmark.id, "Bookmark has the expected Id");
       browser.test.assertTrue("parentId" in bookmark, "Bookmark has a parentId");
       browser.test.assertEq(0, bookmark.index, "Bookmark has the expected index"); // We assume there are no other bookmarks.
       browser.test.assertEq("http://example.org/", bookmark.url, "Bookmark has the expected url");
       browser.test.assertEq("test bookmark", bookmark.title, "Bookmark has the expected title");
       browser.test.assertTrue("dateAdded" in bookmark, "Bookmark has a dateAdded");
       browser.test.assertFalse("dateGroupModified" in bookmark, "Bookmark does not have a dateGroupModified");
       browser.test.assertFalse("unmodifiable" in bookmark, "Bookmark is not unmodifiable");
+      browser.test.assertEq("bookmark", bookmark.type, "Bookmark is of type bookmark");
     }
 
     function checkBookmark(expected, bookmark) {
       browser.test.assertEq(expected.url, bookmark.url, "Bookmark has the expected url");
       browser.test.assertEq(expected.title, bookmark.title, "Bookmark has the expected title");
       browser.test.assertEq(expected.index, bookmark.index, "Bookmark has expected index");
+      browser.test.assertEq("bookmark", bookmark.type, "Bookmark is of type bookmark");
       if ("parentId" in expected) {
         browser.test.assertEq(expected.parentId, bookmark.parentId, "Bookmark has the expected parentId");
       }
     }
 
     function expectedError() {
       browser.test.fail("Did not get expected error");
     }
 
-    function checkOnCreated(id, parentId, index, title, url, dateAdded) {
+    function checkOnCreated(id, parentId, index, title, url, dateAdded, type = "bookmark") {
       let createdData = collectedEvents.pop();
       browser.test.assertEq("onCreated", createdData.event, "onCreated was the last event received");
       browser.test.assertEq(id, createdData.id, "onCreated event received the expected id");
       let bookmark = createdData.bookmark;
       browser.test.assertEq(id, bookmark.id, "onCreated event received the expected bookmark id");
       browser.test.assertEq(parentId, bookmark.parentId, "onCreated event received the expected bookmark parentId");
       browser.test.assertEq(index, bookmark.index, "onCreated event received the expected bookmark index");
       browser.test.assertEq(title, bookmark.title, "onCreated event received the expected bookmark title");
       browser.test.assertEq(url, bookmark.url, "onCreated event received the expected bookmark url");
       browser.test.assertEq(dateAdded, bookmark.dateAdded, "onCreated event received the expected bookmark dateAdded");
+      browser.test.assertEq(type, bookmark.type, "onCreated event received the expected bookmark type");
     }
 
     function checkOnChanged(id, url, title) {
       // If both url and title are changed, then url is fired last.
       let changedData = collectedEvents.pop();
       browser.test.assertEq("onChanged", changedData.event, "onChanged was the last event received");
       browser.test.assertEq(id, changedData.id, "onChanged event received the expected id");
       browser.test.assertEq(url, changedData.info.url, "onChanged event received the expected url");
@@ -75,28 +79,29 @@ add_task(async function test_bookmarks()
       browser.test.assertEq(id, movedData.id, "onMoved event received the expected id");
       let info = movedData.info;
       browser.test.assertEq(parentId, info.parentId, "onMoved event received the expected parentId");
       browser.test.assertEq(oldParentId, info.oldParentId, "onMoved event received the expected oldParentId");
       browser.test.assertEq(index, info.index, "onMoved event received the expected index");
       browser.test.assertEq(oldIndex, info.oldIndex, "onMoved event received the expected oldIndex");
     }
 
-    function checkOnRemoved(id, parentId, index, url) {
+    function checkOnRemoved(id, parentId, index, url, type = "folder") {
       let removedData = collectedEvents.pop();
       browser.test.assertEq("onRemoved", removedData.event, "onRemoved was the last event received");
       browser.test.assertEq(id, removedData.id, "onRemoved event received the expected id");
       let info = removedData.info;
       browser.test.assertEq(parentId, removedData.info.parentId, "onRemoved event received the expected parentId");
       browser.test.assertEq(index, removedData.info.index, "onRemoved event received the expected index");
       let node = info.node;
       browser.test.assertEq(id, node.id, "onRemoved event received the expected node id");
       browser.test.assertEq(parentId, node.parentId, "onRemoved event received the expected node parentId");
       browser.test.assertEq(index, node.index, "onRemoved event received the expected node index");
       browser.test.assertEq(url, node.url, "onRemoved event received the expected node url");
+      browser.test.assertEq(type, node.type, "onRemoved event received the expected node type");
     }
 
     browser.bookmarks.onChanged.addListener((id, info) => {
       collectedEvents.push({event: "onChanged", id, info});
     });
 
     browser.bookmarks.onCreated.addListener((id, bookmark) => {
       collectedEvents.push({event: "onCreated", id, bookmark});
@@ -121,17 +126,17 @@ add_task(async function test_bookmarks()
           nonExistentIdError.message.includes("Bookmark not found"),
           "Expected error thrown when trying to get a bookmark using a non-existent Id"
         );
       });
     }).then(() => {
       return browser.bookmarks.search({});
     }).then(results => {
       initialBookmarkCount = results.length;
-      return browser.bookmarks.create({title: "test bookmark", url: "http://example.org"});
+      return browser.bookmarks.create({title: "test bookmark", url: "http://example.org", type: "bookmark"});
     }).then(result => {
       ourId = result.id;
       checkOurBookmark(result);
       browser.test.assertEq(1, collectedEvents.length, "1 expected event received");
       checkOnCreated(ourId, bookmarkGuids.unfiledGuid, 0, "test bookmark", "http://example.org/", result.dateAdded);
 
       return browser.bookmarks.get(ourId);
     }).then(results => {
@@ -142,21 +147,22 @@ add_task(async function test_bookmarks()
       return browser.bookmarks.get(unsortedId);
     }).then(results => {
       let folder = results[0];
       browser.test.assertEq(1, results.length, "1 bookmark was returned");
 
       browser.test.assertEq(unsortedId, folder.id, "Folder has the expected id");
       browser.test.assertTrue("parentId" in folder, "Folder has a parentId");
       browser.test.assertTrue("index" in folder, "Folder has an index");
-      browser.test.assertFalse("url" in folder, "Folder does not have a url");
+      browser.test.assertEq(undefined, folder.url, "Folder does not have a url");
       browser.test.assertEq("Other Bookmarks", folder.title, "Folder has the expected title");
       browser.test.assertTrue("dateAdded" in folder, "Folder has a dateAdded");
       browser.test.assertTrue("dateGroupModified" in folder, "Folder has a dateGroupModified");
       browser.test.assertFalse("unmodifiable" in folder, "Folder is not unmodifiable"); // TODO: Do we want to enable this?
+      browser.test.assertEq("folder", folder.type, "Folder has a type of folder");
 
       return browser.bookmarks.getChildren(unsortedId);
     }).then(results => {
       browser.test.assertEq(1, results.length, "The folder has one child");
       checkOurBookmark(results[0]);
 
       return browser.bookmarks.update(nonExistentId, {title: "new test title"}).then(expectedError, error => {
         browser.test.assertTrue(
@@ -165,16 +171,17 @@ add_task(async function test_bookmarks()
         );
 
         return browser.bookmarks.update(ourId, {title: "new test title", url: "http://example.com/"});
       });
     }).then(result => {
       browser.test.assertEq("new test title", result.title, "Updated bookmark has the expected title");
       browser.test.assertEq("http://example.com/", result.url, "Updated bookmark has the expected URL");
       browser.test.assertEq(ourId, result.id, "Updated bookmark has the expected id");
+      browser.test.assertEq("bookmark", result.type, "Updated bookmark has a type of bookmark");
 
       browser.test.assertEq(2, collectedEvents.length, "2 expected events received");
       checkOnChanged(ourId, "http://example.com/", "new test title");
 
       return Promise.resolve().then(() => {
         return browser.bookmarks.update(ourId, {url: "this is not a valid url"});
       }).then(expectedError, error => {
         browser.test.assertTrue(
@@ -186,16 +193,18 @@ add_task(async function test_bookmarks()
     }).then(results => {
       browser.test.assertEq(1, results.length, "getTree returns one result");
       let bookmark = results[0].children.find(bookmarkItem => bookmarkItem.id == unsortedId);
       browser.test.assertEq(
           "Other Bookmarks",
           bookmark.title,
           "Folder returned from getTree has the expected title"
       );
+      browser.test.assertEq("folder", bookmark.type,
+                            "Folder returned from getTree has the expected type");
 
       return browser.bookmarks.create({parentId: "invalid"}).then(expectedError, error => {
         browser.test.assertTrue(
           error.message.includes("Invalid bookmark"),
           "Expected error thrown when trying to create a bookmark with an invalid parentId"
         );
         browser.test.assertTrue(
             error.message.includes(`"parentGuid":"invalid"`),
@@ -203,17 +212,17 @@ add_task(async function test_bookmarks()
         );
       });
     }).then(() => {
       return browser.bookmarks.remove(ourId);
     }).then(result => {
       browser.test.assertEq(undefined, result, "Removing a bookmark returns undefined");
 
       browser.test.assertEq(1, collectedEvents.length, "1 expected events received");
-      checkOnRemoved(ourId, bookmarkGuids.unfiledGuid, 0, "http://example.com/");
+      checkOnRemoved(ourId, bookmarkGuids.unfiledGuid, 0, "http://example.com/", "bookmark");
 
       return browser.bookmarks.get(ourId).then(expectedError, error => {
         browser.test.assertTrue(
           error.message.includes("Bookmark not found"),
           "Expected error thrown when trying to get a removed bookmark"
         );
       });
     }).then(() => {
@@ -223,83 +232,91 @@ add_task(async function test_bookmarks()
           "Expected error thrown when trying removed a non-existent bookmark"
         );
       });
     }).then(() => {
       // test bookmarks.search
       return Promise.all([
         browser.bookmarks.create({title: "MØzillä", url: "http://møzîllä.örg/"}),
         browser.bookmarks.create({title: "Example", url: "http://example.org/"}),
-        browser.bookmarks.create({title: "Mozilla Folder"}),
+        browser.bookmarks.create({title: "Mozilla Folder", type: "folder"}),
         browser.bookmarks.create({title: "EFF", url: "http://eff.org/"}),
         browser.bookmarks.create({title: "Menu Item", url: "http://menu.org/", parentId: bookmarkGuids.menuGuid}),
         browser.bookmarks.create({title: "Toolbar Item", url: "http://toolbar.org/", parentId: bookmarkGuids.toolbarGuid}),
       ]);
     }).then(results => {
       browser.test.assertEq(6, collectedEvents.length, "6 expected events received");
       checkOnCreated(results[5].id, bookmarkGuids.toolbarGuid, 0, "Toolbar Item", "http://toolbar.org/", results[5].dateAdded);
       checkOnCreated(results[4].id, bookmarkGuids.menuGuid, 0, "Menu Item", "http://menu.org/", results[4].dateAdded);
       checkOnCreated(results[3].id, bookmarkGuids.unfiledGuid, 0, "EFF", "http://eff.org/", results[3].dateAdded);
-      checkOnCreated(results[2].id, bookmarkGuids.unfiledGuid, 0, "Mozilla Folder", undefined, results[2].dateAdded);
+      checkOnCreated(results[2].id, bookmarkGuids.unfiledGuid, 0, "Mozilla Folder", undefined, results[2].dateAdded, "folder");
       checkOnCreated(results[1].id, bookmarkGuids.unfiledGuid, 0, "Example", "http://example.org/", results[1].dateAdded);
       checkOnCreated(results[0].id, bookmarkGuids.unfiledGuid, 0, "MØzillä", "http://xn--mzll-ooa1dud.xn--rg-eka/", results[0].dateAdded);
 
       for (let result of results) {
         if (result.title !== "Mozilla Folder") {
           createdBookmarks.add(result.id);
         }
       }
       let folderResult = results[2];
       createdFolderId = folderResult.id;
       return Promise.all([
         browser.bookmarks.create({title: "Mozilla", url: "http://allizom.org/", parentId: createdFolderId}),
+        browser.bookmarks.create({parentId: createdFolderId, type: "separator"}),
         browser.bookmarks.create({title: "Mozilla Corporation", url: "http://allizom.com/", parentId: createdFolderId}),
         browser.bookmarks.create({title: "Firefox", url: "http://allizom.org/firefox/", parentId: createdFolderId}),
       ]).then(newBookmarks => {
-        browser.test.assertEq(3, collectedEvents.length, "3 expected events received");
-        checkOnCreated(newBookmarks[2].id, createdFolderId, 0, "Firefox", "http://allizom.org/firefox/", newBookmarks[2].dateAdded);
-        checkOnCreated(newBookmarks[1].id, createdFolderId, 0, "Mozilla Corporation", "http://allizom.com/", newBookmarks[1].dateAdded);
+        browser.test.assertEq(4, collectedEvents.length, "4 expected events received");
+        checkOnCreated(newBookmarks[3].id, createdFolderId, 0, "Firefox", "http://allizom.org/firefox/", newBookmarks[3].dateAdded);
+        checkOnCreated(newBookmarks[2].id, createdFolderId, 0, "Mozilla Corporation", "http://allizom.com/", newBookmarks[2].dateAdded);
+        checkOnCreated(newBookmarks[1].id, createdFolderId, 0, "", "data:", newBookmarks[1].dateAdded, "separator");
         checkOnCreated(newBookmarks[0].id, createdFolderId, 0, "Mozilla", "http://allizom.org/", newBookmarks[0].dateAdded);
 
         return browser.bookmarks.create({
           title: "About Mozilla",
           url: "http://allizom.org/about/",
           parentId: createdFolderId,
           index: 1,
         });
       }).then(result => {
         browser.test.assertEq(1, collectedEvents.length, "1 expected events received");
         checkOnCreated(result.id, createdFolderId, 1, "About Mozilla", "http://allizom.org/about/", result.dateAdded);
 
         // returns all items on empty object
         return browser.bookmarks.search({});
       }).then(bookmarksSearchResults => {
-        browser.test.assertTrue(bookmarksSearchResults.length >= 9, "At least as many bookmarks as added were returned by search({})");
+        browser.test.assertTrue(bookmarksSearchResults.length >= 10, "At least as many bookmarks as added were returned by search({})");
 
         return Promise.resolve().then(() => {
           return browser.bookmarks.remove(createdFolderId);
         }).then(expectedError, error => {
           browser.test.assertTrue(
             error.message.includes("Cannot remove a non-empty folder"),
             "Expected error thrown when trying to remove a non-empty folder"
           );
           return browser.bookmarks.getSubTree(createdFolderId);
         });
       });
     }).then(results => {
       browser.test.assertEq(1, results.length, "Expected number of nodes returned by getSubTree");
       browser.test.assertEq("Mozilla Folder", results[0].title, "Folder has the expected title");
       browser.test.assertEq(bookmarkGuids.unfiledGuid, results[0].parentId, "Folder has the expected parentId");
+      browser.test.assertEq("folder", results[0].type, "Folder has the expected type");
       let children = results[0].children;
-      browser.test.assertEq(4, children.length, "Expected number of bookmarks returned by getSubTree");
+      browser.test.assertEq(5, children.length, "Expected number of bookmarks returned by getSubTree");
       browser.test.assertEq("Firefox", children[0].title, "Bookmark has the expected title");
+      browser.test.assertEq("bookmark", children[0].type, "Bookmark has the expected type");
       browser.test.assertEq("About Mozilla", children[1].title, "Bookmark has the expected title");
+      browser.test.assertEq("bookmark", children[1].type, "Bookmark has the expected type");
       browser.test.assertEq(1, children[1].index, "Bookmark has the expected index");
       browser.test.assertEq("Mozilla Corporation", children[2].title, "Bookmark has the expected title");
-      browser.test.assertEq("Mozilla", children[3].title, "Bookmark has the expected title");
+      browser.test.assertEq("", children[3].title, "Separator has the expected title");
+      browser.test.assertEq("data:", children[3].url, "Separator has the expected url");
+      browser.test.assertEq("separator", children[3].type, "Separator has the expected type");
+      browser.test.assertEq("Mozilla", children[4].title, "Bookmark has the expected title");
 
       // throws an error for invalid query objects
       Promise.resolve().then(() => {
         return browser.bookmarks.search();
       }).then(expectedError, error => {
         browser.test.assertTrue(
           error.message.includes("Incorrect argument types for bookmarks.search"),
           "Expected error thrown when trying to search with no arguments"
@@ -372,16 +389,17 @@ add_task(async function test_bookmarks()
       browser.test.assertEq(1, results.length, "Expected number of results returned for toolbar item search");
       checkBookmark({title: "Toolbar Item", url: "http://toolbar.org/", index: 0, parentId: bookmarkGuids.toolbarGuid}, results[0]);
 
       // finds folders
       return browser.bookmarks.search("Mozilla Folder");
     }).then(results => {
       browser.test.assertEq(1, results.length, "Expected number of folders returned");
       browser.test.assertEq("Mozilla Folder", results[0].title, "Folder has the expected title");
+      browser.test.assertEq("folder", results[0].type, "Folder has the expected type");
 
       // is case-insensitive
       return browser.bookmarks.search("corporation");
     }).then(results => {
       browser.test.assertEq(1, results.length, "Expected number of results returnedfor case-insensitive search");
       browser.test.assertEq("Mozilla Corporation", results[0].title, "Bookmark has the expected title");
 
       // is case-insensitive for non-ascii
@@ -416,23 +434,23 @@ add_task(async function test_bookmarks()
     }).then(results => {
       browser.test.assertEq(results.length, 1, "Expected number of results returned for normalized url field");
       checkBookmark({title: "Mozilla Corporation", url: "http://allizom.com/", index: 2}, results[0]);
 
       // accepts a title field
       return browser.bookmarks.search({title: "Mozilla"});
     }).then(results => {
       browser.test.assertEq(results.length, 1, "Expected number of results returned for title field");
-      checkBookmark({title: "Mozilla", url: "http://allizom.org/", index: 3}, results[0]);
+      checkBookmark({title: "Mozilla", url: "http://allizom.org/", index: 4}, results[0]);
 
       // can combine title and query
       return browser.bookmarks.search({title: "Mozilla", query: "allizom"});
     }).then(results => {
       browser.test.assertEq(1, results.length, "Expected number of results returned for title and query fields");
-      checkBookmark({title: "Mozilla", url: "http://allizom.org/", index: 3}, results[0]);
+      checkBookmark({title: "Mozilla", url: "http://allizom.org/", index: 4}, results[0]);
 
       // uses AND conditions
       return browser.bookmarks.search({title: "EFF", query: "allizom"});
     }).then(results => {
       browser.test.assertEq(
         0,
         results.length,
         "Expected number of results returned for non-matching title and query fields"
@@ -527,40 +545,52 @@ add_task(async function test_bookmarks()
       return browser.bookmarks.search({title: "Mozilla Folder"}).then(result => {
         return browser.bookmarks.removeTree(result[0].id);
       }).then(() => {
         browser.test.assertEq(1, collectedEvents.length, "1 expected events received");
         checkOnRemoved(createdFolderId, bookmarkGuids.unfiledGuid, 1);
 
         return browser.bookmarks.search({}).then(searchResults => {
           browser.test.assertEq(
-            startBookmarkCount - 4,
+            startBookmarkCount - 5,
             searchResults.length,
             "Expected number of results returned after removeTree");
         });
       });
     }).then(() => {
       return browser.bookmarks.create({title: "Empty Folder"});
     }).then(result => {
-      let emptyFolderId = result.id;
+      createdFolderId = result.id;
 
       browser.test.assertEq(1, collectedEvents.length, "1 expected events received");
-      checkOnCreated(emptyFolderId, bookmarkGuids.unfiledGuid, 3, "Empty Folder", undefined, result.dateAdded);
+      checkOnCreated(createdFolderId, bookmarkGuids.unfiledGuid, 3, "Empty Folder", undefined, result.dateAdded, "folder");
 
       browser.test.assertEq("Empty Folder", result.title, "Folder has the expected title");
-      return browser.bookmarks.remove(emptyFolderId).then(() => {
-        browser.test.assertEq(1, collectedEvents.length, "1 expected events received");
-        checkOnRemoved(emptyFolderId, bookmarkGuids.unfiledGuid, 3);
+      browser.test.assertEq("folder", result.type, "Folder has the expected type");
 
-        return browser.bookmarks.get(emptyFolderId).then(expectedError, error => {
-          browser.test.assertTrue(
-            error.message.includes("Bookmark not found"),
-            "Expected error thrown when trying to get a removed folder"
-          );
-        });
+      return browser.bookmarks.create({parentId: createdFolderId, type: "separator"});
+    }).then(result => {
+      createdSeparatorId = result.id;
+      browser.test.assertEq(1, collectedEvents.length, "1 expected events received");
+      checkOnCreated(createdSeparatorId, createdFolderId, 0, "", "data:", result.dateAdded, "separator");
+      return browser.bookmarks.remove(createdSeparatorId);
+    }).then(() => {
+      browser.test.assertEq(1, collectedEvents.length, "1 expected events received");
+      checkOnRemoved(createdSeparatorId, createdFolderId, 0, "data:", "separator");
+
+      return browser.bookmarks.remove(createdFolderId);
+    }).then(() => {
+      browser.test.assertEq(1, collectedEvents.length, "1 expected events received");
+      checkOnRemoved(createdFolderId, bookmarkGuids.unfiledGuid, 3);
+
+      return browser.bookmarks.get(createdFolderId).then(expectedError, error => {
+        browser.test.assertTrue(
+          error.message.includes("Bookmark not found"),
+          "Expected error thrown when trying to get a removed folder"
+        );
       });
     }).then(() => {
       return browser.bookmarks.getChildren(nonExistentId).then(expectedError, error => {
         browser.test.assertTrue(
           error.message.includes("root is null"),
           "Expected error thrown when trying to getChildren for a non-existent folder"
         );
       });