Fixes #918, Enabling "Show Unfiled Items" or "Show Duplicates" breaks UI

chrome/content/zotero/zoteroPane.js

@@ -912,17 +912,16 @@ var ZoteroPane = new function()
 		
 		yield this.collectionsView.refresh();
 		
-		// If group is closed, open it
-		yield this.collectionsView.selectLibrary(libraryID);
-		row = this.collectionsView.selection.currentIndex;
-		if (!this.collectionsView.isContainerOpen(row)) {
-			this.collectionsView.toggleOpenState(row);
-		}
-		
 		// Select new row
 		if (show) {
 			yield this.collectionsView.selectByID(lastViewedFolderID);
 		}
+		// Select library root when hiding
+		else {
+			yield this.collectionsView.selectLibrary(libraryID);
+		}
+		
+		this.collectionsView.selection.selectEventsSuppressed = false;
 	});
 	
 	

test/content/support.js

@@ -251,6 +251,33 @@ function waitForCallback(cb, interval, timeout) {
 }
 
 
+function clickOnItemsRow(itemsView, row, button = 0) {
+	var x = {};
+	var y = {};
+	var width = {};
+	var height = {};
+	itemsView._treebox.getCoordsForCellItem(
+		row,
+		itemsView._treebox.columns.getNamedColumn('zotero-items-column-title'),
+		'text',
+		x, y, width, height
+	);
+	
+	// Select row to trigger multi-select
+	var tree = itemsView._treebox.treeBody;
+	var rect = tree.getBoundingClientRect();
+	var x = rect.left + x.value;
+	var y = rect.top + y.value;
+	tree.dispatchEvent(new MouseEvent("mousedown", {
+		clientX: x,
+		clientY: y,
+		button,
+		detail: 1
+	}));
+}
+
+
+
 /**
  * Get a default group used by all tests that want one, creating one if necessary
  */

test/tests/collectionTreeViewTest.js

@@ -22,8 +22,8 @@ describe("Zotero.CollectionTreeView", function() {
 			Zotero.Prefs.clear('duplicateLibraries');
 			Zotero.Prefs.clear('unfiledLibraries');
 			yield cv.refresh();
-			assert.isTrue(cv.getRowIndexByID("D" + userLibraryID));
+			assert.ok(cv.getRowIndexByID("D" + userLibraryID));
-			assert.isTrue(cv.getRowIndexByID("U" + userLibraryID));
+			assert.ok(cv.getRowIndexByID("U" + userLibraryID));
 			assert.equal(Zotero.Prefs.get('duplicateLibraries'), "" + userLibraryID);
 			assert.equal(Zotero.Prefs.get('unfiledLibraries'), "" + userLibraryID);
 		});

test/tests/duplicatesTest.js

@@ -32,31 +32,9 @@ describe("Duplicate Items", function () {
 			
 			// Select the first item, which should select both
 			var iv = zp.itemsView;
-			var index = iv.getRowIndexByID(item1.id);
+			var row = iv.getRowIndexByID(item1.id);
-			assert.isNumber(index);
+			assert.isNumber(row);
-			
+			clickOnItemsRow(iv, row);
-			var x = {};
-			var y = {};
-			var width = {};
-			var height = {};
-			iv._treebox.getCoordsForCellItem(
-				index,
-				iv._treebox.columns.getNamedColumn('zotero-items-column-title'),
-				'text',
-				x, y, width, height
-			);
-			
-			// Select row to trigger multi-select
-			var tree = iv._treebox.treeBody;
-			var rect = tree.getBoundingClientRect();
-			var x = rect.left + x.value;
-			var y = rect.top + y.value;
-			tree.dispatchEvent(new MouseEvent("mousedown", {
-				clientX: x,
-				clientY: y,
-				detail: 1
-			}));
-			
 			assert.equal(iv.selection.count, 2);
 			
 			// Click merge button

test/tests/zoteroPaneTest.js

@@ -217,6 +217,11 @@ describe("ZoteroPane", function() {
 		})
 		
 		it("should show a hidden virtual folder", function* () {
+			// Create unfiled, duplicate items
+			var title = Zotero.Utilities.randomString();
+			var item1 = yield createDataObject('item', { title });
+			var item2 = yield createDataObject('item', { title });
+			
 			// Start hidden
 			Zotero.Prefs.set('duplicateLibraries', "");
 			Zotero.Prefs.set('unfiledLibraries', "");
@@ -226,7 +231,17 @@ describe("ZoteroPane", function() {
 			var id = "D" + userLibraryID;
 			assert.isFalse(cv.getRowIndexByID(id));
 			yield zp.setVirtual(userLibraryID, 'duplicates', true);
-			assert.ok(cv.getRowIndexByID(id));
+			
+			// Clicking should select both items
+			var row = cv.getRowIndexByID(id);
+			assert.ok(row);
+			assert.equal(cv.selection.currentIndex, row);
+			yield waitForItemsLoad(win);
+			var iv = zp.itemsView;
+			row = iv.getRowIndexByID(item1.id);
+			assert.isNumber(row);
+			clickOnItemsRow(iv, row);
+			assert.equal(iv.selection.count, 2);
 			
 			// Show Unfiled Items
 			id = "U" + userLibraryID;
@@ -254,20 +269,25 @@ describe("ZoteroPane", function() {
 		it("should hide an explicitly shown virtual folder", function* () {
 			// Start shown
 			Zotero.Prefs.set('duplicateLibraries', "" + userLibraryID);
-			Zotero.Prefs.set('unfiledLibraries' "" + userLibraryID);
+			Zotero.Prefs.set('unfiledLibraries', "" + userLibraryID);
 			yield cv.refresh();
 			
 			// Hide Duplicate Items
 			var id = "D" + userLibraryID;
 			assert.ok(yield cv.selectByID(id));
+			yield waitForItemsLoad(win);
 			yield zp.setVirtual(userLibraryID, 'duplicates', false);
 			assert.isFalse(cv.getRowIndexByID(id));
+			assert.equal(cv.getSelectedLibraryID(), userLibraryID);
+			
 			
 			// Hide Unfiled Items
 			id = "U" + userLibraryID;
 			assert.ok(yield cv.selectByID(id));
+			yield waitForItemsLoad(win);
 			yield zp.setVirtual(userLibraryID, 'unfiled', false);
 			assert.isFalse(cv.getRowIndexByID(id));
+			assert.equal(cv.getSelectedLibraryID(), userLibraryID);
 		});
 	});
 })