From 8e01c479579fa8f5631e8e23f84aa86bb22d0be1 Mon Sep 17 00:00:00 2001 From: "mozilla.mano%sent.com" Date: Tue, 22 May 2007 22:03:55 +0000 Subject: [PATCH] Bug 371823 - Optimize item moves. r=sspitzer. --- browser/base/content/browser.js | 2 +- .../components/places/content/controller.js | 59 ++++--------------- .../places/content/moveBookmarks.js | 31 +--------- browser/components/places/content/utils.js | 48 +++++++-------- .../src/nsPlacesImportExportService.cpp | 2 +- .../places/public/nsINavBookmarksService.idl | 34 +++++------ .../components/places/src/nsNavBookmarks.cpp | 34 ++++++----- .../places/src/nsNavHistoryResult.cpp | 36 +++++------ .../places/tests/bookmarks/test_bookmarks.js | 39 +++++++----- .../places/tests/chrome/test_371798.xul | 8 +-- .../places/tests/chrome/test_add_livemark.xul | 6 +- 11 files changed, 119 insertions(+), 180 deletions(-) diff --git a/browser/base/content/browser.js b/browser/base/content/browser.js index ac1b2fc7f89..911e20084e3 100644 --- a/browser/base/content/browser.js +++ b/browser/base/content/browser.js @@ -339,7 +339,7 @@ var gBookmarksObserver = { }, onItemVisited: function() { }, - onFolderMoved: function() { }, + onItemMoved: function() { }, onFolderChanged: function() { } }; diff --git a/browser/components/places/content/controller.js b/browser/components/places/content/controller.js index 9de508bee1c..32e48b1ee9d 100755 --- a/browser/components/places/content/controller.js +++ b/browser/components/places/content/controller.js @@ -1761,66 +1761,29 @@ PlacesCreateLivemarkTransaction.prototype = { } }; -/** - * Move a Folder - */ -function PlacesMoveFolderTransaction(id, oldContainer, oldIndex, newContainer, newIndex) { - NS_ASSERT(!isNaN(id + oldContainer + oldIndex + newContainer + newIndex), "Parameter is NaN!"); - NS_ASSERT(newIndex >= -1, "invalid insertion index"); - this._id = id; - this._oldContainer = oldContainer; - this._oldIndex = oldIndex; - this._newContainer = newContainer; - this._newIndex = newIndex; - this.redoTransaction = this.doTransaction; -} -PlacesMoveFolderTransaction.prototype = { - __proto__: PlacesBaseTransaction.prototype, - - doTransaction: function PMFT_doTransaction() { - this.LOG("Move Folder: " + this._id + " from: " + this._oldContainer + "," + this._oldIndex + " to: " + this._newContainer + "," + this._newIndex); - this.bookmarks.moveFolder(this._id, this._newContainer, this._newIndex); - }, - - undoTransaction: function PMFT_undoTransaction() { - this.LOG("UNMove Folder: " + this._id + " from: " + this._oldContainer + "," + this._oldIndex + " to: " + this._newContainer + "," + this._newIndex); - this.bookmarks.moveFolder(this._id, this._oldContainer, this._oldIndex); - } -}; - /** * Move an Item */ -function PlacesMoveItemTransaction(id, uri, oldContainer, oldIndex, newContainer, newIndex) { - this._id = id; - this._uri = uri; - this._oldContainer = oldContainer; - this._oldIndex = oldIndex; - this._newContainer = newContainer; - this._newIndex = newIndex; +function PlacesMoveItemTransaction(aItemId, aNewContainer, aNewIndex) { + NS_ASSERT(!isNaN(aItemId + aNewContainer + aNewIndex), "Parameter is NaN!"); + NS_ASSERT(aNewIndex >= -1, "invalid insertion index"); + this._id = aItemId; + this._oldContainer = this.utils.bookmarks.getFolderIdForItem(this._id); + this._oldIndex = this.utils.bookmarks.getItemIndex(this._id); + NS_ASSERT(this._oldContainer > 0 && this._oldIndex >= 0, "invalid item"); + this._newContainer = aNewContainer; + this._newIndex = aNewIndex; this.redoTransaction = this.doTransaction; } PlacesMoveItemTransaction.prototype = { __proto__: PlacesBaseTransaction.prototype, doTransaction: function PMIT_doTransaction() { - this.LOG("Move Item: " + this._uri.spec + " from: " + this._oldContainer + "," + this._oldIndex + " to: " + this._newContainer + "," + this._newIndex); - var title = this.bookmarks.getItemTitle(this._id); - var annotations = this.utils.getAnnotationsForItem(this._id); - this.bookmarks.removeItem(this._id); - this._id = this.bookmarks.insertItem(this._newContainer, this._uri, this._newIndex); - this.bookmarks.setItemTitle(this._id, title); - this.utils.setAnnotationsForItem(this._id, annotations); + this.bookmarks.moveItem(this._id, this._newContainer, this._newIndex); }, undoTransaction: function PMIT_undoTransaction() { - this.LOG("UNMove Item: " + this._uri.spec + " from: " + this._oldContainer + "," + this._oldIndex + " to: " + this._newContainer + "," + this._newIndex); - var title = this.bookmarks.getItemTitle(this._id); - var annotations = this.utils.getAnnotationsForItem(this._id); - this.bookmarks.removeItem(this._id); - this._id = this.bookmarks.insertItem(this._oldContainer, this._uri, this._oldIndex); - this.bookmarks.setItemTitle(this._id, title); - this.utils.setAnnotationsForItem(this._id, annotations); + this.bookmarks.moveItem(this._id, this._oldContainer, this._oldIndex); } }; diff --git a/browser/components/places/content/moveBookmarks.js b/browser/components/places/content/moveBookmarks.js index d533d86bd43..421cfd3d53b 100644 --- a/browser/components/places/content/moveBookmarks.js +++ b/browser/components/places/content/moveBookmarks.js @@ -65,37 +65,12 @@ var gMoveBookmarksDialog = { var transactions = []; for (var i=0; i < this._nodes.length; i++) { - var parentId = this._nodes[i].parent.itemId; - // Nothing to do if the node is already under the selected folder - if (parentId == selectedFolderID) + if (this._nodes[i].parent.itemId == selectedFolderID) continue; - var nodeIndex = PlacesUtils.getIndexOfNode(this._nodes[i]); - if (PlacesUtils.nodeIsFolder(this._nodes[i])) { - // Disallow moving a folder into itself - if (this._nodes[i].itemId != selectedFolderID) { - transactions.push(new - PlacesMoveFolderTransaction(this._nodes[i].itemId, - parentId, nodeIndex, - selectedFolderID, -1)); - } - } - else if (PlacesUtils.nodeIsBookmark(this._nodes[i])) { - transactions.push(new - PlacesMoveItemTransaction(this._nodes[i].itemId, - PlacesUtils._uri(this._nodes[i].uri), - parentId, nodeIndex, selectedFolderID, -1)); - } - else if (PlacesUtils.nodeIsSeparator(this._nodes[i])) { - // See makeTransaction in utils.js - var removeTxn = - new PlacesRemoveSeparatorTransaction(parentId, nodeIndex); - var createTxn = - new PlacesCreateSeparatorTransaction(selectedFolderID, -1); - transactions.push(new - PlacesAggregateTransaction("SeparatorMove", [removeTxn, createTxn])); - } + transactions.push(new + PlacesMoveItemTransaction(this._nodes[i].itemId, selectedFolderID, -1)); } if (transactions.length != 0) { diff --git a/browser/components/places/content/utils.js b/browser/components/places/content/utils.js index 258acbc2e61..2a6e327f589 100644 --- a/browser/components/places/content/utils.js +++ b/browser/components/places/content/utils.js @@ -450,7 +450,7 @@ var PlacesUtils = { // bookmarks folder: \n<>\n\n // uri: 0\n\n\n // bookmark: \n\n\n - // separator: 0\n<>\n\n + // separator: \n<>\n\n var wrapped = ""; if (aNode.itemId != -1) // wrapped += aNode.itemId + NEWLINE; @@ -655,50 +655,46 @@ var PlacesUtils = { index, copy) { switch (type) { case this.TYPE_X_MOZ_PLACE_CONTAINER: - if (data.id > 0 && data.uri == null) { + if (data.id > 0) { // Place is a folder. if (copy) return this._getFolderCopyTransaction(data, container, index); - return new PlacesMoveFolderTransaction(data.id, data.parent, - data.index, container, - index); } + break; case this.TYPE_X_MOZ_PLACE: - if (data.id > 0) { - 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.id, container, index, - ["livemark/bookmarkFeedURI"]); - return copyBookmarkAnno; - } - return new PlacesMoveItemTransaction(data.id, data.uri, data.parent, - data.index, container, - index); + if (data.id <= 0) + return this._getURIItemCopyTransaction(data.uri, container, index); + + 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.id, container, index, + ["livemark/bookmarkFeedURI"]); + return copyBookmarkAnno; } - return this._getURIItemCopyTransaction(data.uri, container, index); + break; case this.TYPE_X_MOZ_PLACE_SEPARATOR: if (copy) { // There is no data in a separator, so copying it just amounts to // inserting a new separator. return new PlacesCreateSeparatorTransaction(container, index); } - // Similarly, moving a separator is just removing the old one and - // then creating a new one. - var removeTxn = - new PlacesRemoveSeparatorTransaction(data.parent, data.index); - var createTxn = - new PlacesCreateSeparatorTransaction(container, index); - return new PlacesAggregateTransaction("SeparatorMove", [removeTxn, createTxn]); + break; case this.TYPE_X_MOZ_URL: case this.TYPE_UNICODE: var title = type == this.TYPE_X_MOZ_URL ? data.title : data.uri; var createTxn = new PlacesCreateItemTransaction(data.uri, container, index, title); return createTxn; + default: + return null; } - return null; + if (data.id <= 0) + return null; + + // Move the item otherwise + return new PlacesMoveItemTransaction(data.id, container, index); }, /** diff --git a/browser/components/places/src/nsPlacesImportExportService.cpp b/browser/components/places/src/nsPlacesImportExportService.cpp index 2d3f85452da..486c07d98ae 100644 --- a/browser/components/places/src/nsPlacesImportExportService.cpp +++ b/browser/components/places/src/nsPlacesImportExportService.cpp @@ -1219,7 +1219,7 @@ BookmarkContentSink::NewFrame() if (updateFolder) { // move the menu folder to the current position - mBookmarksService->MoveFolder(ourID, CurFrame().mContainerID, -1); + mBookmarksService->MoveItem(ourID, CurFrame().mContainerID, -1); mBookmarksService->SetItemTitle(ourID, containerName); #ifdef DEBUG_IMPORT printf(" [reparenting]"); diff --git a/toolkit/components/places/public/nsINavBookmarksService.idl b/toolkit/components/places/public/nsINavBookmarksService.idl index 4bb042a9387..163e37758a7 100644 --- a/toolkit/components/places/public/nsINavBookmarksService.idl +++ b/toolkit/components/places/public/nsINavBookmarksService.idl @@ -54,7 +54,7 @@ interface nsITransaction; * Observer for bookmark changes. */ -[scriptable, uuid(7A553BA3-5381-4A66-95D9-C7BA3895603C)] +[scriptable, uuid(73d28ab9-0360-4077-845e-543d1aedba1d)] interface nsINavBookmarkObserver : nsISupports { /** @@ -137,21 +137,21 @@ interface nsINavBookmarkObserver : nsISupports in PRTime time); /** - * Notify this observer that a bookmark folder has been moved. - * @param aFolder - * The id of the folder that was moved. + * Notify this observer that an item has been moved. + * @param aItemId + * The id of the item that was moved. * @param aOldParent - * The id of the folder's old parent. + * The id of the old parent. * @param aOldIndex - * The folder's old index inside oldParent. + * The old index inside aOldParent. * @param aNewParent - * The id of the folder's new parent. + * The id of the new parent. * @param aNewIndex - * The folder's index inside newParent. + * The foindex inside aNewParent. */ - void onFolderMoved(in long long aFolder, - in long long aOldParent, in long aOldIndex, - in long long aNewParent, in long aNewIndex); + void onItemMoved(in long long aItemId, + in long long aOldParent, in long aOldIndex, + in long long aNewParent, in long aNewIndex); /** * Notify this observer that a bookmark folder's information has changed. @@ -293,15 +293,15 @@ interface nsINavBookmarksService : nsISupports void removeFolderChildren(in long long aFolder); /** - * Moves a folder to a different container, preserving its contents. - * @param aFolder - * The folder to move + * Moves an item to a different container, preserving its contents. + * @param aItemId + * The id of the item to move * @param aNewParent - * The id of the folder's new parent + * The id of the new parent * @param aIndex - * The folder's index under newParent, or -1 to append + * The index under aNewParent, or -1 to append */ - void moveFolder(in long long aFolder, in long long newParent, in long aIndex); + void moveItem(in long long aFolder, in long long newParent, in long aIndex); /** * Returns the ID of a child folder with the given name. This does not diff --git a/toolkit/components/places/src/nsNavBookmarks.cpp b/toolkit/components/places/src/nsNavBookmarks.cpp index 32e1733b453..d020b222eaa 100644 --- a/toolkit/components/places/src/nsNavBookmarks.cpp +++ b/toolkit/components/places/src/nsNavBookmarks.cpp @@ -1353,14 +1353,14 @@ nsNavBookmarks::RemoveFolderChildren(PRInt64 aFolder) } NS_IMETHODIMP -nsNavBookmarks::MoveFolder(PRInt64 aFolder, PRInt64 aNewParent, PRInt32 aIndex) +nsNavBookmarks::MoveItem(PRInt64 aItemId, PRInt64 aNewParent, PRInt32 aIndex) { // You can pass -1 to indicate append, but no other negative number is allowed if (aIndex < -1) return NS_ERROR_INVALID_ARG; // Disallow making a folder it's own parent. - if (aFolder == aNewParent) + if (aItemId == aNewParent) return NS_ERROR_INVALID_ARG; mozIStorageConnection *dbConn = DBConn(); @@ -1370,10 +1370,11 @@ nsNavBookmarks::MoveFolder(PRInt64 aFolder, PRInt64 aNewParent, PRInt32 aIndex) nsresult rv; PRInt64 oldParent; PRInt32 oldIndex; - nsCAutoString type; + PRInt32 itemType; + nsCAutoString folderType; { mozStorageStatementScoper scope(mDBGetItemProperties); - rv = mDBGetItemProperties->BindInt64Parameter(0, aFolder); + rv = mDBGetItemProperties->BindInt64Parameter(0, aItemId); NS_ENSURE_SUCCESS(rv, rv); PRBool results; @@ -1385,8 +1386,12 @@ nsNavBookmarks::MoveFolder(PRInt64 aFolder, PRInt64 aNewParent, PRInt32 aIndex) oldParent = mDBGetItemProperties->AsInt64(kGetItemPropertiesIndex_Parent); oldIndex = mDBGetItemProperties->AsInt32(kGetItemPropertiesIndex_Position); - rv = mDBGetItemProperties->GetUTF8String(kGetItemPropertiesIndex_FolderType, type); - NS_ENSURE_SUCCESS(rv, rv); + itemType = mDBGetItemProperties->AsInt32(kGetItemPropertiesIndex_Type); + if (itemType == TYPE_FOLDER) { + rv = mDBGetItemProperties->GetUTF8String(kGetItemPropertiesIndex_FolderType, + folderType); + NS_ENSURE_SUCCESS(rv, rv); + } } // if parent and index are the same, nothing to do @@ -1394,12 +1399,12 @@ nsNavBookmarks::MoveFolder(PRInt64 aFolder, PRInt64 aNewParent, PRInt32 aIndex) return NS_OK; // Make sure aNewParent is not aFolder or a subfolder of aFolder - { + if (itemType == TYPE_FOLDER) { mozStorageStatementScoper scope(mDBGetItemProperties); PRInt64 p = aNewParent; while (p) { - if (p == aFolder) { + if (p == aItemId) { return NS_ERROR_INVALID_ARG; } @@ -1455,7 +1460,7 @@ nsNavBookmarks::MoveFolder(PRInt64 aFolder, PRInt64 aNewParent, PRInt32 aIndex) buffer.AppendInt(newIndex); } buffer.AppendLiteral(" WHERE id = "); - buffer.AppendInt(aFolder); + buffer.AppendInt(aItemId); rv = dbConn->ExecuteSimpleSQL(buffer); NS_ENSURE_SUCCESS(rv, rv); @@ -1484,14 +1489,15 @@ nsNavBookmarks::MoveFolder(PRInt64 aFolder, PRInt64 aNewParent, PRInt32 aIndex) // notify bookmark observers ENUMERATE_WEAKARRAY(mObservers, nsINavBookmarkObserver, - OnFolderMoved(aFolder, oldParent, oldIndex, - aNewParent, newIndex)) + OnItemMoved(aItemId, oldParent, oldIndex, aNewParent, + newIndex)) // notify remote container provider if there is one - if (!type.IsEmpty()) { - nsCOMPtr container = do_GetService(type.get(), &rv); + if (!folderType.IsEmpty()) { + nsCOMPtr container = + do_GetService(folderType.get(), &rv); if (NS_SUCCEEDED(rv)) { - rv = container->OnContainerMoved(aFolder, aNewParent, newIndex); + rv = container->OnContainerMoved(aItemId, aNewParent, newIndex); NS_ENSURE_SUCCESS(rv, rv); } } diff --git a/toolkit/components/places/src/nsNavHistoryResult.cpp b/toolkit/components/places/src/nsNavHistoryResult.cpp index 5187a41a4a0..470099654fa 100644 --- a/toolkit/components/places/src/nsNavHistoryResult.cpp +++ b/toolkit/components/places/src/nsNavHistoryResult.cpp @@ -2595,7 +2595,7 @@ nsNavHistoryQueryResultNode::OnItemVisited(PRInt64 aItemId, return NS_OK; } NS_IMETHODIMP -nsNavHistoryQueryResultNode::OnFolderMoved(PRInt64 aFolder, PRInt64 aOldParent, +nsNavHistoryQueryResultNode::OnItemMoved(PRInt64 aFolder, PRInt64 aOldParent, PRInt32 aOldIndex, PRInt64 aNewParent, PRInt32 aNewIndex) { @@ -3294,12 +3294,12 @@ nsNavHistoryFolderResultNode::OnItemVisited(PRInt64 aItemId, return NS_OK; } -// nsNavHistoryFolderResultNode::OnFolderMoved (nsINavBookmarkObserver) +// nsNavHistoryFolderResultNode::OnItemMoved (nsINavBookmarkObserver) NS_IMETHODIMP -nsNavHistoryFolderResultNode::OnFolderMoved(PRInt64 aFolder, PRInt64 aOldParent, - PRInt32 aOldIndex, PRInt64 aNewParent, - PRInt32 aNewIndex) +nsNavHistoryFolderResultNode::OnItemMoved(PRInt64 aItemId, PRInt64 aOldParent, + PRInt32 aOldIndex, PRInt64 aNewParent, + PRInt32 aNewIndex) { NS_ASSERTION(aOldParent == mItemId || aNewParent == mItemId, "Got a bookmark message that doesn't belong to us"); @@ -3315,8 +3315,8 @@ nsNavHistoryFolderResultNode::OnFolderMoved(PRInt64 aFolder, PRInt64 aOldParent, ReindexRange(aNewIndex, PR_INT32_MAX, 1); PRUint32 index; - nsNavHistoryFolderResultNode* node = FindChildFolder(aFolder, &index); - if (! node) { + nsNavHistoryResultNode* node = FindChildById(aItemId, &index); + if (!node) { NS_NOTREACHED("Can't find folder that is moving!"); return NS_ERROR_FAILURE; } @@ -3332,7 +3332,7 @@ nsNavHistoryFolderResultNode::OnFolderMoved(PRInt64 aFolder, PRInt64 aOldParent, if (DoesChildNeedResorting(index, comparator, sortingAnnotation.get())) { // needs resorting, this will cause everything to be redrawn, so we // don't need to do that explicitly later. - nsRefPtr lock(node); + nsRefPtr lock(node); RemoveChildAt(index, PR_TRUE); InsertChildAt(node, FindInsertionPoint(node, comparator, sortingAnnotation.get()), @@ -3343,9 +3343,9 @@ nsNavHistoryFolderResultNode::OnFolderMoved(PRInt64 aFolder, PRInt64 aOldParent, } else { // moving between two different folders, just do a remove and an add if (aOldParent == mItemId) - OnItemRemoved(aFolder, aOldParent, aOldIndex); + OnItemRemoved(aItemId, aOldParent, aOldIndex); if (aNewParent == mItemId) - OnItemAdded(aFolder, aNewParent, aNewIndex); + OnItemAdded(aItemId, aNewParent, aNewIndex); } return NS_OK; } @@ -3854,26 +3854,26 @@ nsNavHistoryResult::OnItemVisited(PRInt64 aItemId, PRInt64 aVisitId, } -// nsNavHistoryResult::OnFolderMoved (nsINavBookmarkObserver) +// nsNavHistoryResult::OnItemMoved (nsINavBookmarkObserver) // // Need to notify both the source and the destination folders (if they // are different). NS_IMETHODIMP -nsNavHistoryResult::OnFolderMoved(PRInt64 aFolder, - PRInt64 aOldParent, PRInt32 aOldIndex, - PRInt64 aNewParent, PRInt32 aNewIndex) +nsNavHistoryResult::OnItemMoved(PRInt64 aItemId, + PRInt64 aOldParent, PRInt32 aOldIndex, + PRInt64 aNewParent, PRInt32 aNewIndex) { { // scope for loop index for VC6's broken for loop scoping ENUMERATE_BOOKMARK_OBSERVERS_FOR_FOLDER(aOldParent, - OnFolderMoved(aFolder, aOldParent, aOldIndex, aNewParent, aNewIndex)); + OnItemMoved(aItemId, aOldParent, aOldIndex, aNewParent, aNewIndex)); } if (aNewParent != aOldParent) { ENUMERATE_BOOKMARK_OBSERVERS_FOR_FOLDER(aNewParent, - OnFolderMoved(aFolder, aOldParent, aOldIndex, aNewParent, aNewIndex)); + OnItemMoved(aItemId, aOldParent, aOldIndex, aNewParent, aNewIndex)); } - ENUMERATE_HISTORY_OBSERVERS(OnFolderMoved(aFolder, aOldParent, aOldIndex, - aNewParent, aNewIndex)); + ENUMERATE_HISTORY_OBSERVERS(OnItemMoved(aItemId, aOldParent, aOldIndex, + aNewParent, aNewIndex)); return NS_OK; } diff --git a/toolkit/components/places/tests/bookmarks/test_bookmarks.js b/toolkit/components/places/tests/bookmarks/test_bookmarks.js index d67ca962bc8..aeef90761da 100644 --- a/toolkit/components/places/tests/bookmarks/test_bookmarks.js +++ b/toolkit/components/places/tests/bookmarks/test_bookmarks.js @@ -87,12 +87,12 @@ var observer = { this._itemVisitedVistId = visitID; this._itemVisitedTime = time; }, - onFolderMoved: function(folder, oldParent, oldIndex, newParent, newIndex) { - this._folderMoved = folder; - this._folderMovedOldParent = oldParent; - this._folderMovedOldIndex = oldIndex; - this._folderMovedNewParent = newParent; - this._folderMovedNewIndex = newIndex; + onItemMoved: function(id, oldParent, oldIndex, newParent, newIndex) { + this._itemMovedId = id + this._itemMovedOldParent = oldParent; + this._itemMovedOldIndex = oldIndex; + this._itemMovedNewParent = newParent; + this._itemMovedNewIndex = newIndex; }, onFolderChanged: function(folder, property) { this._folderChanged = folder; @@ -271,12 +271,12 @@ function run_test() { // move folder, appending, to different folder var oldParentCC = getChildCount(testRoot); - bmsvc.moveFolder(workFolder, homeFolder, bmsvc.DEFAULT_INDEX); - do_check_eq(observer._folderMoved, workFolder); - do_check_eq(observer._folderMovedOldParent, testRoot); - do_check_eq(observer._folderMovedOldIndex, 0); - do_check_eq(observer._folderMovedNewParent, homeFolder); - do_check_eq(observer._folderMovedNewIndex, 1); + bmsvc.moveItem(workFolder, homeFolder, bmsvc.DEFAULT_INDEX); + do_check_eq(observer._itemMovedId, workFolder); + do_check_eq(observer._itemMovedOldParent, testRoot); + do_check_eq(observer._itemMovedOldIndex, 0); + do_check_eq(observer._itemMovedNewParent, homeFolder); + do_check_eq(observer._itemMovedNewIndex, 1); // test that the new index is properly stored do_check_eq(bmsvc.getItemIndex(workFolder), 1); @@ -292,7 +292,14 @@ function run_test() { // XXX move folder, specify same index, within the same folder // XXX move folder, appending, within the same folder - // XXX move item, appending, to different folder + // move item, appending, to different folder + bmsvc.moveItem(newId5, testRoot, bmsvc.DEFAULT_INDEX); + do_check_eq(observer._itemMovedId, newId5); + do_check_eq(observer._itemMovedOldParent, homeFolder); + do_check_eq(observer._itemMovedOldIndex, 0); + do_check_eq(observer._itemMovedNewParent, testRoot); + do_check_eq(observer._itemMovedNewIndex, 3); + // XXX move item, specified index, to different folder // XXX move item, specified index, within the same folder // XXX move item, specify same index, within the same folder @@ -300,8 +307,8 @@ function run_test() { // Test expected failure of moving a folder to be its own parent try { - bmsvc.moveFolder(workFolder, workFolder, bmsvc.DEFAULT_INDEX); - do_throw("moveFolder() allowed moving a folder to be it's own parent."); + bmsvc.moveItem(workFolder, workFolder, bmsvc.DEFAULT_INDEX); + do_throw("moveItem() allowed moving a folder to be it's own parent."); } catch (e) {} // test insertSeparator and removeChildAt @@ -508,7 +515,7 @@ function run_test() { var newId13 = bmsvc.insertItem(testRoot, uri("http://foobarcheese.com/"), bmsvc.DEFAULT_INDEX); do_check_eq(observer._itemAddedId, newId13); do_check_eq(observer._itemAddedParent, testRoot); - do_check_eq(observer._itemAddedIndex, 12); + do_check_eq(observer._itemAddedIndex, 13); // set bookmark title bmsvc.setItemTitle(newId13, "ZZZXXXYYY"); diff --git a/toolkit/components/places/tests/chrome/test_371798.xul b/toolkit/components/places/tests/chrome/test_371798.xul index 0105c4699a6..1ce9b9f2eae 100644 --- a/toolkit/components/places/tests/chrome/test_371798.xul +++ b/toolkit/components/places/tests/chrome/test_371798.xul @@ -67,12 +67,8 @@ var observer = bmsvc.removeObserver(this); }, onItemVisited: function(bookmarkId, bookmark, aVisitID, time){}, - onFolderAdded: function(folder, parent, index){}, - onFolderRemoved: function(folder, parent, index){}, - onFolderMoved: function(folder, oldParent, oldIndex, newParent, newIndex){}, - onFolderChanged: function(folder, property){}, - onSeparatorAdded: function(parent, index){}, - onSeparatorRemoved: function(parent, index){} + onItemMoved: function(itemId, oldParent, oldIndex, newParent, newIndex){}, + onFolderChanged: function(folder, property){} }; bmsvc.addObserver(observer, false); diff --git a/toolkit/components/places/tests/chrome/test_add_livemark.xul b/toolkit/components/places/tests/chrome/test_add_livemark.xul index 6bd5cff79fa..7077e9fc8a5 100644 --- a/toolkit/components/places/tests/chrome/test_add_livemark.xul +++ b/toolkit/components/places/tests/chrome/test_add_livemark.xul @@ -59,12 +59,8 @@ var observer = onItemRemoved: function(itemId, folder, index){}, onItemChanged: function(itemId, property, isAnnotationProperty, value){}, onItemVisited: function(itemId, aVisitID, time){}, - onFolderAdded: function(folder, parent, index){}, - onFolderRemoved: function(folder, parent, index){}, - onFolderMoved: function(folder, oldParent, oldIndex, newParent, newIndex){}, + onItemMoved: function(itemId, oldParent, oldIndex, newParent, newIndex){}, onFolderChanged: function(folder, property){}, - onSeparatorAdded: function(parent, index){}, - onSeparatorRemoved: function(parent, index){}, // nsIAnnotationObserver onItemAnnotationSet: function(aItemId, aAnnotationName) {