Bug 425884 - Unable to copy bookmarks more than one folder deep (r=mano)

browser/components/places/content/utils.js

@@ -262,16 +262,8 @@ var PlacesUIUtils = {
         if (node.type == PlacesUtils.TYPE_X_MOZ_PLACE_CONTAINER) {
           if (node.livemark && node.annos) // node is a livemark
             txn = self._getLivemarkCopyTransaction(node, aContainer, index);
-          else {
-            var folderItemsTransactions = [];
-            if (node.dateAdded)
-              folderItemsTransactions.push(self.ptm.editItemDateAdded(null, node.dateAdded));
-            if (node.lastModified)
-              folderItemsTransactions.push(self.ptm.editItemLastModified(null, node.lastModified));
-            var annos = node.annos || [];
-            txn = self.ptm.createFolder(node.title, -1, index, annos,
-                                        folderItemsTransactions);
-          }
+          else
+            txn = self._getFolderCopyTransaction(node, aContainer, index);
         }
         else if (node.type == PlacesUtils.TYPE_X_MOZ_PLACE_SEPARATOR)
           txn = self.ptm.createSeparator(-1, index);

browser/components/places/src/nsPlacesTransactionsService.js

@@ -341,11 +341,11 @@ placesCreateFolderTransactions.prototype = {
   },
 
   undoTransaction: function PCFT_undoTransaction() {
-    PlacesUtils.bookmarks.removeFolder(this._id);
     for (var i = 0; i < this._childItemsTransactions.length; ++i) {
-      var txn = this.childItemsTransactions[i];
+      var txn = this._childItemsTransactions[i];
       txn.undoTransaction();
     }
+    PlacesUtils.bookmarks.removeFolder(this._id);
   }
 };
 
