From 2a69885b116a119caf18f67f01742f6de8c814d7 Mon Sep 17 00:00:00 2001
From: Dan Stillman <dstillman@zotero.org>
Date: Thu, 7 May 2015 18:18:48 -0400
Subject: [PATCH] Fix placement of saved searches in collections tree

And unify row add/remove handling between collections tree and items
tree
---
 .../zotero/xpcom/collectionTreeView.js        | 201 +++++++-----------
 chrome/content/zotero/xpcom/itemTreeView.js   | 152 ++++---------
 .../content/zotero/xpcom/libraryTreeView.js   |  75 +++++++
 test/tests/collectionTreeViewTest.js          |  31 +++
 4 files changed, 221 insertions(+), 238 deletions(-)

diff --git a/chrome/content/zotero/xpcom/collectionTreeView.js b/chrome/content/zotero/xpcom/collectionTreeView.js
index b0f978248..951daf0a9 100644
--- a/chrome/content/zotero/xpcom/collectionTreeView.js
+++ b/chrome/content/zotero/xpcom/collectionTreeView.js
@@ -136,7 +136,7 @@ Zotero.CollectionTreeView.prototype.refresh = Zotero.Promise.coroutine(function*
 	// Record open states before refreshing
 	if (this._rows) {
 		for (var i=0, len=this._rows.length; i<len; i++) {
-			var treeRow = this._rows[i][0]
+			var treeRow = this._rows[i];
 			if (treeRow.ref && treeRow.ref.id == 'commons-header') {
 				var commonsExpand = this.isContainerOpen(i);
 			}
@@ -172,43 +172,51 @@ Zotero.CollectionTreeView.prototype.refresh = Zotero.Promise.coroutine(function*
 	
 	var oldCount = this.rowCount || 0;
 	var newRows = [];
-	
-	var self = this;
+	var added = 0;
 	
 	//
 	// Add "My Library"
 	//
-	this._addRow(
+	this._addRowToArray(
 		newRows,
-		new Zotero.CollectionTreeRow('library', { libraryID: Zotero.Libraries.userLibraryID })
+		new Zotero.CollectionTreeRow('library', { libraryID: Zotero.Libraries.userLibraryID }),
+		added++
 	);
-	yield this._expandRow(newRows, 0);
+	added += yield this._expandRow(newRows, 0);
 	
-	this._addRow(newRows, new Zotero.CollectionTreeRow('separator', false));
+	this._addRowToArray(newRows, new Zotero.CollectionTreeRow('separator', false), added++);
 	
 	// Add "My Publications"
-	this._addRow(
+	this._addRowToArray(
 		newRows,
 		new Zotero.CollectionTreeRow('publications', {
 			libraryID: Zotero.Libraries.publicationsLibraryID
-		})
+		}),
+		added++
 	);
 	
 	// Add groups
 	var groups = yield Zotero.Groups.getAll();
 	if (groups.length) {
-		this._addRow(newRows, new Zotero.CollectionTreeRow('separator', false));
-		var row = this._addRow(
+		this._addRowToArray(
+			newRows,
+			new Zotero.CollectionTreeRow('separator', false),
+			added++
+		);
+		this._addRowToArray(
 			newRows,
 			new Zotero.CollectionTreeRow('header', {
 				id: "group-libraries-header",
 				label: Zotero.getString('pane.collections.groupLibraries'),
 				libraryID: -1
-			}, 0)
+			}, 0),
+			added++
 		);
 		for (let i = 0, len = groups.length; i < len; i++) {
-			this._rowMap['L' + groups[i].libraryID] = this._addRow(
-				newRows, new Zotero.CollectionTreeRow('group', groups[i])
+			this._addRowToArray(
+				newRows,
+				new Zotero.CollectionTreeRow('group', groups[i]),
+				added++
 			);
 		}
 	}
@@ -244,8 +252,8 @@ Zotero.CollectionTreeView.prototype.notify = Zotero.Promise.coroutine(function*
 		return;
 	}
 	
-	if (!this._collectionRowMap) {
-		Zotero.debug("Collection row map didn't exist in collectionTreeView.notify()");
+	if (!this._rowMap) {
+		Zotero.debug("Row map didn't exist in collectionTreeView.notify()");
 		return;
 	}
 	
@@ -326,9 +334,9 @@ Zotero.CollectionTreeView.prototype.notify = Zotero.Promise.coroutine(function*
 		for (var i=0; i<ids.length; i++) {
 			var collection = yield Zotero.Collections.getAsync(ids[i]);
 			var parentID = collection.parentID;
-			if (parentID && this._collectionRowMap[parentID] &&
-					!this.isContainerOpen(this._collectionRowMap[parentID])) {
-				yield this.toggleOpenState(this._collectionRowMap[parentID]);
+			if (parentID && this._rowMap["C" + parentID] &&
+					!this.isContainerOpen(this._rowMap["C" + parentID])) {
+				yield this.toggleOpenState(this._rowMap["C" + parentID]);
 			}
 		}
 		
@@ -392,14 +400,14 @@ Zotero.CollectionTreeView.prototype._addSortedRow = Zotero.Promise.coroutine(fun
 		let parentID = collection.parentID;
 		
 		// If parent isn't visible, don't add
-		if (parentID && this._collectionRowMap[parentID] === undefined) {
+		if (parentID && this._rowMap["C" + parentID] === undefined) {
 			return false;
 		}
 		
 		let libraryID = collection.libraryID;
 		let startRow;
 		if (parentID) {
-			startRow = this._collectionRowMap[parentID];
+			startRow = this._rowMap["C" + parentID];
 		}
 		else {
 			startRow = this._rowMap['L' + libraryID];
@@ -450,9 +458,7 @@ Zotero.CollectionTreeView.prototype._addSortedRow = Zotero.Promise.coroutine(fun
 			}
 		}
 		this._addRow(
-			this._rows,
-			new Zotero.CollectionTreeRow('collection', collection),
-			level,
+			new Zotero.CollectionTreeRow('collection', collection, level),
 			beforeRow
 		);
 	}
@@ -476,37 +482,27 @@ Zotero.CollectionTreeView.prototype._addSortedRow = Zotero.Promise.coroutine(fun
 			var inSearches = false;
 			for (let i = startRow; i < this.rowCount; i++) {
 				let treeRow = this.getRow(i);
+				Zotero.debug(treeRow.id);
 				beforeRow = i;
 				
-				// If we've moved on to a different library, stop
-				if (treeRow.ref.libraryID != libraryID) {
-					break;
-				}
-				
+				// If we've reached something other than collections, stop
 				if (treeRow.isSearch()) {
-					inSearches = true;
-					
 					// If current search sorts after, stop
 					if (Zotero.localeCompare(treeRow.ref.name, search.name) > 0) {
 						break;
 					}
 				}
-				// If we've found searches but then see something other than a search, stop
-				else if (inSearches) {
+				// If it's not a search and it's not a collection, stop
+				else if (!treeRow.isCollection()) {
 					break;
 				}
 			}
 		}
 		this._addRow(
-			this._rows,
-			new Zotero.CollectionTreeRow('search', search),
-			level,
+			new Zotero.CollectionTreeRow('search', search, level),
 			beforeRow
 		);
 	}
-	this.rowCount++;
-	this._treebox.rowCountChanged(beforeRow, 1);
-	this._refreshRowMap();
 	return true;
 });
 
@@ -525,7 +521,7 @@ Zotero.CollectionTreeView.prototype.setHighlightedRows = Zotero.Promise.coroutin
 		if (id[0] == 'C') {
 			id = id.substr(1);
 			yield this.expandToCollection(id);
-			row = this._collectionRowMap[id];
+			row = this._rowMap["C" + id];
 		}
 		if (row) {
 			this._highlightedRows[row] = true;
@@ -621,11 +617,6 @@ Zotero.CollectionTreeView.prototype.isContainer = function(row)
 	return treeRow.isLibrary(true) || treeRow.isCollection() || treeRow.isPublications() || treeRow.isBucket();
 }
 
-Zotero.CollectionTreeView.prototype.isContainerOpen = function(row)
-{
-	return this._rows[row][1];
-}
-
 /*
  * Returns true if the collection has no child collections
  */
@@ -653,11 +644,6 @@ Zotero.CollectionTreeView.prototype.isContainerEmpty = function(row)
 	return true;
 }
 
-Zotero.CollectionTreeView.prototype.getLevel = function(row)
-{
-	return this._rows[row][2];
-}
-
 Zotero.CollectionTreeView.prototype.getParentIndex = function(row)
 {
 	var thisLevel = this.getLevel(row);
@@ -798,7 +784,7 @@ Zotero.CollectionTreeView.prototype.expandToCollection = Zotero.Promise.coroutin
 		yield this.toggleOpenState(libraryRow);
 	}
 	
-	var row = this._collectionRowMap[collectionID];
+	var row = this._rowMap["C" + collectionID];
 	if (row !== undefined) {
 		return true;
 	}
@@ -809,7 +795,7 @@ Zotero.CollectionTreeView.prototype.expandToCollection = Zotero.Promise.coroutin
 		col = yield Zotero.Collections.getAsync(parentID);
 	}
 	for each(var id in path) {
-		row = this._collectionRowMap[id];
+		row = this._rowMap["C" + id];
 		if (!this.isContainerOpen(row)) {
 			yield this.toggleOpenState(row);
 		}
@@ -957,8 +943,8 @@ Zotero.CollectionTreeView.prototype.getLastViewedRow = Zotero.Promise.coroutine(
 	var select = 0;
 	if (matches) {
 		if (matches[1] == 'C') {
-			if (this._collectionRowMap[matches[2]]) {
-				select = this._collectionRowMap[matches[2]];
+			if (this._rowMap["C" + matches[2]]) {
+				select = this._rowMap["C" + matches[2]];
 			}
 			// Search recursively
 			else {
@@ -986,11 +972,11 @@ Zotero.CollectionTreeView.prototype.getLastViewedRow = Zotero.Promise.coroutine(
 					lastCol = par;
 					path.push(lastCol);
 				}
-				while (!this._collectionRowMap[lastCol] && failsafe > 0)
+				while (!this._rowMap["C" + lastCol] && failsafe > 0)
 				if (path.length) {
 					for (var i=path.length-1; i>=0; i--) {
 						var id = path[i];
-						var row = this._collectionRowMap[id];
+						var row = this._rowMap["C" + id];
 						if (!row) {
 							var msg = "Collection not found in tree in "
 								+ "Zotero.CollectionTreeView.setTree()";
@@ -1000,8 +986,8 @@ Zotero.CollectionTreeView.prototype.getLastViewedRow = Zotero.Promise.coroutine(
 						}
 						if (!this.isContainerOpen(row)) {
 							yield this.toggleOpenState(row);
-							if (this._collectionRowMap[matches[2]]) {
-								select = this._collectionRowMap[matches[2]];
+							if (this._rowMap["C" + matches[2]]) {
+								select = this._rowMap["C" + matches[2]];
 								break;
 							}
 						}
@@ -1081,8 +1067,8 @@ Zotero.CollectionTreeView.prototype.deleteSelection = Zotero.Promise.coroutine(f
  * Expand row based on last state, or manually from toggleOpenState()
  */
 Zotero.CollectionTreeView.prototype._expandRow = Zotero.Promise.coroutine(function* (rows, row, forceOpen) {
-	var treeRow = rows[row][0];
-	var level = rows[row][2];
+	var treeRow = rows[row];
+	var level = rows[row].level;
 	var isLibrary = treeRow.isLibrary(true);
 	var isGroup = treeRow.isGroup();
 	var isCollection = treeRow.isCollection();
@@ -1120,7 +1106,7 @@ Zotero.CollectionTreeView.prototype._expandRow = Zotero.Promise.coroutine(functi
 	if (!forceOpen &&
 			(this._containerState[treeRow.id] === false
 				|| (isCollection && !this._containerState[treeRow.id]))) {
-		rows[row][1] = false;
+		rows[row].isOpen = false;
 		return 0;
 	}
 	
@@ -1129,7 +1115,7 @@ Zotero.CollectionTreeView.prototype._expandRow = Zotero.Promise.coroutine(functi
 	// If this isn't a manual open, set the initial state depending on whether
 	// there are child nodes
 	if (!forceOpen) {
-		rows[row][1] = startOpen;
+		rows[row].isOpen = startOpen;
 	}
 	
 	if (!startOpen) {
@@ -1140,17 +1126,15 @@ Zotero.CollectionTreeView.prototype._expandRow = Zotero.Promise.coroutine(functi
 	
 	// Add collections
 	for (var i = 0, len = collections.length; i < len; i++) {
-		var newRow = this._addRow(
+		let beforeRow = row + 1 + newRows;
+		this._addRowToArray(
 			rows,
-			new Zotero.CollectionTreeRow('collection', collections[i]),
-			level + 1,
-			row + 1 + newRows
+			new Zotero.CollectionTreeRow('collection', collections[i], level + 1),
+			beforeRow
 		);
-		
-		// Recursively expand child collections that should be open
-		newRows += yield this._expandRow(rows, newRow);
-		
 		newRows++;
+		// Recursively expand child collections that should be open
+		newRows += yield this._expandRow(rows, beforeRow);
 	}
 	
 	if (isCollection) {
@@ -1159,14 +1143,22 @@ Zotero.CollectionTreeView.prototype._expandRow = Zotero.Promise.coroutine(functi
 	
 	// Add searches
 	for (var i = 0, len = savedSearches.length; i < len; i++) {
-		this._addRow(rows, new Zotero.CollectionTreeRow('search', savedSearches[i]), level + 1, row + 1 + newRows);
+		this._addRowToArray(
+			rows,
+			new Zotero.CollectionTreeRow('search', savedSearches[i], level + 1),
+			row + 1 + newRows
+		);
 		newRows++;
 	}
 	
 	// Duplicate items
 	if (showDuplicates) {
 		let d = new Zotero.Duplicates(libraryID);
-		this._addRow(rows, new Zotero.CollectionTreeRow('duplicates', d), level + 1, row + 1 + newRows);
+		this._addRowToArray(
+			rows,
+			new Zotero.CollectionTreeRow('duplicates', d, level + 1),
+			row + 1 + newRows
+		);
 		newRows++;
 	}
 	
@@ -1177,7 +1169,11 @@ Zotero.CollectionTreeView.prototype._expandRow = Zotero.Promise.coroutine(functi
 		s.name = Zotero.getString('pane.collections.unfiled');
 		s.addCondition('libraryID', 'is', libraryID);
 		s.addCondition('unfiled', 'true');
-		this._addRow(rows, new Zotero.CollectionTreeRow('unfiled', s), level + 1, row + 1 + newRows);
+		this._addRowToArray(
+			rows,
+			new Zotero.CollectionTreeRow('unfiled', s, level + 1),
+			row + 1 + newRows
+		);
 		newRows++;
 	}
 	
@@ -1187,7 +1183,11 @@ Zotero.CollectionTreeView.prototype._expandRow = Zotero.Promise.coroutine(functi
 			var ref = {
 				libraryID: libraryID
 			};
-			this._addRow(rows, new Zotero.CollectionTreeRow('trash', ref), level + 1, row + 1 + newRows);
+			this._addRowToArray(
+				rows,
+				new Zotero.CollectionTreeRow('trash', ref, level + 1),
+				row + 1 + newRows
+			);
 			newRows++;
 		}
 		this._trashNotEmpty[libraryID] = !!deletedItems.length;
@@ -1197,45 +1197,6 @@ Zotero.CollectionTreeView.prototype._expandRow = Zotero.Promise.coroutine(functi
 });
 
 
-/*
- *  Called by various view functions to show a row
- */
-Zotero.CollectionTreeView.prototype._addRow = function (rows, treeRow, level, beforeRow) {
-	if (!level) {
-		level = 0;
-	}
-	
-	if (!beforeRow) {
-		beforeRow = rows.length;
-	}
-	
-	rows.splice(beforeRow, 0, [treeRow, false, level]);
-	
-	return beforeRow;
-}
-
-
-/*
- *  Called by view to hide specified row
- */
-Zotero.CollectionTreeView.prototype._removeRow = function(row)
-{
-	this._rows.splice(row,1);
-	this.rowCount--;
-	if (this.selection.isSelected(row)) {
-		this.selection.toggleSelect(row);
-	}
-}
-
-
-/**
- * Returns Zotero.CollectionTreeRow at row
- */
-Zotero.CollectionTreeView.prototype.getRow = function (row) {
-	return this._rows[row][0];
-}
-
-
 /**
  * Returns libraryID or FALSE if not a library
  */
@@ -1295,16 +1256,8 @@ Zotero.CollectionTreeView.prototype.rememberSelection = Zotero.Promise.coroutine
  */
 Zotero.CollectionTreeView.prototype._refreshRowMap = function() {
 	this._rowMap = {};
-	this._collectionRowMap = {};
 	for (let i = 0, len = this.rowCount; i < len; i++) {
-		let treeRow = this.getRow(i);
-		
-		this._rowMap[treeRow.id] = i;
-		
-		// Collections get special treatment for now
-		if (treeRow.isCollection()) {
-			this._collectionRowMap[treeRow.ref.id] = i;
-		}
+		this._rowMap[this.getRow(i).id] = i;
 	}
 }
 
@@ -2197,10 +2150,12 @@ Zotero.CollectionTreeCache = {
 	})
 };
 
-Zotero.CollectionTreeRow = function(type, ref)
+Zotero.CollectionTreeRow = function(type, ref, level, isOpen)
 {
 	this.type = type;
 	this.ref = ref;
+	this.level = level || 0
+	this.isOpen = isOpen || false;
 }
 
 
diff --git a/chrome/content/zotero/xpcom/itemTreeView.js b/chrome/content/zotero/xpcom/itemTreeView.js
index 3209d22f7..2717cf740 100644
--- a/chrome/content/zotero/xpcom/itemTreeView.js
+++ b/chrome/content/zotero/xpcom/itemTreeView.js
@@ -48,7 +48,6 @@ Zotero.ItemTreeView = function (collectionTreeRow, sourcesOnly) {
 	this._ownerDocument = null;
 	this._needsSort = false;
 	
-	this._rows = [];
 	this._cellTextCache = {};
 	this._itemImages = {};
 	
@@ -355,9 +354,8 @@ Zotero.ItemTreeView.prototype.refresh = Zotero.serial(Zotero.Promise.coroutine(f
 			this._addRowToArray(
 				newRows,
 				new Zotero.ItemTreeRow(item, 0, false),
-				added + 1
+				added++
 			);
-			added++;
 		}
 		newSearchItemIDs[item.id] = true;
 	}
@@ -369,9 +367,8 @@ Zotero.ItemTreeView.prototype.refresh = Zotero.serial(Zotero.Promise.coroutine(f
 			this._addRowToArray(
 				newRows,
 				new Zotero.ItemTreeRow(item, 0, false),
-				added + 1
+				added++
 			);
-			added++;
 		}
 	}
 	
@@ -414,7 +411,7 @@ Zotero.ItemTreeView.prototype.notify = Zotero.Promise.coroutine(function* (actio
 		return;
 	}
 	
-	if (!this._itemRowMap) {
+	if (!this._rowMap) {
 		Zotero.debug("Item row map didn't exist in itemTreeView.notify()");
 		return;
 	}
@@ -449,13 +446,13 @@ Zotero.ItemTreeView.prototype.notify = Zotero.Promise.coroutine(function* (actio
 					if (extraData.column == 'title') {
 						delete this._itemImages[id];
 					}
-					this._treebox.invalidateCell(this._itemRowMap[id], col);
+					this._treebox.invalidateCell(this._rowMap[id], col);
 				}
 			}
 			else {
 				for each(var id in ids) {
 					delete this._itemImages[id];
-					this._treebox.invalidateRow(this._itemRowMap[id]);
+					this._treebox.invalidateRow(this._rowMap[id]);
 				}
 			}
 		}
@@ -485,7 +482,7 @@ Zotero.ItemTreeView.prototype.notify = Zotero.Promise.coroutine(function* (actio
 		}
 		// If refreshing a single item, clear caches and then unselect and reselect row
 		else if (savedSelection.length == 1 && savedSelection[0] == ids[0]) {
-			let row = this._itemRowMap[ids[0]];
+			let row = this._rowMap[ids[0]];
 			delete this._cellTextCache[row];
 			
 			this.selection.clearSelection();
@@ -541,7 +538,7 @@ 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._itemRowMap[ids[0]];
+			previousRow = this._rowMap[ids[0]];
 			yield this.refresh();
 			madeChanges = true;
 			sort = true;
@@ -561,7 +558,7 @@ Zotero.ItemTreeView.prototype.notify = Zotero.Promise.coroutine(function* (actio
 				}
 				// Row might already be gone (e.g. if this is a child and
 				// 'modify' was sent to parent)
-				let row = this._itemRowMap[ids[i]];
+				let row = this._rowMap[ids[i]];
 				if (push && row !== undefined) {
 					// Don't remove child items from collections, because it's handled by 'modify'
 					if (action == 'remove' && this.getParentIndex(row) != -1) {
@@ -622,7 +619,7 @@ Zotero.ItemTreeView.prototype.notify = Zotero.Promise.coroutine(function* (actio
 				let item = items[i];
 				let id = item.id;
 				
-				let row = this._itemRowMap[id];
+				let row = this._rowMap[id];
 				
 				// Deleted items get a modify that we have to ignore when
 				// not viewing the trash
@@ -654,7 +651,7 @@ Zotero.ItemTreeView.prototype.notify = Zotero.Promise.coroutine(function* (actio
 					}
 					// If not moved from under one item to another, just resort the row,
 					// which also invalidates it and refreshes it
-					else if (!(parentItemID && parentIndex != -1 && this._itemRowMap[parentItemID] != parentIndex)) {
+					else if (!(parentItemID && parentIndex != -1 && this._rowMap[parentItemID] != parentIndex)) {
 						sort = id;
 					}
 					
@@ -734,7 +731,7 @@ Zotero.ItemTreeView.prototype.notify = Zotero.Promise.coroutine(function* (actio
 						&& collectionTreeRow.ref.libraryID == item.libraryID)
 						|| (collectionTreeRow.isCollection() && item.inCollection(collectionTreeRow.ref.id)))
 					// if we haven't already added it to our hash map
-					&& this._itemRowMap[item.id] == null
+					&& this._rowMap[item.id] == null
 					// Regular item or standalone note/attachment
 					&& item.isTopLevelItem()) {
 					let beforeRow = this.rowCount;
@@ -824,7 +821,7 @@ 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
-			if (quicksearch && this._itemRowMap[ids[0]] == undefined) {
+			if (quicksearch && this._rowMap[ids[0]] == undefined) {
 				Zotero.debug('Selected item no longer matches quicksearch -- clearing');
 				quicksearch.value = '';
 				quicksearch.doCommand();
@@ -847,7 +844,7 @@ Zotero.ItemTreeView.prototype.notify = Zotero.Promise.coroutine(function* (actio
 		else
 		{
 			if (previousRow === false) {
-				previousRow = this._itemRowMap[ids[0]];
+				previousRow = this._rowMap[ids[0]];
 			}
 			
 			if (sort) {
@@ -1116,11 +1113,6 @@ Zotero.ItemTreeView.prototype.isContainer = function(row)
 	return this.getRow(row).ref.isRegularItem();
 }
 
-Zotero.ItemTreeView.prototype.isContainerOpen = function(row)
-{
-	return this._rows[row].isOpen;
-}
-
 Zotero.ItemTreeView.prototype.isContainerEmpty = function(row)
 {
 	if (this._sourcesOnly) {
@@ -1135,11 +1127,6 @@ Zotero.ItemTreeView.prototype.isContainerEmpty = function(row)
 	return item.numNotes(includeTrashed) === 0 && item.numAttachments(includeTrashed) == 0;
 }
 
-Zotero.ItemTreeView.prototype.getLevel = function(row)
-{
-	return this.getRow(row).level;
-}
-
 // Gets the index of the row's container, or -1 if none (top-level)
 Zotero.ItemTreeView.prototype.getParentIndex = function(row)
 {
@@ -1296,13 +1283,13 @@ Zotero.ItemTreeView.prototype.cycleHeader = Zotero.Promise.coroutine(function* (
 	this.selection.selectEventsSuppressed = true;
 	var savedSelection = this.getSelectedItems(true);
 	if (savedSelection.length == 1) {
-		var pos = this._itemRowMap[savedSelection[0]] - this._treebox.getFirstVisibleRow();
+		var pos = this._rowMap[savedSelection[0]] - this._treebox.getFirstVisibleRow();
 	}
 	yield this.sort();
 	yield this.rememberSelection(savedSelection);
 	// If single row was selected, try to keep it in the same place
 	if (savedSelection.length == 1) {
-		var newRow = this._itemRowMap[savedSelection[0]];
+		var newRow = this._rowMap[savedSelection[0]];
 		// Calculate the last row that would give us a full view
 		var fullTop = Math.max(0, this._rows.length - this._treebox.getPageLength());
 		// Calculate the row that would give us the same position
@@ -1327,9 +1314,9 @@ Zotero.ItemTreeView.prototype.sort = Zotero.Promise.coroutine(function* (itemID)
 	this._needsSort = false;
 	
 	// Single child item sort -- just toggle parent closed and open
-	if (itemID && this._itemRowMap[itemID] &&
-			this.getRow(this._itemRowMap[itemID]).ref.parentKey) {
-		let parentIndex = this.getParentIndex(this._itemRowMap[itemID]);
+	if (itemID && this._rowMap[itemID] &&
+			this.getRow(this._rowMap[itemID]).ref.parentKey) {
+		let parentIndex = this.getParentIndex(this._rowMap[itemID]);
 		this._closeContainer(parentIndex);
 		yield this.toggleOpenState(parentIndex);
 		return;
@@ -1537,7 +1524,7 @@ Zotero.ItemTreeView.prototype.sort = Zotero.Promise.coroutine(function* (itemID)
 	
 	// Single-row sort
 	if (itemID) {
-		let row = this._itemRowMap[itemID];
+		let row = this._rowMap[itemID];
 		for (let i=0, len=this._rows.length; i<len; i++) {
 			if (i === row) {
 				continue;
@@ -1604,10 +1591,10 @@ Zotero.ItemTreeView.prototype.selectItem = Zotero.Promise.coroutine(function* (i
 	
 	// If no row map, we're probably in the process of switching collections,
 	// so store the item to select on the item group for later
-	if (!this._itemRowMap) {
+	if (!this._rowMap) {
 		if (this.collectionTreeRow) {
 			this.collectionTreeRow.itemToSelect = { id: id, expand: expand };
-			Zotero.debug("_itemRowMap not yet set; not selecting item");
+			Zotero.debug("_rowMap not yet set; not selecting item");
 			return false;
 		}
 		
@@ -1615,7 +1602,7 @@ Zotero.ItemTreeView.prototype.selectItem = Zotero.Promise.coroutine(function* (i
 		return false;
 	}
 	
-	var row = this._itemRowMap[id];
+	var row = this._rowMap[id];
 	
 	// Get the row of the parent, if there is one
 	var parentRow = null;
@@ -1627,8 +1614,8 @@ Zotero.ItemTreeView.prototype.selectItem = Zotero.Promise.coroutine(function* (i
 	}
 	
 	var parent = item.parentItemID;
-	if (parent && this._itemRowMap[parent] != undefined) {
-		parentRow = this._itemRowMap[parent];
+	if (parent && this._rowMap[parent] != undefined) {
+		parentRow = this._rowMap[parent];
 	}
 	
 	// If row with id not visible, check to see if it's hidden under a parent
@@ -1656,7 +1643,7 @@ Zotero.ItemTreeView.prototype.selectItem = Zotero.Promise.coroutine(function* (i
 		
 		// Open the parent
 		yield this.toggleOpenState(parentRow);
-		row = this._itemRowMap[id];
+		row = this._rowMap[id];
 	}
 	
 	// This function calls nsITreeSelection.select(), which triggers the <tree>'s 'onselect'
@@ -1732,7 +1719,7 @@ Zotero.ItemTreeView.prototype.selectItems = function(ids) {
 	
 	var rows = [];
 	for each(var id in ids) {
-		if(this._itemRowMap[id] !== undefined) rows.push(this._itemRowMap[id]);
+		if(this._rowMap[id] !== undefined) rows.push(this._rowMap[id]);
 	}
 	rows.sort(function (a, b) {
 		return a - b;
@@ -1867,71 +1854,6 @@ Zotero.ItemTreeView.prototype.setFilter = Zotero.Promise.coroutine(function* (ty
 });
 
 
-/**
- * Add a tree row to the main array, update the row count, tell the treebox that the row count
- * changed, and update the row map
- *
- * @param {Array} newRows - Array to operate on
- * @param {Zotero.ItemTreeRow} itemTreeRow
- * @param {Number} beforeRow - Row index to insert new row before
- */
-Zotero.ItemTreeView.prototype._addRow = function (itemTreeRow, beforeRow) {
-	this._addRowToArray(this._rows, itemTreeRow, beforeRow);
-	this.rowCount++;
-	this._treebox.rowCountChanged(beforeRow, 1);
-	// Increment all rows in map at or above insertion point
-	for (let i in this._itemRowMap) {
-		if (this._itemRowMap[j] >= beforeRow) {
-			this._itemRowMap[j]++
-		}
-	}
-	// Add new row to map
-	this._itemRowMap[itemTreeRow.id] = beforeRow;
-}
-
-
-/**
- * Add a tree row into a given array
- *
- * @param {Array} array - Array to operate on
- * @param {Zotero.ItemTreeRow} itemTreeRow
- * @param {Number} beforeRow - Row index to insert new row before
- */
-Zotero.ItemTreeView.prototype._addRowToArray = function (array, itemTreeRow, beforeRow) {
-	array.splice(beforeRow, 0, itemTreeRow);
-}
-
-
-
-/**
- * Remove a row from the main array, decrement the row count, tell the treebox that the row
- * count changed, delete the row from the map, and optionally update all rows above it in the map
- */
-Zotero.ItemTreeView.prototype._removeRow = function (row, skipItemMapUpdate) {
-	var id = this._rows[row].id;
-	this._rows.splice(row, 1);
-	this.rowCount--;
-	this._treebox.rowCountChanged(row + 1, -1);
-	delete this._itemRowMap[id];
-	if (!skipItemMapUpdate) {
-		for (let i in this._itemRowMap) {
-			if (this._itemRowMap[i] > row) {
-				this._itemRowMap[i]--;
-			}
-		}
-	}
-	/*if (this.selection.isSelected(row)) {
-		this.selection.toggleSelect(row);
-	}*/
-}
-
-/*
- *  Returns a reference to the item at row (see Zotero.Item in data_access.js)
- */
-Zotero.ItemTreeView.prototype.getRow = function(row) {
-	return this._rows[row];
-}
-
 /*
  *  Create map of item ids to row indexes
  */
@@ -1946,7 +1868,7 @@ Zotero.ItemTreeView.prototype._refreshItemRowMap = function()
 		}
 		rowMap[id] = i;
 	}
-	this._itemRowMap = rowMap;
+	this._rowMap = rowMap;
 }
 
 
@@ -1972,8 +1894,8 @@ Zotero.ItemTreeView.prototype.rememberSelection = Zotero.Promise.coroutine(funct
 	}
 	for(var i=0; i < selection.length; i++)
 	{
-		if (this._itemRowMap[selection[i]] != null) {
-			this.selection.toggleSelect(this._itemRowMap[selection[i]]);
+		if (this._rowMap[selection[i]] != null) {
+			this.selection.toggleSelect(this._rowMap[selection[i]]);
 		}
 		// Try the parent
 		else {
@@ -1987,10 +1909,10 @@ Zotero.ItemTreeView.prototype.rememberSelection = Zotero.Promise.coroutine(funct
 				continue;
 			}
 			
-			if (this._itemRowMap[parent] != null) {
-				this._closeContainer(this._itemRowMap[parent]);
-				yield this.toggleOpenState(this._itemRowMap[parent]);
-				this.selection.toggleSelect(this._itemRowMap[selection[i]]);
+			if (this._rowMap[parent] != null) {
+				this._closeContainer(this._rowMap[parent]);
+				yield this.toggleOpenState(this._rowMap[parent]);
+				this.selection.toggleSelect(this._rowMap[selection[i]]);
 			}
 		}
 	}
@@ -2045,7 +1967,7 @@ Zotero.ItemTreeView.prototype._saveOpenState = function (close) {
 Zotero.ItemTreeView.prototype.rememberOpenState = Zotero.Promise.coroutine(function* (itemIDs) {
 	var rowsToOpen = [];
 	for each(var id in itemIDs) {
-		var row = this._itemRowMap[id];
+		var row = this._rowMap[id];
 		// Item may not still exist
 		if (row == undefined) {
 			continue;
@@ -2111,8 +2033,8 @@ Zotero.ItemTreeView.prototype.saveFirstRow = function() {
 
 
 Zotero.ItemTreeView.prototype.rememberFirstRow = function(firstRow) {
-	if (firstRow && this._itemRowMap[firstRow]) {
-		this._treebox.scrollToRow(this._itemRowMap[firstRow]);
+	if (firstRow && this._rowMap[firstRow]) {
+		this._treebox.scrollToRow(this._rowMap[firstRow]);
 	}
 }
 
@@ -2892,7 +2814,7 @@ Zotero.ItemTreeView.prototype.canDropCheck = function (row, orient, dataTransfer
 				
 				// Don't allow children to be dragged within their own parents
 				var parentItemID = item.parentItemID;
-				var parentIndex = this._itemRowMap[parentItemID];
+				var parentIndex = this._rowMap[parentItemID];
 				if (row != -1 && this.getLevel(row) > 0) {
 					if (this.getRow(this.getParentIndex(row)).ref.id == parentItemID) {
 						return false;
diff --git a/chrome/content/zotero/xpcom/libraryTreeView.js b/chrome/content/zotero/xpcom/libraryTreeView.js
index 112d66e20..a8cfba52c 100644
--- a/chrome/content/zotero/xpcom/libraryTreeView.js
+++ b/chrome/content/zotero/xpcom/libraryTreeView.js
@@ -28,6 +28,8 @@ Zotero.LibraryTreeView = function () {
 	this._listeners = {
 		load: []
 	};
+	this._rows = [];
+	this._rowMap = {};
 };
 
 Zotero.LibraryTreeView.prototype = {
@@ -52,6 +54,79 @@ Zotero.LibraryTreeView.prototype = {
 	}),
 	
 	
+	/**
+	 * Returns a reference to the tree row at a given row
+	 */
+	getRow: function(row) {
+		return this._rows[row];
+	},
+	
+	
+	/**
+	 * Add a tree row to the main array, update the row count, tell the treebox that the row
+	 * count changed, and update the row map
+	 *
+	 * @param {Array} newRows - Array to operate on
+	 * @param {Zotero.ItemTreeRow} itemTreeRow
+	 * @param {Number} [beforeRow] - Row index to insert new row before
+	 */
+	_addRow: function (treeRow, beforeRow) {
+		this._addRowToArray(this._rows, treeRow, beforeRow);
+		this.rowCount++;
+		this._treebox.rowCountChanged(beforeRow, 1);
+		// Increment all rows in map at or above insertion point
+		for (let i in this._rowMap) {
+			if (this._rowMap[i] >= beforeRow) {
+				this._rowMap[i]++
+			}
+		}
+		// Add new row to map
+		this._rowMap[treeRow.id] = beforeRow;
+	},
+	
+	
+	/**
+	 * Add a tree row into a given array
+	 *
+	 * @param {Array} array - Array to operate on
+	 * @param {Zotero.CollectionTreeRow|ItemTreeRow} treeRow
+	 * @param {Number} beforeRow - Row index to insert new row before
+	 */
+	_addRowToArray: function (array, treeRow, beforeRow) {
+		array.splice(beforeRow, 0, treeRow);
+	},
+	
+	
+	/**
+	* Remove a row from the main array, decrement the row count, tell the treebox that the row
+	* count changed, delete the row from the map, and optionally update all rows above it in the map
+	*/
+	_removeRow: function (row, skipMapUpdate) {
+		var id = this._rows[row].id;
+		this._rows.splice(row, 1);
+		this.rowCount--;
+		this._treebox.rowCountChanged(row + 1, -1);
+		delete this._rowMap[id];
+		if (!skipMapUpdate) {
+			for (let i in this._rowMap) {
+				if (this._rowMap[i] > row) {
+					this._rowMap[i]--;
+				}
+			}
+		}
+	},
+	
+	
+	getLevel: function (row) {
+		return this._rows[row].level;
+	},
+	
+	
+	isContainerOpen: function(row) {
+		return this._rows[row].isOpen;
+	},
+	
+	
 	/**
 	 *  Called while a drag is over the tree
 	 */
diff --git a/test/tests/collectionTreeViewTest.js b/test/tests/collectionTreeViewTest.js
index 1a32f1986..af2594e82 100644
--- a/test/tests/collectionTreeViewTest.js
+++ b/test/tests/collectionTreeViewTest.js
@@ -94,5 +94,36 @@ describe("Zotero.CollectionTreeView", function() {
 			selected = collectionsView.getSelectedCollection(true);
 			assert.equal(selected, id);
 		});
+		
+		it("should add a saved search after collections", function* () {
+			var collection = new Zotero.Collection;
+			collection.name = "Test";
+			var collectionID = yield collection.save();
+			var cv = win.ZoteroPane.collectionsView;
+			
+			var search = new Zotero.Search;
+			search.name = "A Test Search";
+			search.addCondition('title', 'contains', 'test');
+			var searchID = yield search.save();
+			
+			var collectionRow = cv._rowMap["C" + collectionID];
+			var searchRow = cv._rowMap["S" + searchID];
+			var duplicatesRow = cv._rowMap["D" + Zotero.Libraries.userLibraryID];
+			var unfiledRow = cv._rowMap["U" + Zotero.Libraries.userLibraryID];
+			
+			assert.isAbove(searchRow, collectionRow);
+			// If there's a duplicates row or an unfiled row, add before those.
+			// Otherwise, add before the trash
+			if (duplicatesRow !== undefined) {
+				assert.isBelow(searchRow, duplicatesRow);
+			}
+			else if (unfiledRow !== undefined) {
+				assert.isBelow(searchRow, unfiledRow);
+			}
+			else {
+				var trashRow = cv._rowMap["T" + Zotero.Libraries.userLibraryID];
+				assert.isBelow(searchRow, trashRow);
+			}
+		})
 	})
 })