From 0d1d4ee5cbb27c733bec252fb12623c5ced1ab52 Mon Sep 17 00:00:00 2001
From: Dan Stillman <dstillman@zotero.org>
Date: Tue, 23 Jun 2015 16:56:10 -0400
Subject: [PATCH] Don't change selection unnecessarily when creating trashed
 item

---
 chrome/content/zotero/xpcom/itemTreeView.js | 35 +++++++--------------
 test/tests/itemTreeViewTest.js              | 17 ++++++++++
 2 files changed, 28 insertions(+), 24 deletions(-)

diff --git a/chrome/content/zotero/xpcom/itemTreeView.js b/chrome/content/zotero/xpcom/itemTreeView.js
index 4e7798dfe..57a2da6fa 100644
--- a/chrome/content/zotero/xpcom/itemTreeView.js
+++ b/chrome/content/zotero/xpcom/itemTreeView.js
@@ -823,14 +823,14 @@ Zotero.ItemTreeView.prototype.notify = Zotero.Promise.coroutine(function* (actio
 			}
 		}
 		
+		if (sort) {
+			yield this.sort(typeof sort == 'number' ? sort : false);
+		}
+		else {
+			this._refreshItemRowMap();
+		}
+		
 		if (singleSelect) {
-			if (sort) {
-				yield this.sort(typeof sort == 'number' ? sort : false);
-			}
-			else {
-				this._refreshItemRowMap();
-			}
-			
 			if (!extraData[singleSelect] || !extraData[singleSelect].skipSelect) {
 				// Reset to Info tab
 				this._ownerDocument.getElementById('zotero-view-tabbox').selectedIndex = 0;
@@ -841,19 +841,14 @@ Zotero.ItemTreeView.prototype.notify = Zotero.Promise.coroutine(function* (actio
 		else if (action == 'modify' && ids.length == 1 &&
 				savedSelection.length == 1 && savedSelection[0] == ids[0]) {
 			// If the item no longer matches the search term, clear the search
+			// DEBUG: Still needed/wanted? (and search is async, so doesn't work anyway,
+			// here or above)
 			if (quicksearch && this._rowMap[ids[0]] == undefined) {
 				Zotero.debug('Selected item no longer matches quicksearch -- clearing');
 				quicksearch.value = '';
 				quicksearch.doCommand();
 			}
 			
-			if (sort) {
-				yield this.sort(typeof sort == 'number' ? sort : false);
-			}
-			else {
-				this._refreshItemRowMap();
-			}
-			
 			if (activeWindow) {
 				yield this.selectItem(ids[0]);
 			}
@@ -861,16 +856,8 @@ Zotero.ItemTreeView.prototype.notify = Zotero.Promise.coroutine(function* (actio
 				yield this.rememberSelection(savedSelection);
 			}
 		}
-		else
-		{
-			if (sort) {
-				yield this.sort(typeof sort == 'number' ? sort : false);
-			}
-			else {
-				this._refreshItemRowMap();
-			}
-			
-			// On removal of a row, select item at previous position
+		// On removal of a row, select item at previous position
+		else if (savedSelection.length) {
 			if (action == 'remove' || action == 'trash' || action == 'delete') {
 				// In duplicates view, select the next set on delete
 				if (collectionTreeRow.isDuplicates()) {
diff --git a/test/tests/itemTreeViewTest.js b/test/tests/itemTreeViewTest.js
index 6a3e1d9f3..fb44b190a 100644
--- a/test/tests/itemTreeViewTest.js
+++ b/test/tests/itemTreeViewTest.js
@@ -116,6 +116,23 @@ describe("Zotero.ItemTreeView", function() {
 			assert.equal(selected[0], existingItemID);
 		});
 		
+		it("shouldn't change selection outside of trash if new trashed item is created with skipSelect", function* () {
+			yield selectLibrary(win);
+			yield waitForItemsLoad(win);
+			
+			itemsView.selection.clearSelection();
+			
+			var item = createUnsavedDataObject('item');
+			item.deleted = true;
+			var id = yield item.saveTx({
+				skipSelect: true
+			});
+			
+			// Nothing should be selected
+			selected = itemsView.getSelectedItems(true);
+			assert.lengthOf(selected, 0);
+		})
+		
 		it("shouldn't select a modified item", function* () {
 			// Create item
 			var item = new Zotero.Item('book');