@@ -404,7 +404,7 @@ placesCreateSeparatorTransactions.prototype = {
 
   // childItemsTransaction support
   get container() { return this._container; },
-  set container(val) { return this._container = val;clear },
+  set container(val) { return this._container = val; },
 
   doTransaction: function PCST_doTransaction() {
     this._id = PlacesUtils.bookmarks
@@ -412,7 +412,7 @@ placesCreateSeparatorTransactions.prototype = {
   },
 
   undoTransaction: function PCST_undoTransaction() {
-    PlacesUtils.bookmarks.removeChildAt(this.container, this._index);
+    PlacesUtils.bookmarks.removeItem(this._id);
   }
 };
 
@@ -808,12 +808,12 @@ placesEditItemDateAddedTransaction.prototype = {
   get container() { return this.id; },
   set container(val) { return this.id = val; },
 
-  doTransaction: function PEITT_doTransaction() {
+  doTransaction: function PEIDA_doTransaction() {
     this._oldDateAdded = PlacesUtils.bookmarks.getItemDateAdded(this.id);
     PlacesUtils.bookmarks.setItemDateAdded(this.id, this._newDateAdded);
   },
 
-  undoTransaction: function PEITT_undoTransaction() {
+  undoTransaction: function PEIDA_undoTransaction() {
     PlacesUtils.bookmarks.setItemDateAdded(this.id, this._oldDateAdded);
   }
 };
@@ -832,12 +832,12 @@ placesEditItemLastModifiedTransaction.prototype = {
   get container() { return this.id; },
   set container(val) { return this.id = val; },
 
-  doTransaction: function PEITT_doTransaction() {
+  doTransaction: function PEILM_doTransaction() {
     this._oldLastModified = PlacesUtils.bookmarks.getItemLastModified(this.id);
     PlacesUtils.bookmarks.setItemLastModified(this.id, this._newLastModified);
   },
 
-  undoTransaction: function PEITT_undoTransaction() {
+  undoTransaction: function PEILM_undoTransaction() {
     PlacesUtils.bookmarks.setItemLastModified(this.id, this._oldLastModified);
   }
 };

browser/components/places/tests/Makefile.in

@@ -47,4 +47,8 @@ MODULE		= test_browser_places
 
 XPCSHELL_TESTS = unit
 
+ifdef MOZ_MOCHITEST
+	DIRS += browser
+endif
+
 include $(topsrcdir)/config/rules.mk

browser/components/places/tests/browser/Makefile.in

@@ -0,0 +1,51 @@
+# ***** BEGIN LICENSE BLOCK *****
+# Version: MPL 1.1/GPL 2.0/LGPL 2.1
+#
+# The contents of this file are subject to the Mozilla Public License Version
+# 1.1 (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+# http://www.mozilla.org/MPL/
+#
+# Software distributed under the License is distributed on an "AS IS" basis,
+# WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
+# for the specific language governing rights and limitations under the
+# License.
+#
+# The Original Code is Places test code.
+#
+# The Initial Developer of the Original Code is Mozilla Corp.
+# Portions created by the Initial Developer are Copyright (C) 2008
+# the Initial Developer. All Rights Reserved.
+#
+# Contributor(s):
+#  Dietrich Ayala <dietrich@mozilla.com>
+#
+# Alternatively, the contents of this file may be used under the terms of
+# either of the GNU General Public License Version 2 or later (the "GPL"),
+# or the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
+# in which case the provisions of the GPL or the LGPL are applicable instead
+# of those above. If you wish to allow use of your version of this file only
+# under the terms of either the GPL or the LGPL, and not to allow others to
+# use your version of this file under the terms of the MPL, indicate your
+# decision by deleting the provisions above and replace them with the notice
+# and other provisions required by the GPL or the LGPL. If you do not delete
+# the provisions above, a recipient may use your version of this file under
+# the terms of any one of the MPL, the GPL or the LGPL.
+#
+# ***** END LICENSE BLOCK *****
+
+DEPTH			= ../../../../..
+topsrcdir	= @top_srcdir@
+srcdir		= @srcdir@
+VPATH			= @srcdir@
+relativesrcdir  = browser/components/places/tests/browser
+
+include $(DEPTH)/config/autoconf.mk
+include $(topsrcdir)/config/rules.mk
+
+_BROWSER_TEST_FILES = \
+	browser_425884.js \
+	$(NULL)
+
+libs:: $(_BROWSER_TEST_FILES)
+	$(INSTALL) $(foreach f,$^,"$f") $(DEPTH)/_tests/testing/mochitest/browser/$(relativesrcdir)

browser/components/places/tests/browser/browser_425884.js

@@ -0,0 +1,122 @@
+/* vim:set ts=2 sw=2 sts=2 et: */
+/* ***** BEGIN LICENSE BLOCK *****
+ * Version: MPL 1.1/GPL 2.0/LGPL 2.1
+ *
+ * The contents of this file are subject to the Mozilla Public License Version
+ * 1.1 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ * http://www.mozilla.org/MPL/
+ *
+ * Software distributed under the License is distributed on an "AS IS" basis,
+ * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
+ * for the specific language governing rights and limitations under the
+ * License.
+ *
+ * The Original Code is Places test code.
+ *
+ * The Initial Developer of the Original Code is Mozilla Corp.
+ * Portions created by the Initial Developer are Copyright (C) 2008
+ * the Initial Developer. All Rights Reserved.
+ *
+ * Contributor(s):
+ *  Dietrich Ayala <dietrich@mozilla.com>
+ *
+ * Alternatively, the contents of this file may be used under the terms of
+ * either the GNU General Public License Version 2 or later (the "GPL"), or
+ * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
+ * in which case the provisions of the GPL or the LGPL are applicable instead
+ * of those above. If you wish to allow use of your version of this file only
+ * under the terms of either the GPL or the LGPL, and not to allow others to
+ * use your version of this file under the terms of the MPL, indicate your
+ * decision by deleting the provisions above and replace them with the notice
+ * and other provisions required by the GPL or the LGPL. If you do not delete
+ * the provisions above, a recipient may use your version of this file under
+ * the terms of any one of the MPL, the GPL or the LGPL.
+ *
+ * ***** END LICENSE BLOCK ***** */
+
+function test() {
+  // sanity check
+  ok(PlacesUtils, "checking PlacesUtils, running in chrome context?");
+  ok(PlacesUIUtils, "checking PlacesUIUtils, running in chrome context?");
+
+  /*
+   Deep copy of bookmark data, using the front-end codepath:
+
+   - create test folder A
+   - add a subfolder to folder A, and add items to it
+   - validate folder A (sanity check)
+   - copy folder A, creating new folder B, using the front-end path
+   - validate folder B
+   - undo copy transaction
+   - validate folder B (empty)
+   - redo copy transaction
+   - validate folder B's contents
+
+  */
+
+  var toolbarId = PlacesUtils.toolbarFolderId;
+  var toolbarNode = PlacesUtils.getFolderContents(toolbarId).root;
+  is(toolbarNode.childCount, 0, "confirm toolbar is empty");
+
+  // create folder A, fill it, validate it's contents
+  var folderAId = PlacesUtils.bookmarks.createFolder(toolbarId, "A", -1);
+  populate(folderAId);
+  var folderANode = PlacesUtils.getFolderContents(folderAId).root;
+  validate(folderANode);
+  is(toolbarNode.childCount, 1, "create test folder");
+
+  // copy it, using the front-end helper functions
+  var serializedNode = PlacesUtils.wrapNode(folderANode, PlacesUtils.TYPE_X_MOZ_PLACE_CONTAINER);
+  var rawNode = PlacesUtils.unwrapNodes(serializedNode, PlacesUtils.TYPE_X_MOZ_PLACE_CONTAINER).shift();
+  // confirm serialization
+  ok(rawNode.type, "confirm json node");
+  var transaction = PlacesUIUtils.makeTransaction(rawNode,
+                                                  PlacesUtils.TYPE_X_MOZ_PLACE_CONTAINER,
+                                                  toolbarId,
+                                                  -1,
+                                                  true);
+  ok(transaction, "create transaction");
+  PlacesUIUtils.ptm.doTransaction(transaction);
+  // confirm copy
+  is(toolbarNode.childCount, 2, "create test folder via copy");
+
+  // validate the copy
+  var folderBNode = toolbarNode.getChild(1);
+  validate(folderBNode);
+
+  // undo the transaction, confirm the removal
+  PlacesUIUtils.ptm.undoTransaction();
+  is(toolbarNode.childCount, 1, "confirm undo removed the copied folder");
+
+  // redo the transaction
+  PlacesUIUtils.ptm.redoTransaction();
+  is(toolbarNode.childCount, 2, "confirm redo re-copied the folder");
+  folderBNode = toolbarNode.getChild(1);
+  validate(folderBNode);
+
+  // clean up
+  PlacesUIUtils.ptm.undoTransaction();
+  PlacesUtils.bookmarks.removeItem(folderAId);
+}
+
+function populate(aFolderId) {
+  var folderId = PlacesUtils.bookmarks.createFolder(aFolderId, "test folder", -1);
+  PlacesUtils.bookmarks.insertBookmark(folderId, PlacesUtils._uri("http://foo"), -1, "test bookmark");
+  PlacesUtils.bookmarks.insertSeparator(folderId, -1);
+}
+
+function validate(aNode) {
+  asContainer(aNode);
+  aNode.containerOpen = true;
+  is(aNode.childCount, 1, "confirm child count match");
+  var folderNode = aNode.getChild(0);
+  is(folderNode.title, "test folder", "confirm folder title");
+  asContainer(folderNode);
+  folderNode.containerOpen = true;
+  is(folderNode.childCount, 2, "confirm child count match");
+  var bookmarkNode = folderNode.getChild(0);
+  var separatorNode = folderNode.getChild(1);
+  folderNode.containerOpen = false;
+  aNode.containerOpen = false;
+}

toolkit/components/places/src/utils.js

@@ -412,7 +412,7 @@ var PlacesUtils = {
     // organizer
     function convertNode(cNode) {
       if (self.nodeIsFolder(cNode) && asQuery(cNode).queryOptions.excludeItems)
-          return self.getFolderContents(cNode.itemId, false, true).root;
+        return self.getFolderContents(cNode.itemId, false, true).root;
       return cNode;
     }
 
@@ -426,7 +426,7 @@ var PlacesUtils = {
             this.value += aStr;
           }
         };
-        self.serializeNodeAsJSONToOutputStream(aNode, writer, true);
+        self.serializeNodeAsJSONToOutputStream(convertNode(aNode), writer, true);
         return writer.value;
       case this.TYPE_X_MOZ_URL:
         function gatherDataUrl(bNode) {