Bug 430442 - Undo and redo in the library are sometimes not correctly working, r=dietrich

This commit is contained in:
Marco Bonardo 2008-09-11 16:16:54 +02:00
Родитель cbd05b450c
Коммит a5eab6695b
5 изменённых файлов: 217 добавлений и 41 удалений

Просмотреть файл

@ -863,11 +863,13 @@ PlacesController.prototype = {
* An array of nodes to remove. Should all be adjacent.
* @param [out] transactions
* An array of transactions.
* @param [optional] removedFolders
* An array of folder nodes that have already been removed.
*/
_removeRange: function PC__removeRange(range, transactions) {
_removeRange: function PC__removeRange(range, transactions, removedFolders) {
NS_ASSERT(transactions instanceof Array, "Must pass a transactions array");
var removedFolders = [];
if (!removedFolders)
removedFolders = [];
for (var i = 0; i < range.length; ++i) {
var node = range[i];
@ -906,10 +908,11 @@ PlacesController.prototype = {
_removeRowsFromBookmarks: function PC__removeRowsFromBookmarks(txnName) {
var ranges = this._view.getRemovableSelectionRanges();
var transactions = [];
// Delete the selected rows. Do this by walking the selection backward, so
// that when undo is performed they are re-inserted in the correct order.
for (var i = ranges.length - 1; i >= 0 ; --i)
this._removeRange(ranges[i], transactions);
var removedFolders = [];
for (var i = 0; i < ranges.length; i++)
this._removeRange(ranges[i], transactions, removedFolders);
if (transactions.length > 0) {
var txn = PlacesUIUtils.ptm.aggregateTransactions(txnName, transactions);
PlacesUIUtils.ptm.doTransaction(txn);

Просмотреть файл

@ -296,8 +296,11 @@ placesAggregateTransactions.prototype = {
},
commit: function PAT_commit(aUndo) {
for (var i=0; i < this._transactions.length; ++i) {
var txn = this._transactions[i];
var transactions = this._transactions;
if (aUndo)
transactions.reverse();
for (var i = 0; i < transactions.length; i++) {
var txn = transactions[i];
if (this.container > -1)
txn.wrappedJSObject.container = this.container;
if (aUndo)
@ -397,6 +400,7 @@ function placesCreateSeparatorTransactions(aContainer, aIndex) {
this._container = aContainer;
this._index = typeof(aIndex) == "number" ? aIndex : -1;
this._id = null;
this.redoTransaction = this.doTransaction;
}
placesCreateSeparatorTransactions.prototype = {
@ -450,7 +454,6 @@ placesCreateLivemarkTransactions.prototype = {
function placesMoveItemTransactions(aItemId, aNewContainer, aNewIndex) {
this._id = aItemId;
this._oldContainer = PlacesUtils.bookmarks.getFolderIdForItem(this._id);
this._oldIndex = PlacesUtils.bookmarks.getItemIndex(this._id);
this._newContainer = aNewContainer;
this._newIndex = aNewIndex;
this.redoTransaction = this.doTransaction;
@ -460,17 +463,16 @@ placesMoveItemTransactions.prototype = {
__proto__: placesBaseTransaction.prototype,
doTransaction: function PMIT_doTransaction() {
this._oldIndex = PlacesUtils.bookmarks.getItemIndex(this._id);
PlacesUtils.bookmarks.moveItem(this._id, this._newContainer, this._newIndex);
// if newIndex == DEFAULT_INDEX we append, so get correct index for undo
if (this._newIndex == PlacesUtils.bookmarks.DEFAULT_INDEX)
this._newIndex = PlacesUtils.bookmarks.getItemIndex(this._id);
this._undoIndex = PlacesUtils.bookmarks.getItemIndex(this._id);
},
undoTransaction: function PMIT_undoTransaction() {
// moving down in the same container takes in count removal of the item
// so to revert positions we must move to oldIndex + 1
if (this._newContainer == this._oldContainer &&
this._oldIndex > this._newIndex)
this._oldIndex > this._undoIndex)
PlacesUtils.bookmarks.moveItem(this._id, this._oldContainer, this._oldIndex + 1);
else
PlacesUtils.bookmarks.moveItem(this._id, this._oldContainer, this._oldIndex);
@ -541,7 +543,7 @@ placesRemoveItemTransaction.prototype = {
this._transactions[i].undoTransaction();
}
else // TYPE_SEPARATOR
PlacesUtils.bookmarks.insertSeparator(this._oldContainer, this._oldIndex);
this._id = PlacesUtils.bookmarks.insertSeparator(this._oldContainer, this._oldIndex);
if (this._annotations.length > 0)
PlacesUtils.setAnnotationsForItem(this._id, this._annotations);

Просмотреть файл

@ -151,6 +151,14 @@ function run_test() {
do_check_eq(observer._itemRemovedId, folderId);
do_check_eq(observer._itemRemovedFolder, root);
do_check_eq(observer._itemRemovedIndex, bmStartIndex);
txn1.redoTransaction();
do_check_eq(observer._itemAddedIndex, bmStartIndex);
do_check_eq(observer._itemAddedParent, root);
do_check_eq(observer._itemAddedId, folderId);
txn1.undoTransaction();
do_check_eq(observer._itemRemovedId, folderId);
do_check_eq(observer._itemRemovedFolder, root);
do_check_eq(observer._itemRemovedIndex, bmStartIndex);
// Test creating an item
// Create to Root
@ -163,8 +171,19 @@ function run_test() {
txn2.undoTransaction();
do_check_eq(observer._itemRemovedId, b);
do_check_eq(observer._itemRemovedIndex, bmStartIndex);
do_check_false(bmsvc.isBookmarked(uri("http://www.example.com")));
txn2.redoTransaction();
do_check_true(bmsvc.isBookmarked(uri("http://www.example.com")));
var newId = (bmsvc.getBookmarkIdsForURI(uri("http://www.example.com"), {}))[0];
do_check_eq(observer._itemAddedIndex, bmStartIndex);
do_check_eq(observer._itemAddedParent, root);
do_check_eq(observer._itemAddedId, newId);
txn2.undoTransaction();
do_check_eq(observer._itemRemovedId, newId);
do_check_eq(observer._itemRemovedFolder, root);
do_check_eq(observer._itemRemovedIndex, bmStartIndex);
// Create to a folder
// Create item to a folder
var txn2a = ptSvc.createFolder("Folder", root, bmStartIndex);
ptSvc.doTransaction(txn2a);
var fldrId = bmsvc.getChildFolder(root, "Folder");
@ -177,10 +196,19 @@ function run_test() {
txn2b.undoTransaction();
do_check_eq(observer._itemRemovedId, b2);
do_check_eq(observer._itemRemovedIndex, bmStartIndex);
txn2b.redoTransaction();
newId = (bmsvc.getBookmarkIdsForURI(uri("http://www.example2.com"), {}))[0];
do_check_eq(observer._itemAddedIndex, bmStartIndex);
do_check_eq(observer._itemAddedParent, fldrId);
do_check_eq(observer._itemAddedId, newId);
txn2b.undoTransaction();
do_check_eq(observer._itemRemovedId, newId);
do_check_eq(observer._itemRemovedFolder, fldrId);
do_check_eq(observer._itemRemovedIndex, bmStartIndex);
// Testing moving an item
ptSvc.doTransaction(ptSvc.createItem(uri("http://www.example3.com"), root, -1, "Testing2"));
ptSvc.doTransaction(ptSvc.createItem(uri("http://www.example3.com"), root, -1, "Testing3"));
ptSvc.doTransaction(ptSvc.createItem(uri("http://www.example3.com"), root, -1, "Testing3"));
ptSvc.doTransaction(ptSvc.createItem(uri("http://www.example3.com"), fldrId, -1, "Testing4"));
var bkmkIds = bmsvc.getBookmarkIdsForURI(uri("http://www.example3.com"), {});
bkmkIds.sort();
@ -202,6 +230,18 @@ function run_test() {
do_check_eq(observer._itemMovedOldIndex, 2);
do_check_eq(observer._itemMovedNewParent, root);
do_check_eq(observer._itemMovedNewIndex, 1);
txn3.redoTransaction();
do_check_eq(observer._itemMovedId, bkmk1Id);
do_check_eq(observer._itemMovedOldParent, root);
do_check_eq(observer._itemMovedOldIndex, 1);
do_check_eq(observer._itemMovedNewParent, root);
do_check_eq(observer._itemMovedNewIndex, 2);
txn3.undoTransaction();
do_check_eq(observer._itemMovedId, bkmk1Id);
do_check_eq(observer._itemMovedOldParent, root);
do_check_eq(observer._itemMovedOldIndex, 2);
do_check_eq(observer._itemMovedNewParent, root);
do_check_eq(observer._itemMovedNewIndex, 1);
// Moving items between different folders
var txn3b = ptSvc.moveItem(bkmk1Id, fldrId, -1);
@ -217,6 +257,18 @@ function run_test() {
do_check_eq(observer._itemMovedOldIndex, 1);
do_check_eq(observer._itemMovedNewParent, root);
do_check_eq(observer._itemMovedNewIndex, 1);
txn3b.redoTransaction();
do_check_eq(observer._itemMovedId, bkmk1Id);
do_check_eq(observer._itemMovedOldParent, root);
do_check_eq(observer._itemMovedOldIndex, 1);
do_check_eq(observer._itemMovedNewParent, fldrId);
do_check_eq(observer._itemMovedNewIndex, 1);
txn3.undoTransaction();
do_check_eq(observer._itemMovedId, bkmk1Id);
do_check_eq(observer._itemMovedOldParent, fldrId);
do_check_eq(observer._itemMovedOldIndex, 1);
do_check_eq(observer._itemMovedNewParent, root);
do_check_eq(observer._itemMovedNewIndex, 1);
// Test Removing a Folder
ptSvc.doTransaction(ptSvc.createFolder("Folder2", root, -1));
@ -230,6 +282,14 @@ function run_test() {
do_check_eq(observer._itemAddedId, fldrId2);
do_check_eq(observer._itemAddedParent, root);
do_check_eq(observer._itemAddedIndex, 3);
txn4.redoTransaction();
do_check_eq(observer._itemRemovedId, fldrId2);
do_check_eq(observer._itemRemovedFolder, root);
do_check_eq(observer._itemRemovedIndex, 3);
txn4.undoTransaction();
do_check_eq(observer._itemAddedId, fldrId2);
do_check_eq(observer._itemAddedParent, root);
do_check_eq(observer._itemAddedIndex, 3);
// Test removing an item
var txn5 = ptSvc.removeItem(bkmk2Id);
@ -238,6 +298,14 @@ function run_test() {
do_check_eq(observer._itemRemovedFolder, root);
do_check_eq(observer._itemRemovedIndex, 2);
txn5.undoTransaction();
var newbkmk2Id = observer._itemAddedId;
do_check_eq(observer._itemAddedParent, root);
do_check_eq(observer._itemAddedIndex, 2);
txn5.redoTransaction();
do_check_eq(observer._itemRemovedId, newbkmk2Id);
do_check_eq(observer._itemRemovedFolder, root);
do_check_eq(observer._itemRemovedIndex, 2);
txn5.undoTransaction();
do_check_eq(observer._itemAddedParent, root);
do_check_eq(observer._itemAddedIndex, 2);
@ -251,6 +319,14 @@ function run_test() {
do_check_eq(observer._itemRemovedId, sepId);
do_check_eq(observer._itemRemovedFolder, root);
do_check_eq(observer._itemRemovedIndex, 1);
txn6.redoTransaction();
var newSepId = observer._itemAddedId;
do_check_eq(observer._itemAddedIndex, 1);
do_check_eq(observer._itemAddedParent, root);
txn6.undoTransaction();
do_check_eq(observer._itemRemovedId, newSepId);
do_check_eq(observer._itemRemovedFolder, root);
do_check_eq(observer._itemRemovedIndex, 1);
// Test removing a separator
ptSvc.doTransaction(ptSvc.createSeparator(root, 1));
@ -264,6 +340,14 @@ function run_test() {
do_check_eq(observer._itemAddedId, sepId2); //New separator created
do_check_eq(observer._itemAddedParent, root);
do_check_eq(observer._itemAddedIndex, 1);
txn7.redoTransaction();
do_check_eq(observer._itemRemovedId, sepId2);
do_check_eq(observer._itemRemovedFolder, root);
do_check_eq(observer._itemRemovedIndex, 1);
txn7.undoTransaction();
do_check_eq(observer._itemAddedId, sepId2); //New separator created
do_check_eq(observer._itemAddedParent, root);
do_check_eq(observer._itemAddedIndex, 1);
// Test editing item title
var txn8 = ptSvc.editItemTitle(bkmk1Id, "Testing2_mod");
@ -275,6 +359,14 @@ function run_test() {
do_check_eq(observer._itemChangedId, bkmk1Id);
do_check_eq(observer._itemChangedProperty, "title");
do_check_eq(observer._itemChangedValue, "Testing2");
txn8.redoTransaction();
do_check_eq(observer._itemChangedId, bkmk1Id);
do_check_eq(observer._itemChangedProperty, "title");
do_check_eq(observer._itemChangedValue, "Testing2_mod");
txn8.undoTransaction();
do_check_eq(observer._itemChangedId, bkmk1Id);
do_check_eq(observer._itemChangedProperty, "title");
do_check_eq(observer._itemChangedValue, "Testing2");
// Test editing item uri
var txn9 = ptSvc.editBookmarkURI(bkmk1Id, uri("http://newuri.com"));
@ -286,6 +378,14 @@ function run_test() {
do_check_eq(observer._itemChangedId, bkmk1Id);
do_check_eq(observer._itemChangedProperty, "uri");
do_check_eq(observer._itemChangedValue, "http://www.example3.com/");
txn9.redoTransaction();
do_check_eq(observer._itemChangedId, bkmk1Id);
do_check_eq(observer._itemChangedProperty, "uri");
do_check_eq(observer._itemChangedValue, "http://newuri.com/");
txn9.undoTransaction();
do_check_eq(observer._itemChangedId, bkmk1Id);
do_check_eq(observer._itemChangedProperty, "uri");
do_check_eq(observer._itemChangedValue, "http://www.example3.com/");
// Test edit item description
var txn10 = ptSvc.editItemDescription(bkmk1Id, "Description1");
@ -371,6 +471,14 @@ function run_test() {
do_check_eq(0, bmsvc.getItemIndex(b1));
do_check_eq(1, bmsvc.getItemIndex(b2));
do_check_eq(2, bmsvc.getItemIndex(b3));
txn17.redoTransaction();
do_check_eq(2, bmsvc.getItemIndex(b1));
do_check_eq(1, bmsvc.getItemIndex(b2));
do_check_eq(0, bmsvc.getItemIndex(b3));
txn17.undoTransaction();
do_check_eq(0, bmsvc.getItemIndex(b1));
do_check_eq(1, bmsvc.getItemIndex(b2));
do_check_eq(2, bmsvc.getItemIndex(b3));
// editBookmarkMicrosummary
var tmpMs = mss.createMicrosummary(uri("http://testmicro.com"),
@ -436,4 +544,76 @@ function run_test() {
do_check_eq(uneval(tagssvc.getTagsForURI(tagURI, { })), uneval(["bar","foo"]));
untagTxn.redoTransaction();
do_check_eq(uneval(tagssvc.getTagsForURI(tagURI, { })), uneval(["foo"]));
// Test aggregate removeItem transaction
var bkmk1Id = bmsvc.insertBookmark(root, uri("http://www.mozilla.org/"), 0, "Mozilla");
var bkmk2Id = bmsvc.insertSeparator(root, 1);
var bkmk3Id = bmsvc.createFolder(root, "folder", 2);
var bkmk3_1Id = bmsvc.insertBookmark(bkmk3Id, uri("http://www.mozilla.org/"), 0, "Mozilla");
var bkmk3_2Id = bmsvc.insertSeparator(bkmk3Id, 1);
var bkmk3_3Id = bmsvc.createFolder(bkmk3Id, "folder", 2);
var transactions = [];
transactions.push(ptSvc.removeItem(bkmk1Id));
transactions.push(ptSvc.removeItem(bkmk2Id));
transactions.push(ptSvc.removeItem(bkmk3Id));
var txn = ptSvc.aggregateTransactions("RemoveItems", transactions);
txn.doTransaction();
do_check_eq(bmsvc.getItemIndex(bkmk1Id), -1);
do_check_eq(bmsvc.getItemIndex(bkmk2Id), -1);
do_check_eq(bmsvc.getItemIndex(bkmk3Id), -1);
do_check_eq(bmsvc.getItemIndex(bkmk3_1Id), -1);
do_check_eq(bmsvc.getItemIndex(bkmk3_2Id), -1);
do_check_eq(bmsvc.getItemIndex(bkmk3_3Id), -1);
txn.undoTransaction();
var newBkmk1Id = bmsvc.getIdForItemAt(root, 0);
var newBkmk2Id = bmsvc.getIdForItemAt(root, 1);
var newBkmk3Id = bmsvc.getIdForItemAt(root, 2);
var newBkmk3_1Id = bmsvc.getIdForItemAt(newBkmk3Id, 0);
var newBkmk3_2Id = bmsvc.getIdForItemAt(newBkmk3Id, 1);
var newBkmk3_3Id = bmsvc.getIdForItemAt(newBkmk3Id, 2);
do_check_eq(bmsvc.getItemType(newBkmk1Id), bmsvc.TYPE_BOOKMARK);
do_check_eq(bmsvc.getBookmarkURI(newBkmk1Id).spec, "http://www.mozilla.org/");
do_check_eq(bmsvc.getItemType(newBkmk2Id), bmsvc.TYPE_SEPARATOR);
do_check_eq(bmsvc.getItemType(newBkmk3Id), bmsvc.TYPE_FOLDER);
do_check_eq(bmsvc.getItemTitle(newBkmk3Id), "folder");
do_check_eq(bmsvc.getFolderIdForItem(newBkmk3_1Id), newBkmk3Id);
do_check_eq(bmsvc.getItemType(newBkmk3_1Id), bmsvc.TYPE_BOOKMARK);
do_check_eq(bmsvc.getBookmarkURI(newBkmk3_1Id).spec, "http://www.mozilla.org/");
do_check_eq(bmsvc.getFolderIdForItem(newBkmk3_2Id), newBkmk3Id);
do_check_eq(bmsvc.getItemType(newBkmk3_2Id), bmsvc.TYPE_SEPARATOR);
do_check_eq(bmsvc.getFolderIdForItem(newBkmk3_3Id), newBkmk3Id);
do_check_eq(bmsvc.getItemType(newBkmk3_3Id), bmsvc.TYPE_FOLDER);
do_check_eq(bmsvc.getItemTitle(newBkmk3_3Id), "folder");
txn.redoTransaction();
do_check_eq(bmsvc.getItemIndex(newBkmk1Id), -1);
do_check_eq(bmsvc.getItemIndex(newBkmk2Id), -1);
do_check_eq(bmsvc.getItemIndex(newBkmk3Id), -1);
do_check_eq(bmsvc.getItemIndex(newBkmk3_1Id), -1);
do_check_eq(bmsvc.getItemIndex(newBkmk3_2Id), -1);
do_check_eq(bmsvc.getItemIndex(newBkmk3_3Id), -1);
txn.undoTransaction();
newBkmk1Id = bmsvc.getIdForItemAt(root, 0);
newBkmk2Id = bmsvc.getIdForItemAt(root, 1);
newBkmk3Id = bmsvc.getIdForItemAt(root, 2);
newBkmk3_1Id = bmsvc.getIdForItemAt(newBkmk3Id, 0);
newBkmk3_2Id = bmsvc.getIdForItemAt(newBkmk3Id, 1);
newBkmk3_3Id = bmsvc.getIdForItemAt(newBkmk3Id, 2);
do_check_eq(bmsvc.getItemType(newBkmk1Id), bmsvc.TYPE_BOOKMARK);
do_check_eq(bmsvc.getBookmarkURI(newBkmk1Id).spec, "http://www.mozilla.org/");
do_check_eq(bmsvc.getItemType(newBkmk2Id), bmsvc.TYPE_SEPARATOR);
do_check_eq(bmsvc.getItemType(newBkmk3Id), bmsvc.TYPE_FOLDER);
do_check_eq(bmsvc.getItemTitle(newBkmk3Id), "folder");
do_check_eq(bmsvc.getFolderIdForItem(newBkmk3_1Id), newBkmk3Id);
do_check_eq(bmsvc.getItemType(newBkmk3_1Id), bmsvc.TYPE_BOOKMARK);
do_check_eq(bmsvc.getBookmarkURI(newBkmk3_1Id).spec, "http://www.mozilla.org/");
do_check_eq(bmsvc.getFolderIdForItem(newBkmk3_2Id), newBkmk3Id);
do_check_eq(bmsvc.getItemType(newBkmk3_2Id), bmsvc.TYPE_SEPARATOR);
do_check_eq(bmsvc.getFolderIdForItem(newBkmk3_3Id), newBkmk3Id);
do_check_eq(bmsvc.getItemType(newBkmk3_3Id), bmsvc.TYPE_FOLDER);
do_check_eq(bmsvc.getItemTitle(newBkmk3_3Id), "folder");
}

Просмотреть файл

@ -1516,21 +1516,8 @@ nsNavBookmarks::GetRemoveFolderTransaction(PRInt64 aFolder, nsITransaction** aRe
// Create and initialize a RemoveFolderTransaction object that can be used to
// recreate the folder safely later.
nsCAutoString title;
nsresult rv = GetItemTitle(aFolder, title);
NS_ENSURE_SUCCESS(rv, rv);
PRInt64 parent;
PRInt32 index;
rv = GetParentAndIndexOfFolder(aFolder, &parent, &index);
NS_ENSURE_SUCCESS(rv, rv);
nsCAutoString type;
rv = GetFolderType(aFolder, type);
NS_ENSURE_SUCCESS(rv, rv);
RemoveFolderTransaction* rft =
new RemoveFolderTransaction(aFolder, parent, title, index, NS_ConvertUTF8toUTF16(type));
new RemoveFolderTransaction(aFolder);
if (!rft)
return NS_ERROR_OUT_OF_MEMORY;

Просмотреть файл

@ -217,20 +217,24 @@ private:
class RemoveFolderTransaction : public nsITransaction {
public:
RemoveFolderTransaction(PRInt64 aID, PRInt64 aParent,
const nsACString& aTitle, PRInt32 aIndex,
const nsAString& aType)
: mID(aID),
mParent(aParent),
mIndex(aIndex){
mTitle = aTitle;
mType = aType;
}
RemoveFolderTransaction(PRInt64 aID) : mID(aID) {}
NS_DECL_ISUPPORTS
NS_IMETHOD DoTransaction() {
nsNavBookmarks* bookmarks = nsNavBookmarks::GetBookmarksService();
nsresult rv = bookmarks->GetParentAndIndexOfFolder(mID, &mParent, &mIndex);
NS_ENSURE_SUCCESS(rv, rv);
rv = bookmarks->GetItemTitle(mID, mTitle);
NS_ENSURE_SUCCESS(rv, rv);
nsCAutoString type;
rv = bookmarks->GetFolderType(mID, type);
NS_ENSURE_SUCCESS(rv, rv);
mType = NS_ConvertUTF8toUTF16(type);
return bookmarks->RemoveFolder(mID);
}