diff --git a/mozilla/browser/components/places/content/utils.js b/mozilla/browser/components/places/content/utils.js index 5d3d3f3eb16..a8f5394cbc1 100644 --- a/mozilla/browser/components/places/content/utils.js +++ b/mozilla/browser/components/places/content/utils.js @@ -56,6 +56,7 @@ __defineGetter__("PlacesUtils", function() { const LOAD_IN_SIDEBAR_ANNO = "bookmarkProperties/loadInSidebar"; const DESCRIPTION_ANNO = "bookmarkProperties/description"; +const GUID_ANNO = "placesInternal/GUID"; const LMANNO_FEEDURI = "livemark/feedURI"; const LMANNO_SITEURI = "livemark/siteURI"; const ORGANIZER_FOLDER_ANNO = "PlacesOrganizer/OrganizerFolder"; @@ -206,11 +207,13 @@ var PlacesUIUtils = { var itemTitle = aData.title; var keyword = aData.keyword || null; var annos = aData.annos || []; - if (aExcludeAnnotations) { - annos = annos.filter(function(aValue, aIndex, aArray) { - return aExcludeAnnotations.indexOf(aValue.name) == -1; - }); - } + // always exclude GUID when copying any item + var excludeAnnos = [GUID_ANNO]; + if (aExcludeAnnotations) + excludeAnnos = excludeAnnos.concat(aExcludeAnnotations); + annos = annos.filter(function(aValue, aIndex, aArray) { + return excludeAnnos.indexOf(aValue.name) == -1; + }); var childTxns = []; if (aData.dateAdded) childTxns.push(this.ptm.editItemDateAdded(null, aData.dateAdded)); @@ -301,6 +304,10 @@ var PlacesUIUtils = { childItems.push(this.ptm.editItemLastModified(null, aData.lastModified)); var annos = aData.annos || []; + annos = annos.filter(function(aAnno) { + // always exclude GUID when copying any item + return aAnno.name != GUID_ANNO; + }); return this.ptm.createFolder(aData.title, aContainer, aIndex, annos, childItems); } }, @@ -320,8 +327,9 @@ var PlacesUIUtils = { siteURI = PlacesUtils._uri(aAnno.value); return false; } - return true; - }, this); + // always exclude GUID when copying any item + return aAnno.name != GUID_ANNO; + }); return this.ptm.createLivemark(feedURI, siteURI, aData.title, aContainer, aIndex, aData.annos); }, @@ -360,10 +368,8 @@ var PlacesUIUtils = { if (copy) { // Copying a child of a live-bookmark by itself should result // as a new normal bookmark item (bug 376731) - var copyBookmarkAnno = - this._getBookmarkItemCopyTransaction(data, container, index, - ["livemark/bookmarkFeedURI"]); - return copyBookmarkAnno; + return this._getBookmarkItemCopyTransaction(data, container, index, + ["livemark/bookmarkFeedURI"]); } else return this.ptm.moveItem(data.id, container, index); diff --git a/mozilla/browser/components/places/tests/browser/Makefile.in b/mozilla/browser/components/places/tests/browser/Makefile.in index 0302bca7779..0ea61e5d30e 100644 --- a/mozilla/browser/components/places/tests/browser/Makefile.in +++ b/mozilla/browser/components/places/tests/browser/Makefile.in @@ -46,6 +46,7 @@ include $(topsrcdir)/config/rules.mk _BROWSER_TEST_FILES = \ browser_425884.js \ browser_423515.js \ + browser_457473_no_copy_guid.js \ $(NULL) libs:: $(_BROWSER_TEST_FILES) diff --git a/mozilla/browser/components/places/tests/browser/browser_457473_no_copy_guid.js b/mozilla/browser/components/places/tests/browser/browser_457473_no_copy_guid.js new file mode 100644 index 00000000000..e86cc74a317 --- /dev/null +++ b/mozilla/browser/components/places/tests/browser/browser_457473_no_copy_guid.js @@ -0,0 +1,153 @@ +/* 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 + * + * 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?"); + + /* + - create, a test folder, add bookmark, separator, livemark to it + - fetch guids for all + - copy the folder + - test that guids are all different + - undo copy + - redo copy + - test that guids for the copy stay the same + */ + + var toolbarId = PlacesUtils.toolbarFolderId; + var toolbarNode = PlacesUtils.getFolderContents(toolbarId).root; + + var oldCount = toolbarNode.childCount; + var testRootId = PlacesUtils.bookmarks.createFolder(toolbarId, "test root", -1); + is(toolbarNode.childCount, oldCount+1, "confirm test root node is a container, and is empty"); + var testRootNode = toolbarNode.getChild(toolbarNode.childCount-1); + asContainer(testRootNode); + testRootNode.containerOpen = true; + is(testRootNode.childCount, 0, "confirm test root node is a container, and is empty"); + + // create folder A, fill it w/ each item type + var folderAId = PlacesUtils.bookmarks.createFolder(testRootId, "A", -1); + PlacesUtils.bookmarks.insertBookmark(folderAId, PlacesUtils._uri("http://foo"), + -1, "test bookmark"); + PlacesUtils.bookmarks.insertSeparator(folderAId, -1); + PlacesUtils.livemarks.createLivemarkFolderOnly(folderAId, "test livemark", + PlacesUtils._uri("http://test"), + PlacesUtils._uri("http://test"), -1); + + var folderANode = testRootNode.getChild(0); + var folderAGUIDs = getGUIDs(folderANode); + + // test the test function + ok(checkGUIDs(folderANode, folderAGUIDs, true), "confirm guid test works");; + + // serialize the folder + var serializedNode = PlacesUtils.wrapNode(folderANode, PlacesUtils.TYPE_X_MOZ_PLACE_CONTAINER); + var rawNode = PlacesUtils.unwrapNodes(serializedNode, PlacesUtils.TYPE_X_MOZ_PLACE_CONTAINER).shift(); + ok(rawNode.type, "confirm json node was made"); + + // create a transaction from the serialization + // this exercises the guid-filtering + var transaction = PlacesUIUtils.makeTransaction(rawNode, + PlacesUtils.TYPE_X_MOZ_PLACE_CONTAINER, + testRootId, -1, true); + ok(transaction, "create transaction"); + + // execute it, copying to the test root folder + PlacesUIUtils.ptm.doTransaction(transaction); + is(testRootNode.childCount, 2, "create test folder via copy"); + + // check GUIDs are different + var folderBNode = testRootNode.getChild(1); + ok(checkGUIDs(folderBNode, folderAGUIDs, false), "confirm folder A GUIDs don't match folder B GUIDs"); + var folderBGUIDs = getGUIDs(folderBNode); + ok(checkGUIDs(folderBNode, folderBGUIDs, true), "confirm test of new GUIDs"); + + // undo the transaction, confirm the removal + PlacesUIUtils.ptm.undoTransaction(); + is(testRootNode.childCount, 1, "confirm undo removed the copied folder"); + + // redo the transaction + // confirming GUIDs persist through undo/redo + PlacesUIUtils.ptm.redoTransaction(); + is(testRootNode.childCount, 2, "confirm redo re-copied the folder"); + folderBNode = testRootNode.getChild(1); + ok(checkGUIDs(folderBNode, folderAGUIDs, false), "folder B GUIDs after undo/redo don't match folder A GUIDs"); // sanity check + + // XXXdietrich fails since GUIDs are created lazily. the anno + // isn't present at the time the transaction is first executed, + // and undo just undoes the original transaction - doesn't pull + // in any new changes. + //ok(checkGUIDs(folderBNode, folderBGUIDs, true, true), "folder B GUIDs after under/redo should match pre-undo/redo folder B GUIDs"); + + // clean up + PlacesUIUtils.ptm.undoTransaction(); + PlacesUtils.bookmarks.removeItem(testRootId); +} + +function getGUIDs(aNode) { + asContainer(aNode); + aNode.containerOpen = true; + var GUIDs = { + folder: PlacesUtils.bookmarks.getItemGUID(aNode.itemId), + bookmark: PlacesUtils.bookmarks.getItemGUID(aNode.getChild(0).itemId), + separator: PlacesUtils.bookmarks.getItemGUID(aNode.getChild(1).itemId), + livemark: PlacesUtils.bookmarks.getItemGUID(aNode.getChild(2).itemId) + }; + aNode.containerOpen = false; + return GUIDs; +} + +function checkGUIDs(aFolderNode, aGUIDs, aShouldMatch) { + + function check(aNode, aGUID, aEquals) { + var nodeGUID = PlacesUtils.bookmarks.getItemGUID(aNode.itemId); + return aEquals ? (nodeGUID == aGUID) : (nodeGUID != aGUID); + } + + asContainer(aFolderNode); + aFolderNode.containerOpen = true; + + var allMatch = check(aFolderNode, aGUIDs.folder, aShouldMatch) && + check(aFolderNode.getChild(0), aGUIDs.bookmark, aShouldMatch) && + check(aFolderNode.getChild(1), aGUIDs.separator, aShouldMatch) && + check(aFolderNode.getChild(2), aGUIDs.livemark, aShouldMatch); + + aFolderNode.containerOpen = false; + return allMatch; +} diff --git a/mozilla/toolkit/components/places/src/utils.js b/mozilla/toolkit/components/places/src/utils.js index 9cac9959ca4..19f2541c9b9 100644 --- a/mozilla/toolkit/components/places/src/utils.js +++ b/mozilla/toolkit/components/places/src/utils.js @@ -1479,7 +1479,9 @@ var PlacesUtils = { serializeNodeToJSONStream(aNode, null); }, - // XXX testing serializers + /** + * Serialize a JS object to JSON + */ toJSONString: function PU_toJSONString(aObj) { var JSON = Cc["@mozilla.org/dom/json;1"].createInstance(Ci.nsIJSON); return JSON.encode(aObj);