From 2c25257e2bcf218befeb1261184ea465180814f8 Mon Sep 17 00:00:00 2001
From: Dan Stillman <dstillman@zotero.org>
Date: Thu, 24 Sep 2015 07:11:12 -0400
Subject: [PATCH] Don't scroll to containing collection if one is already
 visible

If multiple collections are highlighted and none are in view, scroll to
the first one in the list.

The logic could be improved here a little more to scroll to the closest
collection instead of the first one, and also to scroll to a few rows
above or below the target.

This also fixes what was probably an incorrect highlight if there were
multiple collections and some had to be expanded first.
---
 .../zotero/xpcom/collectionTreeView.js        | 37 +++++++++++++++----
 1 file changed, 29 insertions(+), 8 deletions(-)

diff --git a/chrome/content/zotero/xpcom/collectionTreeView.js b/chrome/content/zotero/xpcom/collectionTreeView.js
index cc9af03de..e134111df 100644
--- a/chrome/content/zotero/xpcom/collectionTreeView.js
+++ b/chrome/content/zotero/xpcom/collectionTreeView.js
@@ -384,17 +384,38 @@ Zotero.CollectionTreeView.prototype.setHighlightedRows = function (ids) {
 	this._highlightedRows = {};
 	this._treebox.invalidate();
 	
-	var scrolled = false;
-	for each(var id in ids) {
-		let row = this._collectionRowMap[id];
+	if (!ids || !ids.length) {
+		return;
+	}
+	
+	// Make sure all highlighted collections are shown
+	for (let id of ids) {
 		this.expandToCollection(id);
-		// TODO: Scroll a little above or below
-		if (!scrolled) {
-			this._treebox.ensureRowIsVisible(row);
-			scrolled = true;
-		}
+	}
+	
+	// Highlight rows
+	var rows = [];
+	for (let id of ids) {
+		let row = this._collectionRowMap[id];
 		this._highlightedRows[row] = true;
 		this._treebox.invalidateRow(row);
+		rows.push(row);
+	}
+	rows.sort();
+	var firstRow = this._treebox.getFirstVisibleRow();
+	var lastRow = this._treebox.getLastVisibleRow();
+	var scrolled = false;
+	for (let row of rows) {
+		// If row is visible, stop
+		if (row >= firstRow && row <= lastRow) {
+			scrolled = true;
+			break;
+		}
+	}
+	// Select first collection
+	// TODO: Select closest? Select a few rows above or below?
+	if (!scrolled) {
+		this._treebox.ensureRowIsVisible(rows[0]);
 	}
 }