From 1d45c6c88258e8b08d43b3c853032d1b00307d30 Mon Sep 17 00:00:00 2001
From: Dan Stillman <dstillman@zotero.org>
Date: Tue, 9 Jun 2015 01:29:06 -0400
Subject: [PATCH] Reselect the same row when an item is removed

---
 chrome/content/zotero/xpcom/itemTreeView.js | 23 ++++++-------
 test/tests/itemTreeViewTest.js              | 36 +++++++++++++++++++--
 2 files changed, 43 insertions(+), 16 deletions(-)

diff --git a/chrome/content/zotero/xpcom/itemTreeView.js b/chrome/content/zotero/xpcom/itemTreeView.js
index 40e7ea28a..047b6dac2 100644
--- a/chrome/content/zotero/xpcom/itemTreeView.js
+++ b/chrome/content/zotero/xpcom/itemTreeView.js
@@ -459,7 +459,7 @@ Zotero.ItemTreeView.prototype.notify = Zotero.Promise.coroutine(function* (actio
 	var sort = false;
 	
 	var savedSelection = this.getSelectedItems(true);
-	var previousRow = false;
+	var previousFirstRow = this._rowMap[ids[0]];
 	
 	// Redraw the tree (for tag color and progress changes)
 	if (action == 'redraw') {
@@ -569,7 +569,6 @@ Zotero.ItemTreeView.prototype.notify = Zotero.Promise.coroutine(function* (actio
 		// On a delete in duplicates mode, just refresh rather than figuring
 		// out what to remove
 		if (collectionTreeRow.isDuplicates()) {
-			previousRow = this._rowMap[ids[0]];
 			yield this.refresh();
 			refreshed = true;
 			madeChanges = true;
@@ -879,10 +878,6 @@ Zotero.ItemTreeView.prototype.notify = Zotero.Promise.coroutine(function* (actio
 		}
 		else
 		{
-			if (previousRow === false) {
-				previousRow = this._rowMap[ids[0]];
-			}
-			
 			if (sort) {
 				yield this.sort(typeof sort == 'number' ? sort : false);
 			}
@@ -894,9 +889,9 @@ Zotero.ItemTreeView.prototype.notify = Zotero.Promise.coroutine(function* (actio
 			if (action == 'remove' || action == 'trash' || action == 'delete') {
 				// In duplicates view, select the next set on delete
 				if (collectionTreeRow.isDuplicates()) {
-					if (this._rows[previousRow]) {
+					if (this._rows[previousFirstRow]) {
 						// Mirror ZoteroPane.onTreeMouseDown behavior
-						var itemID = this._rows[previousRow].ref.id;
+						var itemID = this._rows[previousFirstRow].ref.id;
 						var setItemIDs = collectionTreeRow.ref.getSetItemsByItemID(itemID);
 						this.selectItems(setItemIDs);
 					}
@@ -905,17 +900,17 @@ Zotero.ItemTreeView.prototype.notify = Zotero.Promise.coroutine(function* (actio
 					// If this was a child item and the next item at this
 					// position is a top-level item, move selection one row
 					// up to select a sibling or parent
-					if (ids.length == 1 && previousRow > 0) {
-						var previousItem = yield Zotero.Items.getAsync(ids[0]);
+					if (ids.length == 1 && previousFirstRow > 0) {
+						let previousItem = yield Zotero.Items.getAsync(ids[0]);
 						if (previousItem && !previousItem.isTopLevelItem()) {
-							if (this._rows[previousRow] && this.getLevel(previousRow) == 0) {
-								previousRow--;
+							if (this._rows[previousFirstRow] && this.getLevel(previousFirstRow) == 0) {
+								previousFirstRow--;
 							}
 						}
 					}
 					
-					if (this._rows[previousRow]) {
-						this.selection.select(previousRow);
+					if (previousFirstRow !== undefined && this._rows[previousFirstRow]) {
+						this.selection.select(previousFirstRow);
 					}
 					// If no item at previous position, select last item in list
 					else if (this._rows[this._rows.length - 1]) {
diff --git a/test/tests/itemTreeViewTest.js b/test/tests/itemTreeViewTest.js
index 84bc06138..6a3e1d9f3 100644
--- a/test/tests/itemTreeViewTest.js
+++ b/test/tests/itemTreeViewTest.js
@@ -1,14 +1,19 @@
 describe("Zotero.ItemTreeView", function() {
-	var win, itemsView, existingItemID;
+	var win, zp, itemsView, existingItemID;
 	
 	// Load Zotero pane and select library
 	before(function* () {
 		win = yield loadZoteroPane();
-		itemsView = win.ZoteroPane.itemsView;
+		zp = win.ZoteroPane;
 		
 		var item = new Zotero.Item('book');
 		existingItemID = yield item.saveTx();
 	});
+	beforeEach(function* () {
+		yield zp.collectionsView.selectLibrary();
+		yield waitForItemsLoad(win)
+		itemsView = zp.itemsView;
+	})
 	after(function () {
 		win.close();
 	});
@@ -161,6 +166,33 @@ describe("Zotero.ItemTreeView", function() {
 			assert.lengthOf(selected, 1);
 			assert.equal(selected[0], id);
 		});
+		
+		it("should reselect the same row when an item is removed", function* () {
+			var collection = yield createDataObject('collection');
+			yield waitForItemsLoad(win);
+			itemsView = zp.itemsView;
+			
+			var items = [];
+			var num = 10;
+			for (let i = 0; i < num; i++) {
+				let item = createUnsavedDataObject('item');
+				item.addToCollection(collection.id);
+				yield item.saveTx();
+				items.push(item);
+			}
+			yield Zotero.Promise.delay(2000);
+			assert.equal(itemsView.rowCount, num);
+			
+			// Select the third item in the list
+			itemsView.selection.select(2);
+			var treeRow = itemsView.getRow(2);
+			yield treeRow.ref.eraseTx();
+			
+			// Selection should stay on third row
+			assert.equal(itemsView.selection.currentIndex, 2);
+			
+			yield Zotero.Items.erase(items.map(item => item.id));
+		})
 	})
 	
 	describe("#drop()", function () {