Bug 1067954 - Async Places Transactions: Paste functionality. r=mak
authorAsaf Romano <mano@mozilla.com>
Fri, 19 Sep 2014 16:53:27 +0300
changeset 206241 6ef192784187d7a477dfa402ce0a5b3a755d542e
parent 206240 30a7753deb4bda89d84600c6c9b37d2ec1d29210
child 206242 913568167ec54fb13c34285495275de26638cf2d
push id27517
push userryanvm@gmail.com
push dateFri, 19 Sep 2014 18:13:39 +0000
treeherdermozilla-central@a084c4cfd8a1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs1067954
milestone35.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1067954 - Async Places Transactions: Paste functionality. r=mak
browser/components/places/PlacesUIUtils.jsm
browser/components/places/content/browserPlacesViews.js
browser/components/places/content/controller.js
browser/components/places/content/menu.xml
browser/components/places/content/tree.xml
browser/components/places/content/treeView.js
toolkit/components/places/PlacesUtils.jsm
--- a/browser/components/places/PlacesUIUtils.jsm
+++ b/browser/components/places/PlacesUIUtils.jsm
@@ -8,49 +8,55 @@ this.EXPORTED_SYMBOLS = ["PlacesUIUtils"
 var Ci = Components.interfaces;
 var Cc = Components.classes;
 var Cr = Components.results;
 var Cu = Components.utils;
 
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 Cu.import("resource://gre/modules/Services.jsm");
 
+Cu.import("resource://gre/modules/PlacesUtils.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "PluralForm",
                                   "resource://gre/modules/PluralForm.jsm");
-
 XPCOMUtils.defineLazyModuleGetter(this, "PrivateBrowsingUtils",
                                   "resource://gre/modules/PrivateBrowsingUtils.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "NetUtil",
+                                  "resource://gre/modules/NetUtil.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "Task",
+                                  "resource://gre/modules/Task.jsm");
+
+// PlacesUtils exposes multiple symbols, so we can't use defineLazyModuleGetter.
+Cu.import("resource://gre/modules/PlacesUtils.jsm");
+
+XPCOMUtils.defineLazyModuleGetter(this, "PlacesTransactions",
+                                  "resource://gre/modules/PlacesTransactions.jsm");
 
 #ifdef MOZ_SERVICES_CLOUDSYNC
 XPCOMUtils.defineLazyModuleGetter(this, "CloudSync",
                                   "resource://gre/modules/CloudSync.jsm");
 #else
 let CloudSync = null;
 #endif
 
 #ifdef MOZ_SERVICES_SYNC
 XPCOMUtils.defineLazyModuleGetter(this, "Weave",
                                   "resource://services-sync/main.js");
 #endif
 
-XPCOMUtils.defineLazyGetter(this, "PlacesUtils", function() {
-  Cu.import("resource://gre/modules/PlacesUtils.jsm");
-  return PlacesUtils;
-});
+// copied from utilityOverlay.js
+const TAB_DROP_TYPE = "application/x-moz-tabbrowser-tab";
 
 this.PlacesUIUtils = {
   ORGANIZER_LEFTPANE_VERSION: 7,
   ORGANIZER_FOLDER_ANNO: "PlacesOrganizer/OrganizerFolder",
   ORGANIZER_QUERY_ANNO: "PlacesOrganizer/OrganizerQuery",
 
   LOAD_IN_SIDEBAR_ANNO: "bookmarkProperties/loadInSidebar",
   DESCRIPTION_ANNO: "bookmarkProperties/description",
 
-  TYPE_TAB_DROP: "application/x-moz-tabbrowser-tab",
-
   /**
    * Makes a URI from a spec, and do fixup
    * @param   aSpec
    *          The string spec of the URI
    * @returns A URI object for the spec.
    */
   createFixedURI: function PUIU_createFixedURI(aSpec) {
     return URIFixup.createFixupURI(aSpec, Ci.nsIURIFixup.FIXUP_FLAG_NONE);
@@ -323,27 +329,83 @@ this.PlacesUIUtils = {
         }
 
         // Otherwise move the item.
         return new PlacesMoveItemTransaction(data.id, container, index);
         break;
       default:
         if (type == PlacesUtils.TYPE_X_MOZ_URL ||
             type == PlacesUtils.TYPE_UNICODE ||
-            type == this.TYPE_TAB_DROP) {
+            type == TAB_DROP_TYPE) {
           let title = type != PlacesUtils.TYPE_UNICODE ? data.title
                                                        : data.uri;
           return new PlacesCreateBookmarkTransaction(PlacesUtils._uri(data.uri),
                                                      container, index, title);
         }
     }
     return null;
   },
 
   /**
+   * ********* PlacesTransactions version of the function defined above ********
+   *
+   * Constructs a Places Transaction for the drop or paste of a blob of data
+   * into a container.
+   *
+   * @param aData
+   *        The unwrapped data blob of dropped or pasted data.
+   * @param aType
+   *        The content type of the data.
+   * @param aNewParentGuid
+   *        GUID of the container the data was dropped or pasted into.
+   * @param aIndex
+   *        The index within the container the item was dropped or pasted at.
+   * @param aCopy
+   *        The drag action was copy, so don't move folders or links.
+   *
+   * @returns a Places Transaction that can be passed to
+   *          PlacesTranactions.transact for performing the move/insert command.
+   */
+  getTransactionForData: function(aData, aType, aNewParentGuid, aIndex, aCopy) {
+    if (this.SUPPORTED_FLAVORS.indexOf(aData.type) == -1)
+      throw new Error(`Unsupported '${aData.type}' data type`);
+
+    if ("itemGuid" in aData) {
+      if (this.PLACES_FLAVORS.indexOf(aData.type) == -1)
+        throw new Error (`itemGuid unexpectedly set on ${aData.type} data`);
+
+      let info = { GUID: aData.itemGuid
+                 , newParentGUID: aNewParentGuid
+                 , newIndex: aIndex };
+      if (aCopy)
+        return PlacesTransactions.Copy(info);
+      return PlacesTransactions.Move(info);
+    }
+
+    // Since it's cheap and harmless, we allow the paste of separators and
+    // bookmarks from builds that use legacy transactions (i.e. when itemGuid
+    // was not set on PLACES_FLAVORS data). Containers are a different story,
+    // and thus disallowed.
+    if (aData.type == PlacesUtils.TYPE_X_MOZ_PLACE_CONTAINER)
+      throw new Error("Can't copy a container from a legacy-transactions build");
+
+    if (aData.type == PlacesUtils.TYPE_X_MOZ_PLACE_SEPARATOR) {
+      return PlacesTransactions.NewSeparator({ parentGUID: aNewParentGuid
+                                             , index: aIndex });
+    }
+
+    let title = aData.type != PlacesUtils.TYPE_UNICODE ? aData.title
+                                                       : aData.uri;
+    return PlacesTransactions.NewBookmark({ uri: NetUtil.newURI(aData.uri)
+                                          , title: title
+                                          , parentGUID: aNewParentGuid
+                                          , index: aIndex });
+  },
+
+  /**
    * Shows the bookmark dialog corresponding to the specified info.
    *
    * @param aInfo
    *        Describes the item to be edited/added in the dialog.
    *        See documentation at the top of bookmarkProperties.js
    * @param aWindow
    *        Owner window for the new dialog.
    *
@@ -1019,16 +1081,28 @@ this.PlacesUIUtils = {
     let weaveEnabled = Weave.Service.isLoggedIn &&
                        Weave.Service.engineManager.get("tabs") &&
                        Weave.Service.engineManager.get("tabs").enabled;
     let cloudSyncEnabled = CloudSync && CloudSync.ready && CloudSync().tabsReady && CloudSync().tabs.hasRemoteTabs();
     return weaveEnabled || cloudSyncEnabled;
   },
 };
 
+
+PlacesUIUtils.PLACES_FLAVORS = [PlacesUtils.TYPE_X_MOZ_PLACE_CONTAINER,
+                                PlacesUtils.TYPE_X_MOZ_PLACE_SEPARATOR,
+                                PlacesUtils.TYPE_X_MOZ_PLACE];
+
+PlacesUIUtils.URI_FLAVORS = [PlacesUtils.TYPE_X_MOZ_URL,
+                             TAB_DROP_TYPE,
+                             PlacesUtils.TYPE_UNICODE],
+
+PlacesUIUtils.SUPPORTED_FLAVORS = [...PlacesUIUtils.PLACES_FLAVORS,
+                                   ...PlacesUIUtils.URI_FLAVORS];
+
 XPCOMUtils.defineLazyServiceGetter(PlacesUIUtils, "RDF",
                                    "@mozilla.org/rdf/rdf-service;1",
                                    "nsIRDFService");
 
 XPCOMUtils.defineLazyGetter(PlacesUIUtils, "ellipsis", function() {
   return Services.prefs.getComplexValue("intl.ellipsis",
                                         Ci.nsIPrefLocalizedString).data;
 });
--- a/browser/components/places/content/browserPlacesViews.js
+++ b/browser/components/places/content/browserPlacesViews.js
@@ -164,41 +164,42 @@ PlacesViewBase.prototype = {
         PlacesUtils.asQuery(resultNode).queryOptions.queryType ==
           Ci.nsINavHistoryQueryOptions.QUERY_TYPE_HISTORY)
       return null;
 
     // By default, the insertion point is at the top level, at the end.
     let index = PlacesUtils.bookmarks.DEFAULT_INDEX;
     let container = this._resultNode;
     let orientation = Ci.nsITreeView.DROP_BEFORE;
-    let isTag = false;
+    let tagName = null;
 
     let selectedNode = this.selectedNode;
     if (selectedNode) {
       let popup = document.popupNode;
       if (!popup._placesNode || popup._placesNode == this._resultNode ||
           popup._placesNode.itemId == -1) {
         // If a static menuitem is selected, or if the root node is selected,
         // the insertion point is inside the folder, at the end.
         container = selectedNode;
         orientation = Ci.nsITreeView.DROP_ON;
       }
       else {
         // In all other cases the insertion point is before that node.
         container = selectedNode.parent;
         index = container.getChildIndex(selectedNode);
-        isTag = PlacesUtils.nodeIsTagQuery(container);
+        if (PlacesUtils.nodeIsTagQuery(container))
+          tagName = container.title;
       }
     }
 
     if (PlacesControllerDragHelper.disallowInsertion(container))
       return null;
 
     return new InsertionPoint(PlacesUtils.getConcreteItemId(container),
-                              index, orientation, isTag);
+                              index, orientation, tagName);
   },
 
   buildContextMenu: function PVB_buildContextMenu(aPopup) {
     this._contextMenuShown = aPopup;
     window.updateCommands("places");
     return this.controller.buildContextMenu(aPopup);
   },
 
@@ -1387,20 +1388,22 @@ PlacesToolbar.prototype = {
           dropPoint.ip =
             new InsertionPoint(PlacesUtils.getConcreteItemId(this._resultNode),
                                eltIndex, Ci.nsITreeView.DROP_BEFORE);
           dropPoint.beforeIndex = eltIndex;
         }
         else if (this.isRTL ? (aEvent.clientX > eltRect.left + threshold)
                             : (aEvent.clientX < eltRect.right - threshold)) {
           // Drop inside this folder.
+          let tagName = PlacesUtils.nodeIsTagQuery(elt._placesNode) ?
+                        elt._placesNode.title : null;
           dropPoint.ip =
             new InsertionPoint(PlacesUtils.getConcreteItemId(elt._placesNode),
                                -1, Ci.nsITreeView.DROP_ON,
-                               PlacesUtils.nodeIsTagQuery(elt._placesNode));
+                               tagName);
           dropPoint.beforeIndex = eltIndex;
           dropPoint.folderElt = elt;
         }
         else {
           // Drop after this folder.
           let beforeIndex =
             (eltIndex == this._rootElt.childNodes.length - 1) ?
             -1 : eltIndex + 1;
--- a/browser/components/places/content/controller.js
+++ b/browser/components/places/content/controller.js
@@ -32,28 +32,28 @@ const REMOVE_PAGES_CHUNKLEN = 300;
  *          The identifier of the parent container
  * @param   aIndex
  *          The index within the container where we should insert
  * @param   aOrientation
  *          The orientation of the insertion. NOTE: the adjustments to the
  *          insertion point to accommodate the orientation should be done by
  *          the person who constructs the IP, not the user. The orientation
  *          is provided for informational purposes only!
- * @param   [optional] aIsTag
- *          Indicates if parent container is a tag
+ * @param   [optional] aTag
+ *          The tag name if this IP is set to a tag, null otherwise.
  * @param   [optional] aDropNearItemId
  *          When defined we will calculate index based on this itemId
  * @constructor
  */
-function InsertionPoint(aItemId, aIndex, aOrientation, aIsTag,
-                        aDropNearItemId) {
+function InsertionPoint(aItemId, aIndex, aOrientation, aTagName = null,
+                        aDropNearItemId = false) {
   this.itemId = aItemId;
   this._index = aIndex;
   this.orientation = aOrientation;
-  this.isTag = aIsTag;
+  this.tagName = aTagName;
   this.dropNearItemId = aDropNearItemId;
 }
 
 InsertionPoint.prototype = {
   set index(val) {
     return this._index = val;
   },
 
@@ -62,17 +62,19 @@ InsertionPoint.prototype = {
   get index() {
     if (this.dropNearItemId > 0) {
       // If dropNearItemId is set up we must calculate the real index of
       // the item near which we will drop.
       var index = PlacesUtils.bookmarks.getItemIndex(this.dropNearItemId);
       return this.orientation == Ci.nsITreeView.DROP_BEFORE ? index : index + 1;
     }
     return this._index;
-  }
+  },
+
+  get isTag() typeof(this.tagName) == "string"
 };
 
 /**
  * Places Controller
  */
 
 function PlacesController(aView) {
   this._view = aView;
@@ -122,24 +124,18 @@ PlacesController.prototype = {
     // filters out other commands that we do _not_ support (see 329587).
     const CMD_PREFIX = "placesCmd_";
     return (aCommand.substr(0, CMD_PREFIX.length) == CMD_PREFIX);
   },
 
   isCommandEnabled: function PC_isCommandEnabled(aCommand) {
     if (PlacesUIUtils.useAsyncTransactions) {
       switch (aCommand) {
-      case "cmd_cut":
-      case "placesCmd_cut":
-      case "cmd_copy":
-      case "cmd_paste":
       case "cmd_delete":
       case "placesCmd_delete":
-      case "cmd_paste":
-      case "placesCmd_paste":
       case "placesCmd_new:folder":
       case "placesCmd_new:bookmark":
       case "placesCmd_createBookmark":
         return false;
       }
     }
 
     switch (aCommand) {
@@ -238,17 +234,17 @@ PlacesController.prototype = {
       this.cut();
       break;
     case "cmd_copy":
     case "placesCmd_copy":
       this.copy();
       break;
     case "cmd_paste":
     case "placesCmd_paste":
-      this.paste();
+      this.paste().then(null, Components.utils.reportError);
       break;
     case "cmd_delete":
     case "placesCmd_delete":
       this.remove("Remove Selection");
       break;
     case "placesCmd_deleteDataHost":
       var host;
       if (PlacesUtils.nodeIsHost(this._view.selectedNode)) {
@@ -373,17 +369,17 @@ PlacesController.prototype = {
    * @return true if: - clipboard data is of a TYPE_X_MOZ_PLACE_* flavor,
    *                  - clipboard data is of type TEXT_UNICODE and
    *                    is a valid URI.
    */
   _isClipboardDataPasteable: function PC__isClipboardDataPasteable() {
     // if the clipboard contains TYPE_X_MOZ_PLACE_* data, it is definitely
     // pasteable, with no need to unwrap all the nodes.
 
-    var flavors = PlacesControllerDragHelper.placesFlavors;
+    var flavors = PlacesUIUtils.PLACES_FLAVORS;
     var clipboard = this.clipboard;
     var hasPlacesData =
       clipboard.hasDataMatchingFlavors(flavors, flavors.length,
                                        Ci.nsIClipboard.kGlobalClipboard);
     if (hasPlacesData)
       return this._view.insertionPoint != null;
 
     // if the clipboard doesn't have TYPE_X_MOZ_PLACE_* data, we also allow
@@ -1248,17 +1244,18 @@ PlacesController.prototype = {
       if (!didSuppressNotifications)
         result.suppressNotifications = false;
     }
   },
 
   /**
    * Paste Bookmarks and Folders from the clipboard
    */
-  paste: function PC_paste() {
+  paste: Task.async(function *() {
+
     // No reason to proceed if there isn't a valid insertion point.
     let ip = this._view.insertionPoint;
     if (!ip)
       throw Cr.NS_ERROR_NOT_AVAILABLE;
 
     let action = this.clipboardAction;
 
     let xferable = Cc["@mozilla.org/widget/transferable;1"].
@@ -1280,64 +1277,101 @@ PlacesController.prototype = {
       data = data.value.QueryInterface(Ci.nsISupportsString).data;
       type = type.value;
       items = PlacesUtils.unwrapNodes(data, type);
     } catch(ex) {
       // No supported data exists or nodes unwrap failed, just bail out.
       return;
     }
 
-    let transactions = [];
-    let insertionIndex = ip.index;
-    for (let i = 0; i < items.length; ++i) {
+    let itemsToSelect = [];
+    if (PlacesUIUtils.useAsyncTransactions) {
       if (ip.isTag) {
-        // Pasting into a tag container means tagging the item, regardless of
-        // the requested action.
-        let tagTxn = new PlacesTagURITransaction(NetUtil.newURI(items[i].uri),
-                                                 [ip.itemId]);
-        transactions.push(tagTxn);
-        continue;
+        let uris = [for (item of items) if ("uri" in item)
+                    NetUtil.newURI(item.uri)];
+        yield PlacesTransactions.transact(
+          PlacesTransactions.Tag({ uris: uris, tag: ip.tagName }));
+      }
+      else {
+        yield PlacesTransactions.transact(function *() {
+          let insertionIndex = ip.index;
+          let parent = yield ip.promiseGUID();
+
+          for (let item of items) {
+            let doCopy = action == "copy";
+
+            // If this is not a copy, check for safety that we can move the
+            // source, otherwise report an error and fallback to a copy.
+            if (!doCopy &&
+                !PlacesControllerDragHelper.canMoveUnwrappedNode(item)) {
+              Cu.reportError("Tried to move an unmovable Places node, " +
+                             "reverting to a copy operation.");
+              doCopy = true;
+            }
+            let guid = yield PlacesUIUtils.getTransactionForData(
+              item, type, parent, insertionIndex, doCopy);
+            itemsToSelect.push(yield PlacesUtils.promiseItemId(guid));
+
+            // Adjust index to make sure items are pasted in the correct
+            // position.  If index is DEFAULT_INDEX, items are just appended.
+            if (insertionIndex != PlacesUtils.bookmarks.DEFAULT_INDEX)
+              insertionIndex++;
+          }
+        });
+      }
+    }
+    else {
+      let transactions = [];
+      for (let i = 0; i < items.length; ++i) {
+        let insertionIndex = ip.index + i;
+        if (ip.isTag) {
+          // Pasting into a tag container means tagging the item, regardless of
+          // the requested action.
+          let tagTxn = new PlacesTagURITransaction(NetUtil.newURI(items[i].uri),
+                                                   [ip.itemId]);
+          transactions.push(tagTxn);
+          continue;
+        }
+
+        // Adjust index to make sure items are pasted in the correct position.
+        // If index is DEFAULT_INDEX, items are just appended.
+        if (ip.index != PlacesUtils.bookmarks.DEFAULT_INDEX)
+          insertionIndex = ip.index + i;
+
+        // If this is not a copy, check for safety that we can move the source,
+        // otherwise report an error and fallback to a copy.
+        if (action != "copy" && !PlacesControllerDragHelper.canMoveUnwrappedNode(items[i])) {
+          Components.utils.reportError("Tried to move an unmovable Places node, " +
+                                       "reverting to a copy operation.");
+          action = "copy";
+        }
+        transactions.push(
+          PlacesUIUtils.makeTransaction(items[i], type, ip.itemId,
+                                        insertionIndex, action == "copy")
+        );
       }
 
-      // Adjust index to make sure items are pasted in the correct position.
-      // If index is DEFAULT_INDEX, items are just appended.
-      if (ip.index != PlacesUtils.bookmarks.DEFAULT_INDEX)
-        insertionIndex = ip.index + i;
+      let aggregatedTxn = new PlacesAggregatedTransaction("Paste", transactions);
+      PlacesUtils.transactionManager.doTransaction(aggregatedTxn);
 
-      // If this is not a copy, check for safety that we can move the source,
-      // otherwise report an error and fallback to a copy.
-      if (action != "copy" && !PlacesControllerDragHelper.canMoveUnwrappedNode(items[i])) {
-        Components.utils.reportError("Tried to move an unmovable Places node, " +
-                                     "reverting to a copy operation.");
-        action = "copy";
+      for (let i = 0; i < transactions.length; ++i) {
+        itemsToSelect.push(
+          PlacesUtils.bookmarks.getIdForItemAt(ip.itemId, ip.index + i)
+        );
       }
-      transactions.push(
-        PlacesUIUtils.makeTransaction(items[i], type, ip.itemId,
-                                      insertionIndex, action == "copy")
-      );
     }
- 
-    let aggregatedTxn = new PlacesAggregatedTransaction("Paste", transactions);
-    PlacesUtils.transactionManager.doTransaction(aggregatedTxn);
 
     // Cut/past operations are not repeatable, so clear the clipboard.
     if (action == "cut") {
       this._clearClipboard();
     }
 
-    // Select the pasted items, they should be consecutive.
-    let insertedNodeIds = [];
-    for (let i = 0; i < transactions.length; ++i) {
-      insertedNodeIds.push(
-        PlacesUtils.bookmarks.getIdForItemAt(ip.itemId, ip.index + i)
-      );
-    }
-    if (insertedNodeIds.length > 0)
-      this._view.selectItems(insertedNodeIds, false);
-  },
+    if (itemsToSelect.length > 0)
+      this._view.selectItems(itemsToSelect, false);
+  }),
 
   /**
    * Cache the livemark info for a node.  This allows the controller and the
    * views to treat the given node as a livemark.
    * @param aNode
    *        a places result node.
    * @param aLivemarkInfo
    *        a mozILivemarkInfo object.
@@ -1407,17 +1441,17 @@ let PlacesControllerDragHelper = {
 
   /**
    * Extract the first accepted flavor from a list of flavors.
    * @param aFlavors
    *        The flavors list of type DOMStringList.
    */
   getFirstValidFlavor: function PCDH_getFirstValidFlavor(aFlavors) {
     for (let i = 0; i < aFlavors.length; i++) {
-      if (this.GENERIC_VIEW_DROP_TYPES.indexOf(aFlavors[i]) != -1)
+      if (PlacesUIUtils.SUPPORTED_FLAVORS.indexOf(aFlavors[i]) != -1)
         return aFlavors[i];
     }
 
     // If no supported flavor is found, check if data includes text/plain 
     // contents.  If so, request them as text/unicode, a conversion will happen 
     // automatically.
     if (aFlavors.contains("text/plain")) {
         return PlacesUtils.TYPE_UNICODE;
@@ -1642,29 +1676,17 @@ let PlacesControllerDragHelper = {
   disallowInsertion: function(aContainer) {
     NS_ASSERT(aContainer, "empty container");
     // Allow dropping into Tag containers.
     if (PlacesUtils.nodeIsTagQuery(aContainer))
       return false;
     // Disallow insertion of items under readonly folders.
     return (!PlacesUtils.nodeIsFolder(aContainer) ||
              PlacesUtils.nodeIsReadOnly(aContainer));
-  },
-
-  placesFlavors: [PlacesUtils.TYPE_X_MOZ_PLACE_CONTAINER,
-                  PlacesUtils.TYPE_X_MOZ_PLACE_SEPARATOR,
-                  PlacesUtils.TYPE_X_MOZ_PLACE],
-
-  // The order matters.
-  GENERIC_VIEW_DROP_TYPES: [PlacesUtils.TYPE_X_MOZ_PLACE_CONTAINER,
-                            PlacesUtils.TYPE_X_MOZ_PLACE_SEPARATOR,
-                            PlacesUtils.TYPE_X_MOZ_PLACE,
-                            PlacesUtils.TYPE_X_MOZ_URL,
-                            TAB_DROP_TYPE,
-                            PlacesUtils.TYPE_UNICODE],
+  }
 };
 
 
 XPCOMUtils.defineLazyServiceGetter(PlacesControllerDragHelper, "dragService",
                                    "@mozilla.org/widget/dragservice;1",
                                    "nsIDragService");
 
 function goUpdatePlacesCommands() {
--- a/browser/components/places/content/menu.xml
+++ b/browser/components/places/content/menu.xml
@@ -98,59 +98,62 @@
               let isMenu = elt.localName == "menu" ||
                  (elt.localName == "toolbarbutton" &&
                   elt.getAttribute("type") == "menu");
               if (isMenu && elt.lastChild &&
                   elt.lastChild.hasAttribute("placespopup"))
                 dropPoint.folderElt = elt;
               return dropPoint;
             }
-            else if ((PlacesUtils.nodeIsFolder(elt._placesNode) ||
-                      PlacesUtils.nodeIsTagQuery(elt._placesNode)) &&
-                     !PlacesUtils.nodeIsReadOnly(elt._placesNode)) {
+
+            let tagName = PlacesUtils.nodeIsTagQuery(elt._placesNode) ?
+                            elt._placesNode.title : null;
+            if ((PlacesUtils.nodeIsFolder(elt._placesNode) ||
+                 PlacesUtils.nodeIsTagQuery(elt._placesNode)) &&
+                !PlacesUtils.nodeIsReadOnly(elt._placesNode)) {
               // This is a folder or a tag container.
               if (eventY - eltY < eltHeight * 0.20) {
                 // If mouse is in the top part of the element, drop above folder.
                 dropPoint.ip = new InsertionPoint(
                                     PlacesUtils.getConcreteItemId(resultNode),
                                     -1,
                                     Ci.nsITreeView.DROP_BEFORE,
-                                    PlacesUtils.nodeIsTagQuery(elt._placesNode),
+                                    tagName,
                                     elt._placesNode.itemId);
                 return dropPoint;
               }
               else if (eventY - eltY < eltHeight * 0.80) {
                 // If mouse is in the middle of the element, drop inside folder.
                 dropPoint.ip = new InsertionPoint(
                                     PlacesUtils.getConcreteItemId(elt._placesNode),
                                     -1,
                                     Ci.nsITreeView.DROP_ON,
-                                    PlacesUtils.nodeIsTagQuery(elt._placesNode));
+                                    tagName);
                 dropPoint.folderElt = elt;
                 return dropPoint;
               }
             }
             else if (eventY - eltY <= eltHeight / 2) {
               // This is a non-folder node or a readonly folder.
               // If the mouse is above the middle, drop above this item.
               dropPoint.ip = new InsertionPoint(
                                   PlacesUtils.getConcreteItemId(resultNode),
                                   -1,
                                   Ci.nsITreeView.DROP_BEFORE,
-                                  PlacesUtils.nodeIsTagQuery(elt._placesNode),
+                                  tagName,
                                   elt._placesNode.itemId);
               return dropPoint;
             }
 
             // Drop below the item.
             dropPoint.ip = new InsertionPoint(
                                 PlacesUtils.getConcreteItemId(resultNode),
                                 -1,
                                 Ci.nsITreeView.DROP_AFTER,
-                                PlacesUtils.nodeIsTagQuery(elt._placesNode),
+                                tagName,
                                 elt._placesNode.itemId);
             return dropPoint;
         ]]></body>
       </method>
 
       <!-- Sub-menus should be opened when the mouse drags over them, and closed
            when the mouse drags off.  The overFolder object manages opening and
            closing of folders when the mouse hovers. -->
--- a/browser/components/places/content/tree.xml
+++ b/browser/components/places/content/tree.xml
@@ -524,19 +524,21 @@
                 index = orientation == Ci.nsITreeView.DROP_BEFORE ? lsi : lsi + 1;
               }
             }
           }
 
           if (PlacesControllerDragHelper.disallowInsertion(container))
             return null;
 
+          let tagName = PlacesUtils.nodeIsTagQuery(container) ?
+                          container.title : null;
           return new InsertionPoint(PlacesUtils.getConcreteItemId(container),
                                     index, orientation,
-                                    PlacesUtils.nodeIsTagQuery(container),
+                                    tagName,
                                     dropNearItemId);
         ]]></body>
       </method>
 
       <!-- nsIPlacesView -->
       <method name="selectAll">
         <body><![CDATA[
           this.view.selection.selectAll();
--- a/browser/components/places/content/treeView.js
+++ b/browser/components/places/content/treeView.js
@@ -1338,19 +1338,20 @@ PlacesTreeView.prototype = {
           index = orientation == Ci.nsITreeView.DROP_BEFORE ? lsi : lsi + 1;
         }
       }
     }
 
     if (PlacesControllerDragHelper.disallowInsertion(container))
       return null;
 
+    let tagName = PlacesUtils.nodeIsTagQuery(container) ? container.title : null;
     return new InsertionPoint(PlacesUtils.getConcreteItemId(container),
                               index, orientation,
-                              PlacesUtils.nodeIsTagQuery(container),
+                              tagName,
                               dropNearItemId);
   },
 
   drop: function PTV_drop(aRow, aOrientation, aDataTransfer) {
     // We are responsible for translating the |index| and |orientation|
     // parameters into a container id and index within the container,
     // since this information is specific to the tree view.
     let ip = this._getInsertionPoint(aRow, aOrientation);
--- a/toolkit/components/places/PlacesUtils.jsm
+++ b/toolkit/components/places/PlacesUtils.jsm
@@ -1110,17 +1110,19 @@ this.PlacesUtils = {
    *          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;
-      if (aJSNode.id != -1) {
+      let guid = aPlacesNode.bookmarkGuid;
+      if (guid) {
+        aJSNode.itemGuid = guid;
         var parent = aPlacesNode.parent;
         if (parent) {
           aJSNode.parent = parent.itemId;
           aJSNode.parentReadOnly = PlacesUtils.nodeIsReadOnly(parent);
         }
         var dateAdded = aPlacesNode.dateAdded;
         if (dateAdded)
           aJSNode.dateAdded = dateAdded;