Bug 1095411 - PlacesUtils.wrapNode is using synchronous getKeywordForBookmark. r=ttaubert

browser/components/places/PlacesUIUtils.jsm

@@ -200,14 +200,21 @@ this.PlacesUIUtils = {
    *       annotations are synced from the old one.
    * @see this._copyableAnnotations for the list of copyable annotations.
    */
-  _getFolderCopyTransaction:
+  _getFolderCopyTransaction(aData, aContainer, aIndex) {
-  function PUIU__getFolderCopyTransaction(aData, aContainer, aIndex)
+    function getChildItemsTransactions(aRoot) {
-  {
-    function getChildItemsTransactions(aChildren)
-    {
       let transactions = [];
       let index = aIndex;
-      aChildren.forEach(function (node, i) {
+      for (let i = 0; i < aRoot.childCount; ++i) {
+        let child = aRoot.getChild(i);
+        // Temporary hacks until we switch to PlacesTransactions.jsm.
+        let isLivemark =
+          PlacesUtils.annotations.itemHasAnnotation(child.itemId,
+                                                    PlacesUtils.LMANNO_FEEDURI);
+        let [node] = PlacesUtils.unwrapNodes(
+          PlacesUtils.wrapNode(child, PlacesUtils.TYPE_X_MOZ_PLACE, isLivemark),
+          PlacesUtils.TYPE_X_MOZ_PLACE
+        );
+
         // Make sure that items are given the correct index, this will be
         // passed by the transaction manager to the backend for the insertion.
         // Insertion behaves differently for DEFAULT_INDEX (append).
@@ -238,19 +245,21 @@ this.PlacesUIUtils = {
         else {
           throw new Error("Unexpected item under a bookmarks folder");
         }
-      });
+      }
       return transactions;
     }
 
-    if (aContainer == PlacesUtils.tagsFolderId) { // Copying a tag folder.
+    if (aContainer == PlacesUtils.tagsFolderId) { // Copying into a tag folder.
       let transactions = [];
-      if (aData.children) {
+      if (!aData.livemark && aData.type == PlacesUtils.TYPE_X_MOZ_PLACE_CONTAINER) {
-        aData.children.forEach(function(aChild) {
+        let {root} = PlacesUtils.getFolderContents(aData.id, false, false);
+        let urls = PlacesUtils.getURLsForContainerNode(root);
+        root.containerOpen = false;
+        for (let { uri } of urls) {
           transactions.push(
-            new PlacesTagURITransaction(PlacesUtils._uri(aChild.uri),
+            new PlacesTagURITransaction(NetUtil.newURI(uri), [aData.title])
-                                        [aData.title])
           );
-        });
+        }
       }
       return new PlacesAggregatedTransaction("addTags", transactions);
     }
@@ -259,7 +268,10 @@ this.PlacesUIUtils = {
       return this._getLivemarkCopyTransaction(aData, aContainer, aIndex);
     }
 
-    let transactions = getChildItemsTransactions(aData.children);
+    let {root} = PlacesUtils.getFolderContents(aData.id, false, false);
+    let transactions = getChildItemsTransactions(root);
+    root.containerOpen = false;
+
     if (aData.dateAdded) {
       transactions.push(
         new PlacesEditItemDateAddedTransaction(null, aData.dateAdded)

browser/components/places/content/browserPlacesViews.js

@@ -205,8 +205,12 @@ PlacesViewBase.prototype = {
         // In all other cases the insertion point is before that node.
         container = selectedNode.parent;
         index = container.getChildIndex(selectedNode);
-        if (PlacesUtils.nodeIsTagQuery(container))
+        if (PlacesUtils.nodeIsTagQuery(container)) {
           tagName = container.title;
+          // TODO (Bug 1160193): properly support dropping on a tag root.
+          if (!tagName)
+            return null;
+        }
       }
     }
 

browser/components/places/content/controller.js

@@ -1055,15 +1055,15 @@ PlacesController.prototype = {
     if (!didSuppressNotifications)
       result.suppressNotifications = true;
 
-    function addData(type, index, overrideURI) {
+    function addData(type, index, feedURI) {
-      let wrapNode = PlacesUtils.wrapNode(node, type, overrideURI);
+      let wrapNode = PlacesUtils.wrapNode(node, type, feedURI);
       dt.mozSetDataAt(type, wrapNode, index);
     }
 
-    function addURIData(index, overrideURI) {
+    function addURIData(index, feedURI) {
-      addData(PlacesUtils.TYPE_X_MOZ_URL, index, overrideURI);
+      addData(PlacesUtils.TYPE_X_MOZ_URL, index, feedURI);
-      addData(PlacesUtils.TYPE_UNICODE, index, overrideURI);
+      addData(PlacesUtils.TYPE_UNICODE, index, feedURI);
-      addData(PlacesUtils.TYPE_HTML, index, overrideURI);
+      addData(PlacesUtils.TYPE_HTML, index, feedURI);
     }
 
     try {
@@ -1155,11 +1155,11 @@ PlacesController.prototype = {
         copiedFolders.push(node);
 
       let livemarkInfo = this.getCachedLivemarkInfo(node);
-      let overrideURI = livemarkInfo ? livemarkInfo.feedURI.spec : null;
+      let feedURI = livemarkInfo && livemarkInfo.feedURI.spec;
 
       contents.forEach(function (content) {
         content.entries.push(
-          PlacesUtils.wrapNode(node, content.type, overrideURI)
+          PlacesUtils.wrapNode(node, content.type, feedURI)
         );
       });
     }, this);
@@ -1499,7 +1499,7 @@ let PlacesControllerDragHelper = {
       }
 
       // Only bookmarks and urls can be dropped into tag containers.
-      if (ip.isTag && ip.orientation == Ci.nsITreeView.DROP_ON &&
+      if (ip.isTag &&
           dragged.type != PlacesUtils.TYPE_X_MOZ_URL &&
           (dragged.type != PlacesUtils.TYPE_X_MOZ_PLACE ||
            (dragged.uri && dragged.uri.startsWith("place:")) ))
@@ -1605,8 +1605,7 @@ let PlacesControllerDragHelper = {
         index+= movedCount++;
 
       // If dragging over a tag container we should tag the item.
-      if (insertionPoint.isTag &&
+      if (insertionPoint.isTag) {
-          insertionPoint.orientation == Ci.nsITreeView.DROP_ON) {
         let uri = NetUtil.newURI(unwrapped.uri);
         let tagItemId = insertionPoint.itemId;
         if (PlacesUIUtils.useAsyncTransactions)

browser/components/places/content/tree.xml

@@ -529,8 +529,14 @@
           if (PlacesControllerDragHelper.disallowInsertion(container))
             return null;
 
-          let tagName = PlacesUtils.nodeIsTagQuery(container) ?
+          // TODO (Bug 1160193): properly support dropping on a tag root.
-                          container.title : null;
+          let tagName = null;
+          if (PlacesUtils.nodeIsTagQuery(container)) {
+            tagName = container.title;
+            if (!tagName)
+              return null;
+          }
+
           return new InsertionPoint(PlacesUtils.getConcreteItemId(container),
                                     index, orientation,
                                     tagName,

browser/components/places/content/treeView.js

@@ -1334,7 +1334,14 @@ PlacesTreeView.prototype = {
     if (PlacesControllerDragHelper.disallowInsertion(container))
       return null;
 
-    let tagName = PlacesUtils.nodeIsTagQuery(container) ? container.title : null;
+    // TODO (Bug 1160193): properly support dropping on a tag root.
+    let tagName = null;
+    if (PlacesUtils.nodeIsTagQuery(container)) {
+      tagName = container.title;
+      if (!tagName)
+        return null;
+    }
+
     return new InsertionPoint(PlacesUtils.getConcreteItemId(container),
                               index, orientation,
                               tagName,

toolkit/components/places/PlacesUtils.jsm

@@ -121,6 +121,89 @@ function* notifyKeywordChange(url, keyword) {
   gIgnoreKeywordNotifications = false;
 }
 
+/**
+ * Serializes the given node in JSON format.
+ *
+ * @param aNode
+ *        An nsINavHistoryResultNode
+ * @param aIsLivemark
+ *        Whether the node represents a livemark.
+ */
+function serializeNode(aNode, aIsLivemark) {
+  let data = {};
+
+  data.title = aNode.title;
+  data.id = aNode.itemId;
+  data.livemark = aIsLivemark;
+
+  let guid = aNode.bookmarkGuid;
+  if (guid) {
+    data.itemGuid = guid;
+    if (aNode.parent)
+      data.parent = aNode.parent.itemId;
+    let grandParent = aNode.parent && aNode.parent.parent;
+    if (grandParent)
+      data.grandParentId = grandParent.itemId;
+
+    data.dateAdded = aNode.dateAdded;
+    data.lastModified = aNode.lastModified;
+
+    let annos = PlacesUtils.getAnnotationsForItem(data.id);
+    if (annos.length > 0)
+      data.annos = annos;
+  }
+
+  if (PlacesUtils.nodeIsURI(aNode)) {
+    // Check for url validity.
+    NetUtil.newURI(aNode.uri);
+
+    // Tag root accepts only folder nodes, not URIs.
+    if (data.parent == PlacesUtils.tagsFolderId)
+      throw new Error("Unexpected node type");
+
+    data.type = PlacesUtils.TYPE_X_MOZ_PLACE;
+    data.uri = aNode.uri;
+
+    if (aNode.tags)
+      data.tags = aNode.tags;
+  }
+  else if (PlacesUtils.nodeIsContainer(aNode)) {
+    // Tag containers accept only uri nodes.
+    if (data.grandParentId == PlacesUtils.tagsFolderId)
+      throw new Error("Unexpected node type");
+
+    let concreteId = PlacesUtils.getConcreteItemId(aNode);
+    if (concreteId != -1) {
+      // This is a bookmark or a tag container.
+      if (PlacesUtils.nodeIsQuery(aNode) || concreteId != aNode.itemId) {
+        // This is a folder shortcut.
+        data.type = PlacesUtils.TYPE_X_MOZ_PLACE;
+        data.uri = aNode.uri;
+        data.concreteId = concreteId;
+      }
+      else {
+        // This is a bookmark folder.
+        data.type = PlacesUtils.TYPE_X_MOZ_PLACE_CONTAINER;
+      }
+    }
+    else {
+      // This is a grouped container query, dynamically generated.
+      data.type = PlacesUtils.TYPE_X_MOZ_PLACE;
+      data.uri = aNode.uri;
+    }
+  }
+  else if (PlacesUtils.nodeIsSeparator(aNode)) {
+    // Tag containers don't accept separators.
+    if (data.parent == PlacesUtils.tagsFolderId ||
+        data.grandParentId == PlacesUtils.tagsFolderId)
+      throw new Error("Unexpected node type");
+
+    data.type = PlacesUtils.TYPE_X_MOZ_PLACE_SEPARATOR;
+  }
+
+  return JSON.stringify(data);
+}
+
 this.PlacesUtils = {
   // Place entries that are containers, e.g. bookmark folders or queries.
   TYPE_X_MOZ_PLACE_CONTAINER: "text/x-moz-place-container",
@@ -468,176 +551,125 @@ this.PlacesUtils = {
 
   /**
    * String-wraps a result node according to the rules of the specified
-   * content type.
+   * content type for copy or move operations.
+   *
    * @param   aNode
    *          The Result node to wrap (serialize)
    * @param   aType
    *          The content type to serialize as
-   * @param   [optional] aOverrideURI
+   * @param   [optional] aFeedURI
    *          Used instead of the node's URI if provided.
-   *          This is useful for wrapping a container as TYPE_X_MOZ_URL,
+   *          This is useful for wrapping a livemark as TYPE_X_MOZ_URL,
    *          TYPE_HTML or TYPE_UNICODE.
    * @return  A string serialization of the node
    */
-  wrapNode: function PU_wrapNode(aNode, aType, aOverrideURI) {
+  wrapNode(aNode, aType, aFeedURI) {
     // when wrapping a node, we want all the items, even if the original
     // query options are excluding them.
-    // this can happen when copying from the left hand pane of the bookmarks
+    // This can happen when copying from the left hand pane of the bookmarks
-    // organizer
+    // organizer.
     // @return [node, shouldClose]
-    function convertNode(cNode) {
+    function gatherDataFromNode(node, gatherDataFunc) {
-      if (PlacesUtils.nodeIsFolder(cNode) &&
+      if (PlacesUtils.nodeIsFolder(node) &&
-          cNode.type != Ci.nsINavHistoryResultNode.RESULT_TYPE_FOLDER_SHORTCUT &&
+          node.type != Ci.nsINavHistoryResultNode.RESULT_TYPE_FOLDER_SHORTCUT &&
-          asQuery(cNode).queryOptions.excludeItems) {
+          asQuery(node).queryOptions.excludeItems) {
-        return [PlacesUtils.getFolderContents(cNode.itemId, false, true).root, true];
+        let node = PlacesUtils.getFolderContents(node.itemId, false, true).root;
+        try {
+          return gatherDataFunc(node);
+        } finally {
+          node.containerOpen = false;
+        }
       }
-
+      // If we didn't create our own query, do not alter the node's state.
-      // If we didn't create our own query, do not alter the node's open state.
+      return gatherDataFunc(node);
-      return [cNode, false];
     }
 
-    function gatherLivemarkUrl(aNode) {
+    function gatherDataHtml(node) {
-      try {
+      let htmlEscape = s => s.replace(/&/g, "&")
-        return PlacesUtils.annotations
+                             .replace(/>/g, ">")
-                          .getItemAnnotation(aNode.itemId,
+                             .replace(/</g, "&lt;")
-                                             PlacesUtils.LMANNO_SITEURI);
+                             .replace(/"/g, "&quot;")
-      } catch (ex) {
+                             .replace(/'/g, "&apos;");
-        return PlacesUtils.annotations
+
-                          .getItemAnnotation(aNode.itemId,
+      // escape out potential HTML in the title
-                                             PlacesUtils.LMANNO_FEEDURI);
+      let escapedTitle = node.title ? htmlEscape(node.title) : "";
+
+      if (aFeedURI) {
+        return `<A HREF="${aFeedURI}">${escapedTitle}</A>${NEWLINE}`;
       }
+
+      if (PlacesUtils.nodeIsContainer(node)) {
+        asContainer(node);
+        let wasOpen = node.containerOpen;
+        if (!wasOpen)
+          node.containerOpen = true;
+
+        let childString = "<DL><DT>" + escapedTitle + "</DT>" + NEWLINE;
+        let cc = node.childCount;
+        for (let i = 0; i < cc; ++i) {
+          childString += "<DD>"
+                         + NEWLINE
+                         + gatherDataHtml(node.getChild(i))
+                         + "</DD>"
+                         + NEWLINE;
+        }
+        node.containerOpen = wasOpen;
+        return childString + "</DL>" + NEWLINE;
+      }
+      if (PlacesUtils.nodeIsURI(node))
+        return `<A HREF="${node.uri}">${escapedTitle}</A>${NEWLINE}`;
+      if (PlacesUtils.nodeIsSeparator(node))
+        return "<HR>" + NEWLINE;
+      return "";
     }
 
-    function isLivemark(aNode) {
+    function gatherDataText(node) {
-      return PlacesUtils.nodeIsFolder(aNode) &&
+      if (aFeedURI) {
-             PlacesUtils.annotations
+        return aFeedURI;
-                        .itemHasAnnotation(aNode.itemId,
+      }
-                                           PlacesUtils.LMANNO_FEEDURI);
+
+      if (PlacesUtils.nodeIsContainer(node)) {
+        asContainer(node);
+        let wasOpen = node.containerOpen;
+        if (!wasOpen)
+          node.containerOpen = true;
+
+        let childString = node.title + NEWLINE;
+        let cc = node.childCount;
+        for (let i = 0; i < cc; ++i) {
+          let child = node.getChild(i);
+          let suffix = i < (cc - 1) ? NEWLINE : "";
+          childString += gatherDataText(child) + suffix;
+        }
+        node.containerOpen = wasOpen;
+        return childString;
+      }
+      if (PlacesUtils.nodeIsURI(node))
+        return node.uri;
+      if (PlacesUtils.nodeIsSeparator(node))
+        return "--------------------";
+      return "";
     }
 
     switch (aType) {
       case this.TYPE_X_MOZ_PLACE:
       case this.TYPE_X_MOZ_PLACE_SEPARATOR:
       case this.TYPE_X_MOZ_PLACE_CONTAINER: {
-        let writer = {
+        // Serialize the node to JSON.
-          value: "",
+        return serializeNode(aNode, aFeedURI);
-          write: function PU_wrapNode__write(aStr, aLen) {
-            this.value += aStr;
-          }
-        };
-
-        let [node, shouldClose] = convertNode(aNode);
-        this._serializeNodeAsJSONToOutputStream(node, writer);
-        if (shouldClose)
-          node.containerOpen = false;
-
-        return writer.value;
       }
       case this.TYPE_X_MOZ_URL: {
-        let gatherDataUrl = function (bNode) {
+        if (aFeedURI || PlacesUtils.nodeIsURI(aNode))
-          if (isLivemark(bNode)) {
+          return (aFeedURI || aNode.uri) + NEWLINE + aNode.title;
-            return gatherLivemarkUrl(bNode) + NEWLINE + bNode.title;
+        return "";
-          }
-
-          if (PlacesUtils.nodeIsURI(bNode))
-            return (aOverrideURI || bNode.uri) + NEWLINE + bNode.title;
-          // ignore containers and separators - items without valid URIs
-          return "";
-        };
-
-        let [node, shouldClose] = convertNode(aNode);
-        let dataUrl = gatherDataUrl(node);
-        if (shouldClose)
-          node.containerOpen = false;
-
-        return dataUrl;
       }
       case this.TYPE_HTML: {
-        let gatherDataHtml = function (bNode) {
+        return gatherDataFromNode(aNode, gatherDataHtml);
-          let htmlEscape = function (s) {
-            s = s.replace(/&/g, "&amp;");
-            s = s.replace(/>/g, "&gt;");
-            s = s.replace(/</g, "&lt;");
-            s = s.replace(/"/g, "&quot;");
-            s = s.replace(/'/g, "&apos;");
-            return s;
-          };
-          // escape out potential HTML in the title
-          let escapedTitle = bNode.title ? htmlEscape(bNode.title) : "";
-
-          if (isLivemark(bNode)) {
-            return "<A HREF=\"" + gatherLivemarkUrl(bNode) + "\">" + escapedTitle + "</A>" + NEWLINE;
-          }
-
-          if (PlacesUtils.nodeIsContainer(bNode)) {
-            asContainer(bNode);
-            let wasOpen = bNode.containerOpen;
-            if (!wasOpen)
-              bNode.containerOpen = true;
-
-            let childString = "<DL><DT>" + escapedTitle + "</DT>" + NEWLINE;
-            let cc = bNode.childCount;
-            for (let i = 0; i < cc; ++i)
-              childString += "<DD>"
-                             + NEWLINE
-                             + gatherDataHtml(bNode.getChild(i))
-                             + "</DD>"
-                             + NEWLINE;
-            bNode.containerOpen = wasOpen;
-            return childString + "</DL>" + NEWLINE;
-          }
-          if (PlacesUtils.nodeIsURI(bNode))
-            return "<A HREF=\"" + bNode.uri + "\">" + escapedTitle + "</A>" + NEWLINE;
-          if (PlacesUtils.nodeIsSeparator(bNode))
-            return "<HR>" + NEWLINE;
-          return "";
-        }
-
-        let [node, shouldClose] = convertNode(aNode);
-        let dataHtml = gatherDataHtml(node);
-        if (shouldClose)
-          node.containerOpen = false;
-
-        return dataHtml;
       }
     }
 
     // Otherwise, we wrap as TYPE_UNICODE.
-    function gatherDataText(bNode) {
+    return gatherDataFromNode(aNode, gatherDataText);
-      if (isLivemark(bNode)) {
-        return gatherLivemarkUrl(bNode);
-      }
-
-      if (PlacesUtils.nodeIsContainer(bNode)) {
-        asContainer(bNode);
-        let wasOpen = bNode.containerOpen;
-        if (!wasOpen)
-          bNode.containerOpen = true;
-
-        let childString = bNode.title + NEWLINE;
-        let cc = bNode.childCount;
-        for (let i = 0; i < cc; ++i) {
-          let child = bNode.getChild(i);
-          let suffix = i < (cc - 1) ? NEWLINE : "";
-          childString += gatherDataText(child) + suffix;
-        }
-        bNode.containerOpen = wasOpen;
-        return childString;
-      }
-      if (PlacesUtils.nodeIsURI(bNode))
-        return (aOverrideURI || bNode.uri);
-      if (PlacesUtils.nodeIsSeparator(bNode))
-        return "--------------------";
-      return "";
-    }
-
-    let [node, shouldClose] = convertNode(aNode);
-    let dataText = gatherDataText(node);
-    // Convert node could pass an open container node.
-    if (shouldClose)
-      node.containerOpen = false;
-
-    return dataText;
   },
 
   /**
@@ -1141,207 +1173,6 @@ this.PlacesUtils = {
     return urls;
   },
 
-  /**
-   * Serializes the given node (and all its descendents) as JSON
-   * and writes the serialization to the given output stream.
-   * 
-   * @param   aNode
-   *          An nsINavHistoryResultNode
-   * @param   aStream
-   *          An nsIOutputStream. NOTE: it only uses the write(str, len)
-   *          method of nsIOutputStream. The caller is responsible for
-   *          closing the stream.
-   */
-  _serializeNodeAsJSONToOutputStream: function (aNode, aStream) {
-    function addGenericProperties(aPlacesNode, aJSNode) {
-      aJSNode.title = aPlacesNode.title;
-      aJSNode.id = aPlacesNode.itemId;
-      let guid = aPlacesNode.bookmarkGuid;
-      if (guid) {
-        aJSNode.itemGuid = guid;
-        var parent = aPlacesNode.parent;
-        if (parent)
-          aJSNode.parent = parent.itemId;
-
-        var dateAdded = aPlacesNode.dateAdded;
-        if (dateAdded)
-          aJSNode.dateAdded = dateAdded;
-        var lastModified = aPlacesNode.lastModified;
-        if (lastModified)
-          aJSNode.lastModified = lastModified;
-
-        // XXX need a hasAnnos api
-        var annos = [];
-        try {
-          annos = PlacesUtils.getAnnotationsForItem(aJSNode.id).filter(function(anno) {
-            // XXX should whitelist this instead, w/ a pref for
-            // backup/restore of non-whitelisted annos
-            // XXX causes JSON encoding errors, so utf-8 encode
-            //anno.value = unescape(encodeURIComponent(anno.value));
-            if (anno.name == PlacesUtils.LMANNO_FEEDURI)
-              aJSNode.livemark = 1;
-            return true;
-          });
-        } catch(ex) {}
-        if (annos.length != 0)
-          aJSNode.annos = annos;
-      }
-      // XXXdietrich - store annos for non-bookmark items
-    }
-
-    function addURIProperties(aPlacesNode, aJSNode) {
-      aJSNode.type = PlacesUtils.TYPE_X_MOZ_PLACE;
-      aJSNode.uri = aPlacesNode.uri;
-      if (aJSNode.id && aJSNode.id != -1) {
-        // harvest bookmark-specific properties
-        var keyword = PlacesUtils.bookmarks.getKeywordForBookmark(aJSNode.id);
-        if (keyword)
-          aJSNode.keyword = keyword;
-      }
-
-      if (aPlacesNode.tags)
-        aJSNode.tags = aPlacesNode.tags;
-
-      // last character-set
-      var uri = PlacesUtils._uri(aPlacesNode.uri);
-      try {
-        var lastCharset = PlacesUtils.annotations.getPageAnnotation(
-                            uri, PlacesUtils.CHARSET_ANNO);
-        aJSNode.charset = lastCharset;
-      } catch (e) {}
-    }
-
-    function addSeparatorProperties(aPlacesNode, aJSNode) {
-      aJSNode.type = PlacesUtils.TYPE_X_MOZ_PLACE_SEPARATOR;
-    }
-
-    function addContainerProperties(aPlacesNode, aJSNode) {
-      var concreteId = PlacesUtils.getConcreteItemId(aPlacesNode);
-      if (concreteId != -1) {
-        // This is a bookmark or a tag container.
-        if (PlacesUtils.nodeIsQuery(aPlacesNode) ||
-            concreteId != aPlacesNode.itemId) {
-          aJSNode.type = PlacesUtils.TYPE_X_MOZ_PLACE;
-          aJSNode.uri = aPlacesNode.uri;
-          // folder shortcut
-          aJSNode.concreteId = concreteId;
-        }
-        else { // Bookmark folder or a shortcut we should convert to folder.
-          aJSNode.type = PlacesUtils.TYPE_X_MOZ_PLACE_CONTAINER;
-
-          // Mark root folders.
-          if (aJSNode.id == PlacesUtils.placesRootId)
-            aJSNode.root = "placesRoot";
-          else if (aJSNode.id == PlacesUtils.bookmarksMenuFolderId)
-            aJSNode.root = "bookmarksMenuFolder";
-          else if (aJSNode.id == PlacesUtils.tagsFolderId)
-            aJSNode.root = "tagsFolder";
-          else if (aJSNode.id == PlacesUtils.unfiledBookmarksFolderId)
-            aJSNode.root = "unfiledBookmarksFolder";
-          else if (aJSNode.id == PlacesUtils.toolbarFolderId)
-            aJSNode.root = "toolbarFolder";
-        }
-      }
-      else {
-        // This is a grouped container query, generated on the fly.
-        aJSNode.type = PlacesUtils.TYPE_X_MOZ_PLACE;
-        aJSNode.uri = aPlacesNode.uri;
-      }
-    }
-
-    function appendConvertedComplexNode(aNode, aSourceNode, aArray) {
-      var repr = {};
-
-      for (let [name, value] in Iterator(aNode))
-        repr[name] = value;
-
-      // write child nodes
-      var children = repr.children = [];
-      if (!aNode.livemark) {
-        asContainer(aSourceNode);
-        var wasOpen = aSourceNode.containerOpen;
-        if (!wasOpen)
-          aSourceNode.containerOpen = true;
-        var cc = aSourceNode.childCount;
-        for (var i = 0; i < cc; ++i) {
-          var childNode = aSourceNode.getChild(i);
-          appendConvertedNode(aSourceNode.getChild(i), i, children);
-        }
-        if (!wasOpen)
-          aSourceNode.containerOpen = false;
-      }
-
-      aArray.push(repr);
-      return true;
-    }
-
-    function appendConvertedNode(bNode, aIndex, aArray) {
-      var node = {};
-
-      // set index in order received
-      // XXX handy shortcut, but are there cases where we don't want
-      // to export using the sorting provided by the query?
-      if (aIndex)
-        node.index = aIndex;
-
-      addGenericProperties(bNode, node);
-
-      var parent = bNode.parent;
-      var grandParent = parent ? parent.parent : null;
-      if (grandParent)
-        node.grandParentId = grandParent.itemId;
-
-      if (PlacesUtils.nodeIsURI(bNode)) {
-        // Tag root accept only folder nodes
-        if (parent && parent.itemId == PlacesUtils.tagsFolderId)
-          return false;
-
-        // Check for url validity, since we can't halt while writing a backup.
-        // This will throw if we try to serialize an invalid url and it does
-        // not make sense saving a wrong or corrupt uri node.
-        try {
-          PlacesUtils._uri(bNode.uri);
-        } catch (ex) {
-          return false;
-        }
-
-        addURIProperties(bNode, node);
-      }
-      else if (PlacesUtils.nodeIsContainer(bNode)) {
-        // Tag containers accept only uri nodes
-        if (grandParent && grandParent.itemId == PlacesUtils.tagsFolderId)
-          return false;
-
-        addContainerProperties(bNode, node);
-      }
-      else if (PlacesUtils.nodeIsSeparator(bNode)) {
-        // Tag root accept only folder nodes
-        // Tag containers accept only uri nodes
-        if ((parent && parent.itemId == PlacesUtils.tagsFolderId) ||
-            (grandParent && grandParent.itemId == PlacesUtils.tagsFolderId))
-          return false;
-
-        addSeparatorProperties(bNode, node);
-      }
-
-      if (!node.feedURI && node.type == PlacesUtils.TYPE_X_MOZ_PLACE_CONTAINER)
-        return appendConvertedComplexNode(node, bNode, aArray);
-
-      aArray.push(node);
-      return true;
-    }
-
-    // serialize to stream
-    var array = [];
-    if (appendConvertedNode(aNode, null, array)) {
-      var json = JSON.stringify(array[0]);
-      aStream.write(json, json.length);
-    }
-    else {
-      throw Cr.NS_ERROR_UNEXPECTED;
-    }
-  },
-
   /**
    * Gets the shared Sqlite.jsm readonly connection to the Places database.
    * This is intended to be used mostly internally, and by other Places modules.
@@ -3126,7 +2957,7 @@ PlacesEditBookmarkURITransaction.prototype = {
     PlacesUtils.bookmarks.changeBookmarkURI(this.item.id, this.new.uri);
     // move tags from old URI to new URI
     this.item.tags = PlacesUtils.tagging.getTagsForURI(this.item.uri);
-    if (this.item.tags.length != 0) {
+    if (this.item.tags.length > 0) {
       // only untag the old URI if this is the only bookmark
       if (PlacesUtils.getBookmarksForURI(this.item.uri, {}).length == 0)
         PlacesUtils.tagging.untagURI(this.item.uri, this.item.tags);
@@ -3138,7 +2969,7 @@ PlacesEditBookmarkURITransaction.prototype = {
   {
     PlacesUtils.bookmarks.changeBookmarkURI(this.item.id, this.item.uri);
     // move tags from new URI to old URI 
-    if (this.item.tags.length != 0) {
+    if (this.item.tags.length > 0) {
       // only untag the new URI if this is the only bookmark
       if (PlacesUtils.getBookmarksForURI(this.new.uri, {}).length == 0)
         PlacesUtils.tagging.untagURI(this.new.uri, this.item.tags);