Item tree refactor megacommit

Split ItemTree megaclass into:
- ItemTree - concerned with drawing the virtualized table container and
  column interaction
- ItemTreeRowProvider - provides rows and issues notifications for
  render updates
- ItemTreeRow and subclasses - contains row-specific data and rendering
  logic
- CollectionViewItemTree and its accompanying classes - a version of
  ItemTree that renders items attached to a given Collection or
  CollectionView (CollectionTreeRow).

Various improvements in logic and rendering, separation of concerns.

chrome/content/zotero/advancedSearch.js

@@ -24,7 +24,7 @@
 */
 
 
-import ItemTree from 'zotero/itemTree';
+import CollectionViewItemTree from 'zotero/collectionViewItemTree';
 import { COLUMNS } from 'zotero/itemTreeColumns';
 
 
@@ -39,6 +39,7 @@ var ZoteroAdvancedSearch = new function () {
 
 	var _searchBox;
 	var _libraryID;
+	var _searchCounter = 0;
 	
 	async function onLoad() {
 		_searchBox = document.getElementById('zotero-search-box');
@@ -61,16 +62,16 @@ var ZoteroAdvancedSearch = new function () {
 			column.hidden = !['title', 'firstCreator', 'year', 'hasAttachment'].includes(column.dataKey);
 			return column;
 		});
-		this.itemsView = await ItemTree.init(elem, {
+		this.itemsView = await CollectionViewItemTree.init(elem, {
 			id: "advanced-search",
 			dragAndDrop: true,
-			persistColumns: true,
 			columnPicker: true,
 			onActivate: this.onItemActivate.bind(this),
 			columns,
 		});
 
 		await this.itemsView.changeCollectionTreeRow({
+			id: 'advanced-search-' + _searchCounter++,
 			ref: _searchBox.search,
 			visibilityGroup: 'default',
 			isSearchMode: () => true,
@@ -103,7 +104,8 @@ var ZoteroAdvancedSearch = new function () {
 		_searchBox.updateSearch();
 		_searchBox.active = true;
 		
-		var collectionTreeRow = {
+		return this.itemsView.changeCollectionTreeRow({
+			id: 'advanced-search-' + _searchCounter++,
 			ref: _searchBox.search,
 			visibilityGroup: 'default',
 			isSearchMode: () => true,
@@ -115,21 +117,8 @@ var ZoteroAdvancedSearch = new function () {
 				search.libraryID = _libraryID;
 				var ids = await search.search();
 				return Zotero.Items.get(ids);
-			},
+			}
-			isLibrary: () => false,
+		});
-			isCollection: () => false,
-			isPublications: () => false,
-			isDuplicates: () => false,
-			isFeed: () => false,
-			isFeeds: () => false,
-			isFeedsOrFeed: () => false,
-			isRecentlyRead: () => false,
-			isSortable: () => true,
-			isShare: () => false,
-			isTrash: () => false
-		};
-
-		return this.itemsView.changeCollectionTreeRow(collectionTreeRow);
 	}
 	
 	

chrome/content/zotero/collectionTree.jsx

@@ -26,7 +26,7 @@
 const React = require('react');
 const ReactDOM = require('react-dom');
 const LibraryTree = require('./libraryTree');
-const VirtualizedTable = require('components/virtualized-table');
+const VirtualizedTree = require('components/virtualized-table').VirtualizedTree;
 const { getCSSIcon } = require('components/icons');
 const { getDragTargetOrient } = require('components/utils');
 const { noop } = require("./components/utils");
@@ -70,6 +70,8 @@ var CollectionTree = class CollectionTree extends LibraryTree {
 		this.type = 'collection';
 		this.name = "CollectionTree";
 		this.id = "collection-tree";
+		this._rows = [];
+		this._rowMap = {};
 		this._highlightedRows = new Set();
 		this._unregisterID = Zotero.Notifier.registerObserver(
 			this,
@@ -300,12 +302,6 @@ var CollectionTree = class CollectionTree extends LibraryTree {
 		// Div creation and content
 		let div = oldDiv || document.createElement('div');
 		div.innerHTML = "";
-		// When a hidden focused row is added last during filtering, it
-		// is removed on focus change, which can happen at the same time as rendering.
-		// In this case, just return empty div.
-		if (index >= this._rows.length) {
-			return div;
-		}
 		
 		// Classes
 		div.className = "row";
@@ -463,7 +459,7 @@ var CollectionTree = class CollectionTree extends LibraryTree {
 	}
 	
 	render() {
-		return React.createElement(VirtualizedTable,
+		return React.createElement(VirtualizedTree,
 			{
 				getRowCount: () => this._rows.length,
 				id: this.id,
@@ -478,7 +474,7 @@ var CollectionTree = class CollectionTree extends LibraryTree {
 				isContainer: this.isContainer,
 				isContainerEmpty: this.isContainerEmpty,
 				isContainerOpen: this.isContainerOpen,
-				toggleOpenState: this.toggleOpenState,
+				onToggleOpenState: this.toggleOpenState,
 				getRowString: this.getRowString.bind(this),
 				
 				onItemContextMenu: (...args) => this.props.onContextMenu && this.props.onContextMenu(...args),
@@ -486,7 +482,6 @@ var CollectionTree = class CollectionTree extends LibraryTree {
 				onKeyDown: this.handleKeyDown,
 				onActivate: (...args) => (this.props.onActivate ? this.props.onActivate(...args) : this.handleActivate(...args)),
 
-				role: 'tree',
 				label: Zotero.getString('pane.collections.title')
 			}
 		);

chrome/content/zotero/components/virtualized-table.jsx

@@ -143,9 +143,10 @@ class TreeSelection {
 	 * @returns {boolean} False if nothing to select and select handlers won't be called
 	 */
 	select(index, shouldDebounce) {
-		if (!this._tree.props.isSelectable(index)) return;
 		index = Math.max(0, index);
+		if (!this._tree.props.isSelectable(index)) return;
 		if (this.selected.size == 1 && this.isSelected(index)) {
+			this._updateTree(shouldDebounce);
 			return false;
 		}
 
@@ -161,7 +162,12 @@ class TreeSelection {
 		this._tree.scrollToRow(index);
 		this._updateTree(shouldDebounce);
 		if (this._tree.invalidate) {
-			toInvalidate.forEach(this._tree.invalidateRow.bind(this._tree));
+			const rowCount = this._tree.props.getRowCount();
+			toInvalidate.forEach((idx) => {
+				// this._updateTree() may change row count
+				if (idx >= rowCount) return;
+				this._tree.invalidateRow(idx);
+			});
 		}
 		return true;
 	}
@@ -272,8 +278,9 @@ class TreeSelection {
 	}
 
 	set selectEventsSuppressed(val) {
+		let valChanged = val !== this._selectEventsSuppressed;
 		this._selectEventsSuppressed = val;
-		if (!val) {
+		if (!val && valChanged) {
 			this._updateTree();
 			if (this._tree.invalidate) {
 				this._tree.invalidate();
@@ -323,6 +330,8 @@ class VirtualizedTable extends React.Component {
 		this._typingString = "";
 		this._jsWindowID = `virtualized-table-list-${Zotero.Utilities.randomString(5)}`;
 		this._containerWidth = props.containerWidth || window.innerWidth;
+		this.className = props.className || "";
+		this.firstColumnExtraWidth = props.firstColumnExtraWidth || 0;
 		
 		this._columns = new Columns(this);
 		
@@ -362,6 +371,8 @@ class VirtualizedTable extends React.Component {
 		staticColumns: false,
 		alternatingRowColors: Zotero.isMac ? ['-moz-OddTreeRow', '-moz-EvenTreeRow'] : null,
 
+		firstColumnExtraWidth: 0,
+
 		// Render with display: none
 		hide: false,
 
@@ -419,6 +430,8 @@ class VirtualizedTable extends React.Component {
 		staticColumns: PropTypes.bool,
 		// Used for initial column widths calculation
 		containerWidth: PropTypes.number,
+		// If first column is injected with extra stuff, like an item icon
+		// and we need to reserve extra min-width for it, set this prop
 		firstColumnExtraWidth: PropTypes.number,
 
 		// Internal windowed-list ref
@@ -640,7 +653,7 @@ class VirtualizedTable extends React.Component {
 			if (this.props.isContainer(this.selection.focused)
 					&& !this.props.isContainerEmpty(this.selection.focused)
 					&& this.props.isContainerOpen(this.selection.focused)) {
-				this.props.toggleOpenState(this.selection.focused);
+				this.toggleOpenState(this.selection.focused);
 			}
 			else if (parentIndex != -1) {
 				this.onSelection(parentIndex);
@@ -651,7 +664,7 @@ class VirtualizedTable extends React.Component {
 			if (this.props.isContainer(this.selection.focused)
 					&& !this.props.isContainerEmpty(this.selection.focused)) {
 				if (!this.props.isContainerOpen(this.selection.focused)) {
-					this.props.toggleOpenState(this.selection.focused);
+					this.toggleOpenState(this.selection.focused);
 				}
 				else {
 					this.onSelection(this.selection.focused + 1);
@@ -864,10 +877,7 @@ class VirtualizedTable extends React.Component {
 		event.stopPropagation();
 		const result = this._getResizeColumns();
 		if (!result) return;
-		const columns = this._getVisibleColumns();
 		const [aColumn, bColumn, resizingColumn] = result;
-		const isFirstColumn = columns[0].dataKey === aColumn.dataKey;
-		const firstColumnExtraWidth = isFirstColumn ? (this.props.firstColumnExtraWidth || 0) : 0;
 		const a = document.querySelector(`#${this.props.id} .virtualized-table-header .cell.${window.CSS.escape(aColumn.dataKey)}`);
 		const b = document.querySelector(`#${this.props.id} .virtualized-table-header .cell.${window.CSS.escape(bColumn.dataKey)}`);
 		const resizing = document.querySelector(`#${this.props.id} .virtualized-table-header .cell.${window.CSS.escape(resizingColumn.dataKey)}`);
@@ -881,9 +891,12 @@ class VirtualizedTable extends React.Component {
 		const widthSum = aRect.width + bRect.width;
 		const aColumnPadding = aColumn.iconLabel ? 0 : COLUMN_PADDING;
 		const bColumnPadding = bColumn.iconLabel ? 0 : COLUMN_PADDING;
-		const aSpacingOffset = (aColumn.minWidth ? aColumn.minWidth : COLUMN_MIN_WIDTH) + aColumnPadding + firstColumnExtraWidth;
+		const aMinWidth = (aColumn.minWidth ? aColumn.minWidth : COLUMN_MIN_WIDTH) + aColumnPadding;
-		const bSpacingOffset = (bColumn.minWidth ? bColumn.minWidth : COLUMN_MIN_WIDTH) + bColumnPadding;
+		const bMinWidth = (bColumn.minWidth ? bColumn.minWidth : COLUMN_MIN_WIDTH) + bColumnPadding;
-		const aColumnWidth = Math.min(widthSum - bSpacingOffset, Math.max(aSpacingOffset, event.clientX - (RESIZER_WIDTH / 2) - offset));
+		const aMaxWidth = widthSum - bMinWidth;
+		const aDragWidth = event.clientX - (RESIZER_WIDTH / 2) - offset;
+		// Constrain the drag position to the min and max widths
+		const aColumnWidth = Math.min(aMaxWidth, Math.max(aMinWidth, aDragWidth));
 		const bColumnWidth = widthSum - aColumnWidth;
 		let onResizeData = {};
 		onResizeData[aColumn.dataKey] = aColumnWidth;
@@ -1055,7 +1068,7 @@ class VirtualizedTable extends React.Component {
 	
 		this._setXulTooltip();
 
-		this._topDiv.style.setProperty("--firstColumnExtraWidth", `${this.props.firstColumnExtraWidth || 0}px`);
+		this._topDiv.style.setProperty("--first-column-extra-width", `${this.firstColumnExtraWidth}px`);
 		window.addEventListener("resize", () => {
 			this._debouncedRerender();
 		});
@@ -1113,13 +1126,13 @@ class VirtualizedTable extends React.Component {
 		return {
 			getItemCount: this.props.getRowCount,
 			itemHeight: this._rowHeight,
-			renderItem: this._renderItem,
+			renderItem: this._renderItem.bind(this),
 			targetElement: document.getElementById(this._jsWindowID),
 			customRowHeights: this.props.customRowHeights ?? []
 		};
 	}
 
-	_renderItem = (index, oldElem = null) => {
+	_renderItem(index, oldElem = null) {
 		let node = this.props.renderItem(index, this.selection, oldElem, this._getColumns());
 		if (!node.dataset.eventHandlersAttached) {
 			node.dataset.eventHandlersAttached = true;
@@ -1184,7 +1197,7 @@ class VirtualizedTable extends React.Component {
 			if (!column.iconLabel && column.sortDirection) {
 				sortIndicator = <CSSIcon name="sort-indicator" className={"icon-8 sort-indicator " + (column.sortDirection === 1 ? "ascending" : "descending")} />;
 			}
-			const className = cx("cell", column.className, { 'first-column': index === 0, dragging: this.state.draggingColumn == index },
+			const className = cx("cell", column.className, { dragging: this.state.draggingColumn == index },
 				{ "cell-icon": !!column.iconLabel });
 			return (<Draggable
 				onDragStart={this._handleColumnDragStart.bind(this, index)}
@@ -1238,7 +1251,9 @@ class VirtualizedTable extends React.Component {
 				{
 				resizing: this.state.resizing,
 				'multi-select': this.props.multiSelect
-			}]),
+			},
+				this.className
+			]),
 			id: this.props.id,
 			ref: ref => this._topDiv = ref,
 			tabIndex: 0,
@@ -1408,6 +1423,11 @@ class VirtualizedTable extends React.Component {
 			&& row <= this._jsWindow.getLastVisibleRow();
 	}
 
+	toggleOpenState(index, ...args) {
+		let onToggleOpenState = this.props.toggleOpenState;
+		if (typeof onToggleOpenState == 'function') return onToggleOpenState(index, ...args);
+	}
+
 	async _resetColumns() {
 		this.invalidate();
 		this._columns = new Columns(this);
@@ -1424,6 +1444,143 @@ class VirtualizedTable extends React.Component {
 	}
 }
 
+/**
+ * VirtualizedTree wraps VirtualizedTable to provide common tree affordances:
+ * - Adds an indent spacer based on depth to the first visible cell
+ * - Adds a twisty for non-empty containers
+ * - Sets tree-specific ARIA attributes on rows and the container
+ * - Wires twisty mouse handlers to toggle container open state
+ *
+ * Consumers should provide isContainer/isContainerEmpty/isContainerOpen/onToggleOpenState
+ * and getParentIndex(index) to compute ancestry.
+ */
+class VirtualizedTree extends VirtualizedTable {
+	static propTypes = { ...VirtualizedTable.propTypes,
+		getParentIndex: PropTypes.func.isRequired,
+		isContainer: PropTypes.func.isRequired,
+		isContainerEmpty: PropTypes.func.isRequired,
+		isContainerOpen: PropTypes.func.isRequired,
+		onToggleOpenState: PropTypes.func.isRequired,
+	}
+
+	_toggledOpenStateIndex = null;
+
+	constructor(props) {
+		super(props);
+		this.className += " virtualized-tree";
+		this.firstColumnExtraWidth += 16; // 16px for twisty
+	}
+
+	toggleOpenState(index, ...args) {
+		this._toggledOpenStateIndex = index;
+		return this.props.onToggleOpenState(index, ...args);
+	}
+
+	_renderItem(index, oldElem=null) {
+		let node = super._renderItem(index, oldElem);
+		if (!(node instanceof (node?.ownerDocument?.defaultView || window).Element)) {
+			return node;
+		}
+		node = this._addIndentAndTwisty(node, index);
+		this._setRowAria(node, index);
+		return node;
+	}
+
+	_getDepth(index) {
+		let depth = 0;
+		try {
+			let parent = typeof this.props.getParentIndex == 'function' ? this.props.getParentIndex(index) : -1;
+			while (parent != -1 && typeof parent == 'number') {
+				depth++;
+				parent = this.props.getParentIndex(parent);
+			}
+		}
+		catch (e) {}
+		return depth;
+	}
+	
+	/**
+	 * Adds an indent spacer and twisty to the first cell of the node
+	 * 
+	 * We add it to the first cell instead of as a separate pseudo-cell or just elements before
+	 * the first cell because otherwise it messes with column spacing.
+	 * 
+	 * @param node {HTMLElement} The rendered item row
+	 * @param index {number} The index of the node being rendered
+	 * @returns {HTMLElement}
+	 */
+	_addIndentAndTwisty(node, index) {
+		let firstCell = node.querySelector('.cell');
+		if (!firstCell) return node;
+
+		let twisty;
+		if (this.props.isContainerEmpty(index)) {
+			twisty = firstCell.querySelector('.spacer-twisty');
+			if (!twisty) {
+				twisty = node.ownerDocument.createElement('span');
+				firstCell.prepend(twisty);
+				twisty.classList.add('spacer-twisty');
+			}
+			firstCell.querySelector(`:scope > .twisty`)?.remove();
+		}
+		else {
+			twisty = firstCell.querySelector('.twisty');
+			if (!twisty) {
+				twisty = getCSSIcon('twisty');
+				twisty.classList.add('twisty');
+				twisty.style.pointerEvents = 'auto';
+				twisty.addEventListener('mousedown', (event) => event.stopPropagation());
+				twisty.addEventListener('mouseup', (event) => {
+					this.toggleOpenState(index);
+					event.stopPropagation();
+				}, { passive: true });
+				twisty.addEventListener('dblclick', (event) => event.stopImmediatePropagation(), { passive: true });
+				firstCell.prepend(twisty);
+			}
+			firstCell.querySelector(`:scope > .spacer-twisty`)?.remove();
+
+			// Apply the twisty animation
+			if (this._toggledOpenStateIndex == index) {
+				twisty.classList.toggle('open', !this.props.isContainerOpen(index));
+				requestAnimationFrame(() => {
+					twisty.classList.toggle('open', this.props.isContainerOpen(index));
+					this._toggledOpenStateIndex = null;
+				});
+			}
+			else {
+				twisty.classList.toggle('open', this.props.isContainerOpen(index));
+			}
+		}
+
+		let indentSpan = firstCell.querySelector('.cell-indent');
+		if (!indentSpan) {
+			indentSpan = node.ownerDocument.createElement('span');
+			indentSpan.className = 'cell-indent';
+			firstCell.prepend(indentSpan);
+		}
+		// Use padding for indent similar to ItemTree
+		const CHILD_INDENT = 16;
+		indentSpan.style.paddingInlineStart = (CHILD_INDENT * this._getDepth(index)) + 'px';
+
+		return node;
+	}
+
+	_setRowAria(node, index) {
+		const depth = this._getDepth(index);
+		node.setAttribute('role', 'treeitem');
+		node.setAttribute('aria-level', depth + 1);
+		if (!this.props.isContainerEmpty(index)) {
+			node.setAttribute('aria-expanded', !!this.props.isContainerOpen(index));
+		}
+		else {
+			node.removeAttribute('aria-expanded');
+		}
+	}
+}
+
+VirtualizedTree.propTypes = Object.assign({}, VirtualizedTable.propTypes);
+VirtualizedTree.defaultProps = Object.assign({}, VirtualizedTable.defaultProps, { role: 'tree' });
+
 /**
  * Create a function that calls the given function `fn` only once per animation
  * frame.
@@ -1514,7 +1671,6 @@ var Columns = class {
 		// Storing back persist settings to account for legacy upgrades
 		this._storePrefs(columnsSettings);
 
-		this._adjustColumnWidths();
 		// Set column width CSS rules
 		this.onResize(columnWidths);
 		// Whew, all this just to get a list of columns
@@ -1581,24 +1737,6 @@ var Columns = class {
 		this._virtualizedTable.props.storeColumnPrefs(prefs);
 	}
 
-	_adjustColumnWidths = () => {
-		if (!this._virtualizedTable.props.firstColumnExtraWidth) {
-			return;
-		}
-
-		const extraWidth = this._virtualizedTable.props.firstColumnExtraWidth;
-		this._columns.filter(c => !c.hidden).forEach((column, index) => {
-			const isFirstColumn = index === 0;
-			if (column.fixedWidth) {
-				column.width = isFirstColumn ? parseInt(column.originalWidth) + extraWidth : column.originalWidth;
-			}
-			if (column.staticWidth) {
-				column.minWidth = isFirstColumn ? (column.originalMinWidth ?? 20) + extraWidth : column.originalMinWidth;
-				column.width = isFirstColumn ? Math.max(parseInt(column.width) ?? 0, column.minWidth) : column.width;
-			}
-		});
-	};
-
 	/**
 	 * Programatically sets the injected CSS width rules for each column.
 	 * This is necessary for performance reasons
@@ -1610,11 +1748,14 @@ var Columns = class {
 			var prefs = this._getPrefs();
 		}
 
+		let visibleColumns = this.getAsArray().filter(column => !column.hidden);
+
 		for (let [dataKey, width] of Object.entries(columnWidths)) {
 			if (typeof dataKey == "number") {
 				dataKey = this._columns[dataKey].dataKey;
 			}
 			const column = this._columns.find(column => column.dataKey == dataKey);
+			if (column.hidden) continue;
 			const styleIndex = this._columnStyleMap[window.CSS.escape(dataKey)];
 			const columnPadding = column.iconLabel ? 0 : COLUMN_PADDING;
 			if (storePrefs && !column.fixedWidth) {
@@ -1626,12 +1767,16 @@ var Columns = class {
 			}
 			if (column.fixedWidth && column.width || column.staticWidth) {
 				this._stylesheet.sheet.cssRules[styleIndex].style.setProperty('flex', `0 0`, `important`);
-				this._stylesheet.sheet.cssRules[styleIndex].style.setProperty('max-width', `${width}px`, 'important');
+				this._stylesheet.sheet.cssRules[styleIndex].style.setProperty('max-width', `calc(var(--extra-width, 0px) + ${width}px`, 'important');
-				this._stylesheet.sheet.cssRules[styleIndex].style.setProperty('min-width', `${width}px`, 'important');
+				this._stylesheet.sheet.cssRules[styleIndex].style.setProperty('min-width', `calc(var(--extra-width, 0px) + ${width}px`, 'important');
 			} else {
+				// It's set in CSS, so we subtract it here to prevent sliding
+				if (column.dataKey === visibleColumns[0].dataKey) {
+					width -= this._virtualizedTable.firstColumnExtraWidth;
+				}
 				width = (width - columnPadding);
 				Zotero.debug(`Columns ${dataKey} width ${width}`);
-				this._stylesheet.sheet.cssRules[styleIndex].style.setProperty('flex-basis', `${width}px`);
+				this._stylesheet.sheet.cssRules[styleIndex].style.setProperty('flex-basis', `calc(var(--extra-width, 0px) + ${width}px`);
 			}
 		}
 		if (storePrefs) {
@@ -1649,7 +1794,6 @@ var Columns = class {
 			return a.ordinal - b.ordinal;
 		});
 
-		this._adjustColumnWidths();
 		this.onResize(Object.fromEntries(this._columns.map(c => [c.dataKey, c.width])));
 
 		let prefs = this._getPrefs();
@@ -1689,7 +1833,6 @@ var Columns = class {
 			this._columns.find(c => c.dataKey === 'title').hidden = false;
 		}
 		
-		this._adjustColumnWidths();
 		this.onResize(Object.fromEntries(this._columns.map(c => [c.dataKey, c.width])));
 		this._storePrefs(prefs);
 		this._updateVirtualizedTable();
@@ -1699,16 +1842,36 @@ var Columns = class {
 		const column = this._columns[index];
 		column.hidden = !column.hidden;
 
+		if (!column.hidden && !column.width) {
+			column.width = this._computeFlexWidth(column);
+		}
+
 		let prefs = this._getPrefs();
 		if (prefs[column.dataKey]) {
 			prefs[column.dataKey].hidden = column.hidden;
 		}
-		this._adjustColumnWidths();
 		this.onResize(Object.fromEntries(this._columns.map(c => [c.dataKey, c.width])));
 		this._storePrefs(prefs);
 		this._updateVirtualizedTable();
 	}
 
+	_computeFlexWidth(column) {
+		const containerWidth = this._virtualizedTable._containerWidth;
+		const visibleColumns = this._columns.filter(c => !c.hidden);
+		let fixedWidth = 0;
+		let totalFlex = 0;
+		for (let col of visibleColumns) {
+			if (col.fixedWidth || col.staticWidth || !col.flex) {
+				fixedWidth += parseFloat(col.width) || col.minWidth || 0;
+			}
+			else {
+				totalFlex++;
+			}
+		}
+		let availableWidth = containerWidth - fixedWidth;
+		return availableWidth / totalFlex * (column.flex || 1);
+	}
+	
 	toggleSort(sortIndex) {
 		if (!this._virtualizedTable.props.onColumnSort) return;
 		
@@ -1727,8 +1890,9 @@ var Columns = class {
 				}
 			}
 		});
-		this._virtualizedTable.props.onColumnSort(sortIndex, sortedColumn.sortDirection);
+		let result = this._virtualizedTable.props.onColumnSort(sortIndex, sortedColumn.sortDirection);
 		this._virtualizedTable.forceUpdate();
+		return result;
 	}
 
 	getAsArray() {
@@ -1738,8 +1902,8 @@ var Columns = class {
 
 function renderCell(index, data, column, dir = null) {
 	column = column || { dataKey: "" };
-	if (column.renderer) {
+	if (column.renderCell) {
-		return column.renderer(index, data, column, dir);
+		return column.renderCell(index, data, column, dir);
 	}
 	let span = document.createElement('span');
 	span.className = `cell ${column.className}`;
@@ -1881,6 +2045,8 @@ function formatColumnName(column) {
 }
 
 module.exports = VirtualizedTable;
+module.exports.VirtualizedTree = VirtualizedTree;
+
 module.exports.TreeSelection = TreeSelection;
 module.exports.TreeSelectionStub = TreeSelectionStub;
 module.exports.renderCell = renderCell;

chrome/content/zotero/components/windowed-list.js

@@ -214,11 +214,18 @@ module.exports = class {
 		index = Math.max(0, Math.min(index, itemCount - 1));
 		let startPosition = this._getItemPosition(index);
 		let endPosition = this._getItemPosition(index + 1);
+		// If forceScrollToTop is set, always scroll to the start position even if the row is
+		// already visible. This is used when restoring scroll position, where we need an exact
+		// first-visible-row rather than just ensuring the row is within view.
+		if (forceScrollToTop) {
+			this.scrollTo(startPosition);
+			return;
+		}
 		if (startPosition < scrollOffset) {
 			this.scrollTo(startPosition);
 		}
 		else if (endPosition > scrollOffset + height) {
-			this.scrollTo(forceScrollToTop ? startPosition : endPosition - height - 1);
+			this.scrollTo(endPosition - height - 1);
 		}
 	}
 	

chrome/content/zotero/containers/tagSelectorContainer.jsx

@@ -114,6 +114,27 @@ Zotero.TagSelector = class TagSelectorContainer extends React.PureComponent {
 		return null;
 	}
 	
+	/**
+	 * Safely fetch tags from the current collection tree row, returning [] on search error.
+	 * CollectionTreeRow.getTags() calls getSearchResults() under the hood, which throws
+	 * Zotero.CollectionTreeRow.SearchError if the underlying search query fails (e.g., a
+	 * saved search with invalid conditions). The tag selector should degrade gracefully in
+	 * that case — showing no tags — rather than throwing upwards and breaking the UI.
+	 * Real bugs (TypeError, etc.) are re-thrown so they surface in tests and logs.
+	 */
+	async _safeGetTags(...args) {
+		try {
+			return await this.collectionTreeRow.getTags(...args);
+		}
+		catch (e) {
+			if (e instanceof Zotero.CollectionTreeRow.SearchError) {
+				Zotero.logError(e);
+				return [];
+			}
+			throw e;
+		}
+	}
+
 	// Update trigger #1 (triggered by ZoteroPane)
 	async onItemViewChanged({ collectionTreeRow, libraryID }) {
 		Zotero.debug('Updating tag selector from current view');
@@ -192,7 +213,7 @@ Zotero.TagSelector = class TagSelectorContainer extends React.PureComponent {
 			}
 			// Check tags for each tag type to see if they're in view/scope
 			for (let [type, tagIDs] of tagsByType) {
-				changedTagsInScope.push(...await this.collectionTreeRow.getTags([type], tagIDs));
+				changedTagsInScope.push(...await this._safeGetTags([type], tagIDs));
 				if (this.displayAllTags) {
 					changedTagsInView.push(
 						...await Zotero.Tags.getAllWithin({ libraryID: this.libraryID, tagIDs })
@@ -315,7 +336,7 @@ Zotero.TagSelector = class TagSelectorContainer extends React.PureComponent {
 	}
 	
 	async getTagsAndScope() {
-		var tags = await this.collectionTreeRow.getTags();
+		var tags = await this._safeGetTags();
 		// The scope is all visible tags, not all tags in the library
 		var scope = new Set(tags.map(t => t.tag));
 		if (this.displayAllTags) {

chrome/content/zotero/elements/duplicatesMergePane.js

@@ -179,7 +179,6 @@
 		
 		async merge() {
 			let itembox = document.getElementById('zotero-duplicates-merge-info-box');
-			Zotero.CollectionTreeCache.clear();
 			// Update master item with any field alternatives from the item box
 			let json = this._masterItem.toJSON();
 			// Exclude certain properties that are empty in the cloned object, so we don't clobber them

chrome/content/zotero/integration/citationDialog.js

@@ -24,7 +24,7 @@
 */
 
 
-const ItemTree = require('zotero/itemTree');
+const CollectionViewItemTree = require('zotero/collectionViewItemTree');
 const { getCSSIcon } = require('components/icons');
 const { COLUMNS } = require('zotero/itemTreeColumns');
 var doc, io, ioReadyPromise, ioIsReady, accepted;
@@ -660,10 +660,11 @@ class LibraryLayout extends Layout {
 			label: columnLabel,
 			htmlLabel: ' ', // space for column label to appear empty
 			width: 26,
+			hidden: false,
 			staticWidth: true,
 			fixedWidth: true,
 			showInColumnPicker: false,
-			renderer: (index, inCitation, column) => {
+			renderCell: (index, inCitation, column) => {
 				let cell = Helpers.createNode("span", {}, `cell ${column.className} clickable`);
 				if (inCitation === null) {
 					// no icon should be shown when an item cannot be added
@@ -688,7 +689,7 @@ class LibraryLayout extends Layout {
 				return cell;
 			}
 		});
-		this.itemsView = await ItemTree.init(itemsTree, {
+		this.itemsView = await CollectionViewItemTree.init(itemsTree, {
 			id: "citationDialog",
 			dragAndDrop: DIALOG_STATE.isCitingItems(),
 			persistColumns: true,
@@ -717,7 +718,7 @@ class LibraryLayout extends Layout {
 				if (!isClick) {
 					let lastItemID = items[items.length - 1].id;
 					let rowIndex = this.itemsView.getRowIndexByID(lastItemID);
-					row = doc.querySelector(`#item-tree-citationDialog-row-${rowIndex}`) || row;
+					row = doc.getElementById(`${this.itemsView.id}-row-${rowIndex}`) || row;
 				}
 				let rowTopBeforeRefresh = row.getBoundingClientRect().top;
 				IOManager.addItemsToCitation(items, { noInputRefocus: true }).then(() => {
@@ -848,9 +849,14 @@ class LibraryLayout extends Layout {
 			id: collectionTreeRow.id,
 			getItems: async () => {
 				let items = await collectionTreeRow.getItems();
-				// when citing notes, only keep notes or note parents
+				// In add-note mode, note parent checks call item.getNotes(), which requires childItems
 				if (DIALOG_STATE.isAddingNote()) {
-					items = items.filter(item => SearchHandler.isItemWithNotes(item));
+					let regularItems = items.filter(item => SearchHandler.isItemWithNotes(item));
+					if (regularItems.length) {
+						await Zotero.Items.loadDataTypes(regularItems, ['childItems']);
+					}
+					// when citing notes, only keep notes or note parents
+					items = items.filter(item => item.isNote() || item.getNotes().length);
 				}
 				// when adding annotations, only keep annotations, their attachments, and their top-level items
 				if (DIALOG_STATE.isAddingAnnotations()) {
@@ -861,9 +867,10 @@ class LibraryLayout extends Layout {
 			isSearch: () => true,
 			isSearchMode: () => true,
 			setSearch: (searchText, mode) => collectionTreeRow.setSearch(searchText, mode),
+			clearCache: () => collectionTreeRow.clearCache(),
 			ref: collectionTreeRow.ref
 		});
-		await this.itemsView.setFilter('search', SearchHandler.searchValue);
+		await this.itemsView.setFilter('citation-search', SearchHandler.searchValue);
 		
 		this.itemsView.clearItemsPaneMessage();
 	}
@@ -883,7 +890,7 @@ class LibraryLayout extends Layout {
 
 	// click on + icon will add the item to the citation
 	_handleItemsViewIconClick(index) {
-		let rowNode = doc.querySelector(`#item-tree-citationDialog-row-${index}`);
+		let rowNode = doc.getElementById(`${this.itemsView.id}-row-${index}`);
 		let rowTopBeforeRefresh = rowNode.getBoundingClientRect().top;
 		this.itemsView.selection.clearSelection();
 		let row = this.itemsView.getRow(index);

chrome/content/zotero/integration/citationExplorer.js

@@ -28,6 +28,10 @@ const ReactDOM = require('react-dom');
 const diff = require('diff');
 const VirtualizedTable = require('components/virtualized-table');
 const { getCSSIcon, IconAttachSmall } = require('components/icons');
+// TODO: Create a custom row provider for citationExplorer to use with base ItemTree.
+// Currently uses changeCollectionTreeRow which only exists on CollectionViewItemTree,
+// so this is broken until we either switch to CollectionViewItemTree or create a
+// simple row provider that can display arbitrary items.
 const ItemTree = require('zotero/itemTree');
 const { getColumnDefinitionsByDataKey } = require('zotero/itemTreeColumns');
 const { makeRowRenderer } = VirtualizedTable;
@@ -48,7 +52,7 @@ const citationColumns = [
 		width: 26,
 		staticWidth: true,
 		fixedWidth: true,
-		renderer: (index, data, column) => {
+		renderCell: (index, data, column) => {
 			let icon = getCSSIcon('IconCross');
 			if (data) {
 				icon = getCSSIcon('IconTick');
@@ -72,7 +76,7 @@ itemColumns.push({
 	width: 26,
 	staticWidth: true,
 	fixedWidth: true,
-	renderer: (index, data, column) => {
+	renderCell: (index, data, column) => {
 		let icon = getCSSIcon('IconCross');
 		if (data) {
 			icon = getCSSIcon('IconTick');

chrome/content/zotero/itemTreeColumns.jsx

@@ -188,6 +188,7 @@ const COLUMNS = [
 	},
 	{
 		dataKey: "lastRead",
+		sortReverse: true,
 		defaultSort: -1,
 		defaultIn: ["recentlyRead"],
 		disabledIn: ["feeds", "feed"],

chrome/content/zotero/itemTreeRow.js

@@ -0,0 +1,648 @@
+const { getCSSIcon, getCSSItemTypeIcon } = require('components/icons');
+const { renderCell: baseRenderCell } = require('components/virtualized-table');
+const { XPCOMUtils } = ChromeUtils.importESModule("resource://gre/modules/XPCOMUtils.sys.mjs");
+
+const lazy = {};
+XPCOMUtils.defineLazyPreferenceGetter(
+	lazy,
+	"BIDI_BROWSER_UI",
+	"bidi.browser.ui",
+	false
+);
+
+const ATTACHMENT_STATE_LOAD_DELAY = 150;
+
+/**
+ * Base row in an ItemTree.
+ *
+ * Provides safe defaults for all row types. Subclass for specific reference
+ * types (ZoteroItemTreeRow, CollectionItemTreeRow, SearchItemTreeRow, etc.).
+ */
+class ItemTreeRow {
+	constructor(ref, level, isOpen, id) {
+		this.ref = ref;
+		this.level = level;
+		this.isOpen = isOpen;
+		this.id = id ?? ref.treeViewID;
+		if (this.id == null) {
+			throw new Error('ItemTreeRow: ref.treeViewID is required (or pass id explicitly)');
+		}
+	}
+
+	get type() {
+		return 'item';
+	}
+
+	get isDraggable() {
+		return false;
+	}
+
+	/**
+	 * Whether child rows should be sorted by the tree's active comparator
+	 * when this container is opened. Default false preserves the order
+	 * returned by getChildItems().
+	 */
+	get sortChildren() {
+		return false;
+	}
+
+	isContainer() {
+		return false;
+	}
+
+	isContainerOpen() {
+		return this.isOpen;
+	}
+
+	isContainerEmpty() {
+		return true;
+	}
+
+	getChildItems() {
+		return [];
+	}
+
+	getBestAttachmentStateCached() {
+		return null;
+	}
+
+	getBestAttachmentState() {
+		return null;
+	}
+
+	numNotes() {
+		return 0;
+	}
+
+	getField(field) {
+		if (Zotero.ItemTreeManager.isCustomColumn(field)) {
+			return Zotero.ItemTreeManager.getCustomCellData(this.ref, field);
+		}
+		return '';
+	}
+
+	getTypeLabel() {
+		return '';
+	}
+
+	getDisplayTitle() {
+		return '';
+	}
+
+	getIcon() {
+		return getCSSItemTypeIcon('document');
+	}
+
+	renderRow(div, index, columns, rowData, renderCtx) {
+		for (let column of columns) {
+			if (column.hidden) continue;
+			div.appendChild(renderCtx.renderCell(index, rowData[column.dataKey], column, column === renderCtx.firstColumn));
+		}
+	}
+
+	renderCell(index, data, column, isFirstColumn) {
+		let cell;
+		if (column.primary) {
+			cell = this.renderPrimaryCell(index, data, column);
+		}
+		else {
+			cell = baseRenderCell(index, data, column, isFirstColumn);
+			if (column.dataKey === 'numNotes' && data) {
+				cell.dataset.l10nId = 'items-table-cell-notes';
+				cell.dataset.l10nArgs = JSON.stringify({ count: data });
+			}
+			else if (column.dataKey === 'itemType') {
+				cell.setAttribute('aria-hidden', true);
+			}
+		}
+		if (column.noPadding) {
+			cell.classList.add('no-padding');
+		}
+		return cell;
+	}
+
+	renderPrimaryCell(index, data, column) {
+		let span = document.createElement('span');
+		span.className = `cell ${column.className}`;
+		span.classList.add('primary');
+
+		let textSpan = document.createElement('span');
+		textSpan.className = 'cell-text';
+		Zotero.Utilities.Internal.renderItemTitle(data, textSpan);
+		span.append(textSpan);
+
+		return span;
+	}
+}
+
+/**
+ * Row wrapping a Zotero.Item (regular items, notes, and non-file attachments).
+ *
+ * Provides field access, container logic for child notes/attachments,
+ * and full primary-cell rendering (tags, retraction marks, BIDI handling).
+ */
+class ZoteroItemTreeRow extends ItemTreeRow {
+	get isDraggable() {
+		return true;
+	}
+
+	getField(field, unformatted) {
+		if (this.ref.hasOwnProperty(field) && this.ref[field] != null) {
+			return this.ref[field];
+		}
+		else if (!Zotero.ItemTreeManager.isCustomColumn(field)) {
+			return this.ref.getField(field, unformatted, true);
+		}
+		return Zotero.ItemTreeManager.getCustomCellData(this.ref, field);
+	}
+
+	numNotes() {
+		if (this.ref.isNote()) {
+			return 0;
+		}
+		if (this.ref.isAttachment()) {
+			return this.ref.note !== '' ? 1 : 0;
+		}
+		return this.ref.numNotes(false, true) || 0;
+	}
+
+	isContainer() {
+		return this.ref.isRegularItem();
+	}
+
+	isContainerEmpty({ includeTrashed } = {}) {
+		if (!this.ref.isRegularItem()) {
+			return true;
+		}
+		return this.ref.numNotes(includeTrashed) === 0
+			&& this.ref.numAttachments(includeTrashed) == 0;
+	}
+
+	getChildItems({ includeTrashed, filterChildItems } = {}) {
+		if (!this.ref.isRegularItem()) {
+			return [];
+		}
+
+		let attachments = this.ref.getAttachments(includeTrashed);
+		let notes = this.ref.getNotes(includeTrashed);
+		let childIDs;
+		if (attachments.length && notes.length) {
+			childIDs = notes.concat(attachments);
+		}
+		else if (attachments.length) {
+			childIDs = attachments;
+		}
+		else if (notes.length) {
+			childIDs = notes;
+		}
+		else {
+			return [];
+		}
+
+		let items = Zotero.Items.get(childIDs);
+		// TODO: This is a bad pattern after item tree refactor and needs to be fixed (should not be used as an example)
+		// Skip unwanted child items (e.g. in citation dialog)
+		if (filterChildItems) {
+			items = items.filter(filterChildItems);
+		}
+		return items;
+	}
+
+	_supportsBestAttachmentState() {
+		return this.ref.isRegularItem() && this.ref.numAttachments();
+	}
+
+	getBestAttachmentStateCached() {
+		if (!this._supportsBestAttachmentState()) {
+			return null;
+		}
+		return this.ref.getBestAttachmentStateCached();
+	}
+
+	getBestAttachmentState() {
+		if (!this._supportsBestAttachmentState()) {
+			return null;
+		}
+		return this.ref.getBestAttachmentState();
+	}
+
+	getTypeLabel() {
+		if (!this.ref.itemTypeID) {
+			return '';
+		}
+		try {
+			return Zotero.ItemTypes.getLocalizedString(this.ref.itemTypeID);
+		}
+		catch (e) {
+			Zotero.debug(`Error getting localized item type for ${this.ref.itemTypeID}`, 1);
+			Zotero.debug(e, 1);
+			return '';
+		}
+	}
+
+	getDisplayTitle() {
+		return this.ref.getDisplayTitle();
+	}
+	
+	getAddedBy() {
+		return this.ref.createdByUserID
+			? Zotero.Users.getName(this.ref.createdByUserID) : "";
+	}
+	
+	getLastModifiedBy() {
+		return this.ref.lastModifiedByUserID
+			? Zotero.Users.getName(this.ref.lastModifiedByUserID) : this.getAddedBy();
+	}
+	
+	getIcon() {
+		return getCSSItemTypeIcon(this.ref.getItemTypeIconName());
+	}
+
+	renderCell(index, data, column, isFirstColumn, renderCtx) {
+		if (column.dataKey === 'hasAttachment') {
+			return this.renderHasAttachmentCell(index, data, column, renderCtx);
+		}
+		return super.renderCell(index, data, column, isFirstColumn, renderCtx);
+	}
+
+	renderPrimaryCell(index, data, column) {
+		let span = document.createElement('span');
+		span.className = `cell ${column.className}`;
+		span.classList.add('primary');
+
+		const item = this.ref;
+		let retracted = '';
+		let retractedAriaLabel = '';
+		if (Zotero.Retractions.isRetracted(item)) {
+			retracted = getCSSIcon('cross');
+			retracted.classList.add('icon-16');
+			retracted.classList.add('retracted');
+			retractedAriaLabel = Zotero.getString('retraction.banner');
+		}
+
+		let tagAriaLabel = '';
+		let tagSpans = [];
+		let coloredTags = item.getItemsListTags();
+		if (coloredTags.length) {
+			let { emoji, colored } = coloredTags.reduce((acc, tag) => {
+				acc[Zotero.Utilities.Internal.containsEmoji(tag.tag) ? 'emoji' : 'colored'].push(tag);
+				return acc;
+			}, { emoji: [], colored: [] });
+
+			if (colored.length) {
+				let coloredTagSpans = colored.map(x => this.getTagSwatch(x.tag, x.color));
+				let coloredTagSpanWrapper = document.createElement('span');
+				coloredTagSpanWrapper.className = 'colored-tag-swatches';
+				coloredTagSpanWrapper.append(...coloredTagSpans);
+				tagSpans.push(coloredTagSpanWrapper);
+			}
+
+			tagSpans.push(...emoji.map(x => this.getTagSwatch(x.tag)));
+
+			tagAriaLabel = coloredTags.length == 1 ? Zotero.getString('search-conditions-tag') : Zotero.getString('itemFields.tags');
+			tagAriaLabel += ' ' + coloredTags.map(x => x.tag).join(', ') + '.';
+		}
+
+		let itemTypeAriaLabel = this.getTypeLabel();
+		if (itemTypeAriaLabel) {
+			itemTypeAriaLabel += '.';
+		}
+
+		let textSpan = document.createElement('span');
+		let textWithFullStop = Zotero.Utilities.Internal.renderItemTitle(data, textSpan);
+		if (!textWithFullStop.match(/\.$/)) {
+			textWithFullStop += '.';
+		}
+		let textSpanAriaLabel = [textWithFullStop, itemTypeAriaLabel, tagAriaLabel, retractedAriaLabel]
+			.filter(Boolean)
+			.join(' ');
+		textSpan.className = 'cell-text';
+		if (item.itemTypeID && lazy.BIDI_BROWSER_UI) {
+			textSpan.dir = Zotero.ItemFields.getDirection(
+				item.itemTypeID, column.dataKey, item.getField('language')
+			);
+		}
+		textSpan.setAttribute('aria-label', textSpanAriaLabel);
+
+		if (Zotero.Prefs.get('ui.tagsAfterTitle')) {
+			span.append(retracted, textSpan, ...tagSpans);
+		}
+		else {
+			span.append(retracted, ...tagSpans, textSpan);
+		}
+
+		return span;
+	}
+
+	getTagSwatch(tag, color) {
+		let span = document.createElement('span');
+		span.className = 'tag-swatch';
+		let extractedEmojis = Zotero.Tags.extractEmojiForItemsList(tag);
+		if (extractedEmojis) {
+			span.textContent = extractedEmojis;
+			span.className += ' emoji';
+		}
+		else {
+			span.className += ' colored';
+			span.dataset.color = color.toLowerCase();
+			span.style.color = color;
+		}
+		return span;
+	}
+
+	renderHasAttachmentCell(index, data, column, renderCtx = {}) {
+		let span = document.createElement('span');
+		span.className = `cell ${column.className}`;
+
+		if (renderCtx.includeTrashed) {
+			return span;
+		}
+
+		const item = this.ref;
+		if ((!this.isContainer() || !this.isContainerOpen())) {
+			let progressValue = Zotero.Sync.Storage.getItemDownloadProgress(item);
+			if (progressValue) {
+				let progress = document.createElement('progress');
+				progress.value = progressValue;
+				progress.max = 100;
+				progress.style.setProperty('--progress', `${progressValue}%`);
+				progress.className = 'attachment-progress';
+				span.append(progress);
+				return span;
+			}
+		}
+
+		const attachmentState = this.getBestAttachmentStateCached();
+		if (!attachmentState) {
+			return span;
+		}
+		const { type, exists } = attachmentState;
+		let icon;
+		let ariaLabel;
+		if (type !== null && type != 'none') {
+			if (type == 'pdf') {
+				icon = getCSSItemTypeIcon('attachmentPDF', 'attachment-type');
+				ariaLabel = Zotero.getString('pane.item.attachments.hasPDF');
+			}
+			else if (type == 'snapshot') {
+				icon = getCSSItemTypeIcon('attachmentSnapshot', 'attachment-type');
+				ariaLabel = Zotero.getString('pane.item.attachments.hasSnapshot');
+			}
+			else if (type == 'epub') {
+				icon = getCSSItemTypeIcon('attachmentEPUB', 'attachment-type');
+				ariaLabel = Zotero.getString('pane.item.attachments.hasEPUB');
+			}
+			else if (type == 'image') {
+				icon = getCSSItemTypeIcon('attachmentImage', 'attachment-type');
+				ariaLabel = Zotero.getString('pane.item.attachments.hasImage');
+			}
+			else if (type == 'video') {
+				icon = getCSSItemTypeIcon('attachmentVideo', 'attachment-type');
+				ariaLabel = Zotero.getString('pane.item.attachments.hasVideo');
+			}
+			else {
+				icon = getCSSItemTypeIcon('attachmentFile', 'attachment-type');
+				ariaLabel = Zotero.getString('pane.item.attachments.has');
+			}
+
+			if (!exists) {
+				icon.classList.add('icon-missing-file');
+			}
+		}
+		if (icon && ariaLabel) {
+			icon.setAttribute('aria-label', ariaLabel + '.');
+			span.setAttribute('title', ariaLabel);
+		}
+		if (icon) {
+			span.append(icon);
+		}
+
+		let invalidateRow = renderCtx.invalidateRow;
+		if (!invalidateRow) {
+			return span;
+		}
+
+		setTimeout(() => {
+			let statePromise = this.getBestAttachmentState();
+			if (!statePromise?.then) {
+				return;
+			}
+			statePromise
+				.then(({ type: newType, exists: newExists } = {}) => {
+					if (newType !== type || newExists !== exists) {
+						invalidateRow(index);
+					}
+				})
+				.catch((e) => Zotero.logError(e));
+		}, ATTACHMENT_STATE_LOAD_DELAY);
+
+		return span;
+	}
+}
+
+/**
+ * Row wrapping a file attachment (PDF, snapshot, EPUB, etc.).
+ *
+ * Acts as a container for annotation child rows and overrides title display
+ * to show attachment filenames when configured.
+ */
+class FileItemTreeRow extends ZoteroItemTreeRow {
+	isContainer() {
+		return true;
+	}
+
+	isContainerEmpty({ searchMode, searchItemIDs } = {}) {
+		if (Zotero.Prefs.get("hideContextAnnotationRows") && searchMode) {
+			return !this.ref.getAnnotations().some(annotation => searchItemIDs.has(annotation.id));
+		}
+		return this.ref.numAnnotations() == 0;
+	}
+
+	getChildItems({ searchMode, searchItemIDs } = {}) {
+		let annotations = this.ref.getAnnotations();
+		if (Zotero.Prefs.get("hideContextAnnotationRows") && searchMode) {
+			annotations = annotations.filter(annotation => searchItemIDs.has(annotation.id));
+		}
+		return annotations;
+	}
+
+	_supportsBestAttachmentState() {
+		return this.ref.isTopLevelItem();
+	}
+
+	getDisplayTitle() {
+		if (!(this.ref.isSnapshotAttachment()
+				&& /snapshot/i.test(this.ref.getField('title')))
+				&& Zotero.Prefs.get('showAttachmentFilenames')) {
+			try {
+				return this.ref.attachmentFilename;
+			}
+			catch {
+				// Path wasn't parseable - it could be truly invalid, or just
+				// invalid for this platform (e.g., Windows path on macOS/Linux)
+				return this.ref.attachmentPath;
+			}
+		}
+		return this.ref.getDisplayTitle();
+	}
+}
+
+/**
+ * Row wrapping an annotation item.
+ *
+ * Never a container. Uses the annotation-specific icon and custom row content
+ * layout used in the items tree.
+ */
+class AnnotationItemTreeRow extends ZoteroItemTreeRow {
+	get type() {
+		return 'annotation';
+	}
+
+	isContainer() {
+		return false;
+	}
+
+	getIcon() {
+		let itemType = this.ref.getItemTypeIconName();
+		return getCSSItemTypeIcon(itemType, `annotation-${this.ref.annotationType}-${this.ref.annotationColor}`);
+	}
+
+	renderRow(div, index, columns, rowData, renderCtx) {
+		div.classList.add('annotation-row');
+		div.classList.remove('tight');
+
+		let titleRowData = Object.assign({}, columns.find(column => column.dataKey == 'title'));
+		titleRowData.className = 'title';
+
+		let title;
+		let parserUtils = Cc["@mozilla.org/parserutils;1"].getService(Ci.nsIParserUtils);
+		let plainText = parserUtils.convertToPlainText(this.ref.annotationText || "", Ci.nsIDocumentEncoder.OutputRaw, 0);
+		let plainComment = parserUtils.convertToPlainText(this.ref.annotationComment || "", Ci.nsIDocumentEncoder.OutputRaw, 0);
+		if (["highlight", "underline"].includes(this.ref.annotationType)) {
+			title = renderCtx.renderCell(index, plainText, titleRowData, true);
+			let titleCell = title.querySelector('.cell-text');
+			titleCell.classList.add('italics');
+			titleCell.setAttribute('q-mark-open', Zotero.getString('punctuation.openingQMark'));
+			title.setAttribute('q-mark-close', Zotero.getString('punctuation.closingQMark'));
+			if (this.ref.annotationComment) {
+				let comment = baseRenderCell(null, plainComment, { className: 'annotation-comment' });
+				div.appendChild(comment);
+			}
+			let containsCJK = /[\p{Script=Han}\p{Script=Hiragana}\p{Script=Katakana}\p{Script=Hangul}]/u.test(this.ref.annotationText);
+			div.classList.toggle('tight', !containsCJK);
+		}
+		else if (this.ref.annotationComment) {
+			title = renderCtx.renderCell(index, plainComment, titleRowData, true);
+		}
+		else {
+			let annotationTypeName = Zotero.getString(`reader-${this.ref.annotationType}-annotation`);
+			title = renderCtx.renderCell(index, annotationTypeName, titleRowData, true);
+		}
+		div.prepend(title);
+
+		let addToCitationColumn = columns.find(column => column.dataKey == 'addToCitation');
+		if (addToCitationColumn) {
+			let data = rowData?.[addToCitationColumn.dataKey];
+			let cell = renderCtx.renderCell(index, data, addToCitationColumn, false);
+			if (cell) {
+				div.append(cell);
+			}
+		}
+	}
+}
+
+/**
+ * Row wrapping a Zotero.Collection (shown in trash view).
+ */
+class CollectionItemTreeRow extends ItemTreeRow {
+	get type() {
+		return 'collection';
+	}
+
+	getIcon() {
+		let icon = getCSSIcon('collection');
+		icon.classList.add('icon-item-type');
+		return icon;
+	}
+
+	getTypeLabel() {
+		return Zotero.getString('search-conditions-collection');
+	}
+
+	getDisplayTitle() {
+		return this.ref.name;
+	}
+
+	getField(field) {
+		if (field == 'title') {
+			return this.ref.name;
+		}
+		if (Zotero.ItemTreeManager.isCustomColumn(field)) {
+			return Zotero.ItemTreeManager.getCustomCellData(this.ref, field);
+		}
+		return '';
+	}
+
+	renderPrimaryCell(index, data, column) {
+		return super.renderPrimaryCell(index, data, column);
+	}
+}
+
+/**
+ * Row wrapping a Zotero.Search (saved search, shown in trash view).
+ */
+class SearchItemTreeRow extends ItemTreeRow {
+	get type() {
+		return 'search';
+	}
+
+	getIcon() {
+		let icon = getCSSIcon('search');
+		icon.classList.add('icon-item-type');
+		return icon;
+	}
+
+	getTypeLabel() {
+		return Zotero.getString('search-conditions-savedSearch');
+	}
+
+	getDisplayTitle() {
+		return this.ref.name;
+	}
+
+	getField(field) {
+		if (field == 'title') {
+			return this.ref.name;
+		}
+		if (Zotero.ItemTreeManager.isCustomColumn(field)) {
+			return Zotero.ItemTreeManager.getCustomCellData(this.ref, field);
+		}
+		return '';
+	}
+
+	renderPrimaryCell(index, data, column) {
+		return super.renderPrimaryCell(index, data, column);
+	}
+}
+
+/**
+ * Create the appropriate ItemTreeRow subclass for a reference object.
+ *
+ * Dispatch order: Collection, Search, annotation item, file attachment item,
+ * generic Zotero.Item, and finally the base ItemTreeRow fallback.
+ */
+ItemTreeRow.create = function (ref, level, isOpen) {
+	if (ref instanceof Zotero.Collection) return new CollectionItemTreeRow(ref, level, isOpen);
+	if (ref instanceof Zotero.Search) return new SearchItemTreeRow(ref, level, isOpen);
+	if (ref.isAnnotation?.()) return new AnnotationItemTreeRow(ref, level, isOpen);
+	if (ref.isFileAttachment?.()) return new FileItemTreeRow(ref, level, isOpen);
+	return new ZoteroItemTreeRow(ref, level, isOpen);
+};
+
+module.exports = ItemTreeRow;
+module.exports.ItemTreeRow = ItemTreeRow;
+module.exports.ZoteroItemTreeRow = ZoteroItemTreeRow;
+module.exports.FileItemTreeRow = FileItemTreeRow;
+module.exports.AnnotationItemTreeRow = AnnotationItemTreeRow;
+module.exports.CollectionItemTreeRow = CollectionItemTreeRow;
+module.exports.SearchItemTreeRow = SearchItemTreeRow;

chrome/content/zotero/libraryTree.js

@@ -33,14 +33,11 @@ const React = require('react');
 var LibraryTree = class LibraryTree extends React.Component {
 	constructor(props) {
 		super(props);
-		this._rows = [];
-		this._rowMap = {};
 
 		this.domEl = props.domEl;
 		this._ownerDocument = props.domEl.ownerDocument;
 		
 		this.onSelect = this.createEventBinding('select');
-		this.onRefresh = this.createEventBinding('refresh');
 	}
 	
 	get window() {

chrome/content/zotero/selectItemsDialog.js

@@ -24,7 +24,7 @@
 */
 
 import CollectionTree from 'zotero/collectionTree';
-import ItemTree from 'zotero/itemTree';
+import CollectionViewItemTree from 'zotero/collectionViewItemTree';
 
 var itemsView;
 var collectionsView;
@@ -63,7 +63,7 @@ var doLoad = async function () {
 	if(io.addBorder) document.getElementsByTagName("dialog")[0].style.border = "1px solid black";
 	if(io.singleSelection) document.getElementById("zotero-items-tree").setAttribute("seltype", "single");
 	
-	itemsView = await ItemTree.init(document.getElementById('zotero-items-tree'), {
+	itemsView = await CollectionViewItemTree.init(document.getElementById('zotero-items-tree'), {
 		onSelectionChange: () => {
 			if (isEditBibliographyDialog) {
 				Zotero_Bibliography_Dialog.treeItemSelected();
@@ -77,7 +77,6 @@ var doLoad = async function () {
 		},
 		id: io.itemTreeID || "select-items-dialog",
 		dragAndDrop: false,
-		persistColumns: true,
 		regularOnly: io.onlyRegularItems,
 		columnPicker: true,
 		multiSelect: io.multiSelect,

chrome/content/zotero/xpcom/collectionTreeRow.js

@@ -32,8 +32,40 @@ Zotero.CollectionTreeRow = function (collectionTreeView, type, ref, level, isOpe
 	this.level = level || 0;
 	this.isOpen = isOpen || false;
 	this.onUnload = null;
+	this.searchText = "";
+	this.searchMode = "search";
+	this.tags = new Set();
+	
+	// Per-instance search cache. Within a single refresh cycle, multiple consumers need the
+	// same search results — getItems() for the items pane and getTags() for the tag selector
+	// both call getSearchResults(), and getSearchResults() calls getSearchObject(). This cache
+	// ensures the underlying DB query only runs once per cycle. Call clearCache() to invalidate
+	// (e.g., at the start of a refresh, or when filters change).
+	//
+	// On search failure (e.g., a saved search with invalid conditions), getSearchResults() throws
+	// a Zotero.CollectionTreeRow.SearchError. This is caught in
+	// CollectionViewItemTreeRowProvider.refresh() to show a load-error message without bricking
+	// the UI, so the user can still edit/delete the broken search. See the catch block in
+	// refresh() for details.
+	this._cachedResults = null;
+	this._cachedSearch = null;
+	this._cachedTempTable = null;
 }
 
+/**
+ * Error thrown by CollectionTreeRow.getSearchResults() when the underlying
+ * Zotero.Search query fails (e.g., a saved search with invalid conditions).
+ * Caught by CollectionViewItemTreeRowProvider.refresh() to show a load-error
+ * message without bricking the UI.
+ */
+Zotero.CollectionTreeRow.SearchError = class SearchError extends Error {
+	constructor(cause) {
+		super('ZoteroSearchError');
+		this.name = 'ZoteroSearchError';
+		this.cause = cause;
+	}
+};
+
 Zotero.CollectionTreeRow.IDCounter = 0;
 
 
@@ -322,32 +354,24 @@ Zotero.CollectionTreeRow.prototype.getItems = async function () {
 };
 
 Zotero.CollectionTreeRow.prototype.getSearchResults = async function (asTempTable) {
-	if (Zotero.CollectionTreeCache.lastTreeRow && Zotero.CollectionTreeCache.lastTreeRow.id !== this.id) {
+	if (!this._cachedResults) {
-		Zotero.CollectionTreeCache.clear();
-	}
-	
-	if(!Zotero.CollectionTreeCache.lastResults) {
 		let s = await this.getSearchObject();
-		Zotero.CollectionTreeCache.error = false;
 		try {
-			Zotero.CollectionTreeCache.lastResults = await s.search();
+			this._cachedResults = await s.search();
 		}
 		catch (e) {
 			Zotero.logError(e);
-			Zotero.CollectionTreeCache.lastResults = [];
+			throw new Zotero.CollectionTreeRow.SearchError(e);
-			// Flag error so ZoteroPane::onCollectionSelected() can show a message
-			Zotero.CollectionTreeCache.error = true;
 		}
-		Zotero.CollectionTreeCache.lastTreeRow = this;
 	}
 	
-	if(asTempTable) {
+	if (asTempTable) {
-		if(!Zotero.CollectionTreeCache.lastTempTable) {
+		if (!this._cachedTempTable) {
-			Zotero.CollectionTreeCache.lastTempTable = await Zotero.Search.idsToTempTable(Zotero.CollectionTreeCache.lastResults);
+			this._cachedTempTable = await Zotero.Search.idsToTempTable(this._cachedResults);
 		}
-		return Zotero.CollectionTreeCache.lastTempTable;
+		return this._cachedTempTable;
 	}
-	return Zotero.CollectionTreeCache.lastResults;
+	return this._cachedResults;
 };
 
 /*
@@ -356,12 +380,8 @@ Zotero.CollectionTreeRow.prototype.getSearchResults = async function (asTempTabl
  * This accounts for the collection, saved search, quicksearch, tags, etc.
  */
 Zotero.CollectionTreeRow.prototype.getSearchObject = async function () {
-	if (Zotero.CollectionTreeCache.lastTreeRow && Zotero.CollectionTreeCache.lastTreeRow.id !== this.id) {
+	if (this._cachedSearch) {
-		Zotero.CollectionTreeCache.clear();
+		return this._cachedSearch;
-	}
-	
-	if(Zotero.CollectionTreeCache.lastSearch) {
-		return Zotero.CollectionTreeCache.lastSearch;
 	}
 	
 	var s;
@@ -456,8 +476,7 @@ Zotero.CollectionTreeRow.prototype.getSearchObject = async function () {
 		}
 	}
 	
-	Zotero.CollectionTreeCache.lastTreeRow = this;
+	this._cachedSearch = s2;
-	Zotero.CollectionTreeCache.lastSearch = s2;
 	return s2;
 };
 
@@ -488,15 +507,54 @@ Zotero.CollectionTreeRow.prototype.getTags = async function (types, tagIDs) {
 };
 
 
+/**
+ * Clear the per-instance search cache. Call this at the start of a refresh cycle
+ * or when search/tag filters change, so the next getSearchResults()/getSearchObject()
+ * call runs a fresh DB query.
+ */
+Zotero.CollectionTreeRow.prototype.clearCache = function () {
+	this._cachedSearch = null;
+	if (this._cachedTempTable) {
+		let tableName = this._cachedTempTable;
+		let id = Zotero.DB.addCallback('commit', async function () {
+			await Zotero.DB.queryAsync(
+				"DROP TABLE IF EXISTS " + tableName, false, { noCache: true }
+			);
+			Zotero.DB.removeCallback('commit', id);
+		});
+	}
+	this._cachedTempTable = null;
+	this._cachedResults = null;
+};
+
 Zotero.CollectionTreeRow.prototype.setSearch = function (searchText, mode = null) {
-	Zotero.CollectionTreeCache.clear();
+	if (this.searchText === searchText && this.searchMode === mode) {
+		return false;
+	}
+	this.clearCache();
 	this.searchText = searchText;
 	this.searchMode = mode;
+	return true;
 }
 
 Zotero.CollectionTreeRow.prototype.setTags = function (tags) {
-	Zotero.CollectionTreeCache.clear();
+	let oldTags = this.tags instanceof Set ? this.tags : new Set(this.tags || []);
-	this.tags = tags;
+	let newTags = tags instanceof Set ? new Set(tags) : new Set(tags || []);
+	if (oldTags.size === newTags.size) {
+		let hasChanges = false;
+		for (let tag of newTags) {
+			if (!oldTags.has(tag)) {
+				hasChanges = true;
+				break;
+			}
+		}
+		if (!hasChanges) {
+			return false;
+		}
+	}
+	this.clearCache();
+	this.tags = newTags;
+	return true;
 }
 
 /*
@@ -507,6 +565,8 @@ Zotero.CollectionTreeRow.prototype.isSearchMode = function () {
 		case 'search':
 		case 'publications':
 		case 'trash':
+		case 'unfiled':
+		case 'recentlyRead':
 			return true;
 	}
 	
@@ -524,26 +584,3 @@ Zotero.CollectionTreeRow.prototype.isSearchMode = function () {
 Zotero.CollectionTreeRow.prototype.isSortable = function () {
 	return !this.isFeedsOrFeed() && !this.isRecentlyRead();
 }
-
-Zotero.CollectionTreeCache = {
-	"lastTreeRow":null,
-	"lastTempTable":null,
-	"lastSearch":null,
-	"lastResults":null,
-
-	"clear": function () {
-		this.lastTreeRow = null;
-		this.lastSearch = null;
-		if (this.lastTempTable) {
-			let tableName = this.lastTempTable;
-			let id = Zotero.DB.addCallback('commit', async function () {
-				await Zotero.DB.queryAsync(
-					"DROP TABLE IF EXISTS " + tableName, false, { noCache: true }
-				);
-				Zotero.DB.removeCallback('commit', id);
-			});
-		}
-		this.lastTempTable = null;
-		this.lastResults = null;
-	}
-}

chrome/content/zotero/xpcom/data/item.js

@@ -4009,7 +4009,7 @@ Zotero.Item.prototype.getAttachments = function (includeTrashed) {
  */
 Zotero.Item.prototype.getBestAttachment = async function () {
 	if (!this.isRegularItem()) {
-		throw ("getBestAttachment() can only be called on regular items");
+		throw new Error(`getBestAttachment() can only be called on regular items. Called on ${this.attachmentContentType}`);
 	}
 	var attachments = await this.getBestAttachments();
 	let bestAttachment = attachments ? attachments[0] : false;
@@ -4061,7 +4061,7 @@ Zotero.Item.prototype.getBestAttachmentState = async function () {
 	if (this._bestAttachmentState !== null && this._bestAttachmentState.type) {
 		return this._bestAttachmentState;
 	}
-	var item = this.isAttachment() && this.isTopLevelItem()
+	var item = !this.isRegularItem()
 		? this
 		: await this.getBestAttachment();
 	if (!item) {

chrome/content/zotero/xpcom/utilities_internal.js

@@ -2017,7 +2017,7 @@ Zotero.Utilities.Internal = {
 	 */
 	makeClassEventDispatcher: function (cls) {
 		cls.prototype._events = null;
-		cls.prototype.runListeners = async function (event) {
+		cls.prototype.runListeners = async function (event, ...args) {
 			// Zotero.debug(`Running ${event} listeners on ${cls.toString()}`);
 			if (!this._events) this._events = {};
 			if (!this._events[event]) {
@@ -2030,7 +2030,7 @@ Zotero.Utilities.Internal = {
 			// at the time of runListeners() call to prevent triggering listeners that are added right
 			// runListeners() invocation
 			for (let [listener, once] of Array.from(this._events[event].listeners.entries())) {
-				await Promise.resolve(listener.call(this));
+				await Promise.resolve(listener.call(this, ...args));
 				if (once) {
 					this._events[event].listeners.delete(listener);
 				}

chrome/content/zotero/zoteroPane.js

@@ -1659,12 +1659,11 @@ var ZoteroPane = new function () {
 
 	this.initItemsTree = async function () {
 		try {
-			const ItemTree = require('zotero/itemTree');
+			const CollectionViewItemTree = require('zotero/collectionViewItemTree');
 			var itemsTree = document.getElementById('zotero-items-tree');
-			ZoteroPane.itemsView = await ItemTree.init(itemsTree, {
+			ZoteroPane.itemsView = await CollectionViewItemTree.init(itemsTree, {
 				id: "main",
 				dragAndDrop: true,
-				persistColumns: true,
 				columnPicker: true,
 				onSelectionChange: selection => ZoteroPane.itemSelected(selection),
 				onContextMenu: (...args) => ZoteroPane.onItemsContextMenuOpen(...args),
@@ -1857,7 +1856,7 @@ var ZoteroPane = new function () {
 			}
 		}
 		
-		this.itemsView.changeCollectionTreeRow(collectionTreeRow);
+		await this.itemsView.changeCollectionTreeRow(collectionTreeRow);
 		
 		Zotero.Prefs.set('lastViewedFolder', collectionTreeRow.id);
 	});

resource/concurrentCaller.mjs

@@ -204,7 +204,7 @@ ConcurrentCaller.prototype._processNext = function () {
 	if (!task) {
 		if (this._numRunning == 0 && !this._pausing) {
 			this._log("All tasks are done");
-			this._deferred.resolve();
+			this._deferred?.resolve();
 		}
 		else {
 			this._log("Nothing left to run -- waiting for running tasks to complete");

scss/components/_item-tree.scss

@@ -32,7 +32,7 @@
 			text-align: center;
 		}
 
-		.first-column {
+		.cell:first-child {
 			&::before {
 				content: "";
 				display: inline-block;
@@ -42,11 +42,11 @@
 				position: absolute;
 				inset-inline-start: -8px;
 				
-				@include state(".first-column:hover") {
+				@include state(".cell:hover") {
 					background: var(--material-mix-quinary);
 				}
 				
-				@include state(".first-column.dragging") {
+				@include state(".cell.dragging") {
 					background: var(--material-mix-quarternary);
 				}
 			}
@@ -110,13 +110,13 @@
 			.icon-item-type + .colored-tag-swatches,
 			.annotation-icon + .tag-swatch,
 			.annotation-icon + .colored-tag-swatches {
-				margin-inline-start: 4px;
+				margin-inline-end: 4px;
 			}
 
 			.emoji + .emoji,
 			.emoji + .colored-tag-swatches,
 			.colored-tag-swatches + .emoji {
-				margin-inline-start: 4px;
+				margin-inline-end: 4px;
 			}
 			
 			.tag-swatch {
@@ -245,6 +245,10 @@
 			}
 		}
 		
+		.cell:not(.hasAttachment) .item-icon {
+			margin-inline-end: 4px;
+		}
+		
 		.cell .annotation-icon {
 			-moz-context-properties: fill;
 		}

scss/components/_virtualized-table.scss

@@ -27,6 +27,11 @@
 	flex-direction: column;
 	position: relative;
 
+	// Used in virtualized-table.jsx when setting column widths
+	// to reserve extra space for the first column. See onResize()
+	--extra-width: 0px;
+	--first-column-extra-width: 0px;
+
 	&:focus {
 		outline: none;
 	}
@@ -55,12 +60,13 @@
 			padding-inline-end: 4px;
 		}
 
-		&.first-column {
+		&:first-child {
+			--extra-width: var(--first-column-extra-width, 0px);
+			// No padding on the first cell since it's done via twisty and indent
 			padding-inline-start: 0;
-			min-width: calc(var(--firstColumnExtraWidth, 0px) + 30px);
 		}
 
-		&.first-column,
+		&:first-child,
 		&.primary {
 			display: flex;
 			align-items: center;
@@ -73,12 +79,6 @@
 				flex-grow: 1;
 				text-overflow: ellipsis;
 				overflow: hidden;
-
-				&:not(:first-child) {
-					@include state(".cell.first-column:not(.hasAttachment)") {
-						margin-inline-start: 4px;
-					}
-				}
 			}
 
 			.twisty + .cell-text, .spacer-twisty + .cell-text {
@@ -286,11 +286,9 @@
 		text-align: center;
 	}
 
-	.first-column {
+	.cell:first-child > :first-child {
-		> :first-child {
 		// offset header's column label/icon to align with the text in the first column, without introducing padding to the cell itself to avoid flexbox issues
-			padding-inline-start: calc(var(--firstColumnExtraWidth, 0px) + 8px);
+		padding-inline-start: var(--first-column-extra-width, 0px);
-		}
 	}
 
 	.cell {

test/tests/citationDialogTest.js

@@ -36,7 +36,11 @@ describe("Citation Dialog", function () {
 	beforeEach(async function () {
 		// Many operations (e.g. IOManager.addItemsToCitation) are disabled
 		// when search runs. Search can be triggered by a variety of events
-		// so before each test, we make sure that search has finished running
+		// (notably the window "focus" listener, which kicks off a search after
+		// a 100ms delay). Before each test, wait out that delay, then wait for
+		// any searches — including ones that only just started during the
+		// delay — to finish running.
+		await Zotero.Promise.delay(150);
 		while (SearchHandler.searching) {
 			await Zotero.Promise.delay(10);
 		}
@@ -218,11 +222,12 @@ describe("Citation Dialog", function () {
 	
 			// open popup
 			let firstBubble = CitationDataManager.items[0];
-			IOManager._openItemDetailsPopup(firstBubble.dialogReferenceID);
 			let popup = dialog.document.getElementById("itemDetails");
-			
+			let popupOpenPromise = popup.state == "open"
-			// give the popup time to open
+				? Promise.resolve()
-			await Zotero.Promise.delay(50);
+				: waitForDOMEvent(popup, "popupshown");
+			IOManager._openItemDetailsPopup(firstBubble.dialogReferenceID);
+			await popupOpenPromise;
 			assert.equal(popup.state, "open");
 	
 			// set locator/suffix/prefix values
@@ -472,7 +477,7 @@ describe("Citation Dialog", function () {
 			await dialog.libraryLayout.itemsView.selectItem(itemOne.id);
 			// Make sure the row node is highlighted
 			let rowIndex = dialog.libraryLayout.itemsView.getRowIndexByID(itemOne.id);
-			let rowID = "item-tree-citationDialog-row-" + rowIndex;
+			let rowID = `${dialog.libraryLayout.itemsView.id}-row-${rowIndex}`;
 			let rowNode = dialog.document.getElementById(rowID);
 			assert.isTrue(rowNode.classList.contains("highlighted"));
 		});
@@ -882,9 +887,11 @@ describe("Citation Dialog", function () {
 			let bubble = dialog.document.querySelector("bubble-input .bubble");
 			let popup = dialog.document.getElementById("itemDetails");
 
+			let popupOpenPromise = popup.state == "open"
+				? Promise.resolve()
+				: waitForDOMEvent(popup, "popupshown");
 			bubble.click();
-			// give the popup time to open
+			await popupOpenPromise;
-			await Zotero.Promise.delay(50);
 			assert.equal(popup.state, "open");
 
 			// make sure the annotation-row preview is visible in the popup

test/tests/collectionTreeRowTest.js

@@ -16,6 +16,122 @@ describe("CollectionTreeRow", function () {
 		win.close();
 	});
 
+	describe("Search cache", function () {
+		var collectionTreeRow;
+
+		// Stub getSearchObject on a collectionTreeRow so that the search's .search() throws,
+		// simulating a broken saved search (e.g., "too many SQL variables").
+		function stubBrokenSearch(ctr) {
+			return sinon.stub(ctr, 'getSearchObject').resolves({
+				search: () => { throw new Error('simulated search failure'); }
+			});
+		}
+
+		beforeEach(function () {
+			collectionTreeRow = zp.getCollectionTreeRow();
+			collectionTreeRow.clearCache();
+		});
+
+		afterEach(function () {
+			collectionTreeRow.setSearch('');
+			collectionTreeRow.setTags([]);
+		});
+
+		it("should memoize getSearchResults() within a refresh cycle", async function () {
+			var results1 = await collectionTreeRow.getSearchResults();
+			var results2 = await collectionTreeRow.getSearchResults();
+			assert.strictEqual(results1, results2, 'should return same cached array');
+		});
+
+		it("should memoize getSearchObject() within a refresh cycle", async function () {
+			var search1 = await collectionTreeRow.getSearchObject();
+			var search2 = await collectionTreeRow.getSearchObject();
+			assert.strictEqual(search1, search2, 'should return same cached search');
+		});
+
+		it("should invalidate cache on clearCache()", async function () {
+			var results1 = await collectionTreeRow.getSearchResults();
+			collectionTreeRow.clearCache();
+			var results2 = await collectionTreeRow.getSearchResults();
+			assert.notStrictEqual(results1, results2, 'should return new array after clearCache()');
+		});
+
+		it("should invalidate cache when setSearch() changes filters", async function () {
+			await collectionTreeRow.getSearchResults();
+			assert.isNotNull(collectionTreeRow._cachedResults);
+
+			collectionTreeRow.setSearch('test-query');
+			assert.isNull(collectionTreeRow._cachedResults);
+			assert.isNull(collectionTreeRow._cachedSearch);
+		});
+
+		it("should not invalidate cache when setSearch() is called with same value", async function () {
+			collectionTreeRow.setSearch('same');
+
+			await collectionTreeRow.getSearchResults();
+			var cached = collectionTreeRow._cachedResults;
+			assert.isNotNull(cached);
+
+			collectionTreeRow.setSearch('same');
+			assert.strictEqual(collectionTreeRow._cachedResults, cached,
+				'cache should survive idempotent setSearch()');
+		});
+
+		it("should throw SearchError on search failure", async function () {
+			var stub = stubBrokenSearch(collectionTreeRow);
+			try {
+				var err;
+				try {
+					await collectionTreeRow.getSearchResults();
+				}
+				catch (e) {
+					err = e;
+				}
+				assert.ok(err, 'getSearchResults() should throw');
+				assert.instanceOf(err, Zotero.CollectionTreeRow.SearchError);
+			}
+			finally {
+				stub.restore();
+			}
+		});
+
+		it("should propagate SearchError through getItems()", async function () {
+			var stub = stubBrokenSearch(collectionTreeRow);
+			try {
+				var err;
+				try {
+					await collectionTreeRow.getItems();
+				}
+				catch (e) {
+					err = e;
+				}
+				assert.ok(err, 'getItems() should throw');
+				assert.instanceOf(err, Zotero.CollectionTreeRow.SearchError);
+			}
+			finally {
+				stub.restore();
+			}
+		});
+
+		it("should propagate SearchError through getTags()", async function () {
+			var stub = stubBrokenSearch(collectionTreeRow);
+			try {
+				var err;
+				try {
+					await collectionTreeRow.getTags();
+				}
+				catch (e) {
+					err = e;
+				}
+				assert.ok(err, 'getTags() should throw');
+				assert.instanceOf(err, Zotero.CollectionTreeRow.SearchError);
+			}
+			finally {
+				stub.restore();
+			}
+		});
+	});
+	
 	describe("Unfiled Items", function () {
 		// https://github.com/zotero/zotero/issues/2771
 		it("shouldn't show filed attachments with annotations", async function () {

test/tests/collectionTreeTest.js

@@ -1444,23 +1444,7 @@ describe("Zotero.CollectionTree", function () {
 				await feedItem.saveTx();
 				var translateFn = sinon.spy(feedItem, 'translate');
 				
-				// Add observer to wait for collection-item add, using setTimeout
+				var ids = ((await onDrop('item', 'C' + collection.id, [feedItem.id]))).ids;
-				// to ensure all synchronous notifier processing completes first
-				var deferred = Zotero.Promise.defer();
-				var observerID = Zotero.Notifier.registerObserver({
-					notify: function (event, type, ids) {
-						if (type == 'collection-item' && event == 'add'
-								&& ids.some(id => id.startsWith(collection.id + "-"))) {
-							setTimeout(function () {
-								deferred.resolve();
-							});
-						}
-					}
-				}, 'collection-item', 'test');
-
-				await onDrop('item', 'C' + collection.id, [feedItem.id], deferred.promise);
-
-				Zotero.Notifier.unregisterObserver(observerID);
 				
 				// Check that the translated item was the one that was created after drag
 				var item = await translateFn.returnValues[0];

test/tests/collectionViewItemTreeTest.js

@@ -1,6 +1,9 @@
+// Integration tests for CollectionViewItemTree via ZoteroPane.itemsView.
+// Inherited ItemTree/ItemTreeRowProvider behavior is also tested through
+// the CVIT instance.
 "use strict";
 
-describe("Zotero.ItemTree", function () {
+describe("CollectionViewItemTree", function () {
 	var win, zp, cv, itemsView;
 	var existingItemID;
 	var existingItemID2;
@@ -179,7 +182,7 @@ describe("Zotero.ItemTree", function () {
 			assert.equal(quicksearch.value, "test");
 		});
 
-		it("should hide context annotation rows if hideContextAnnotationRows=true", async function () {
+		it("should keep attachment rows collapsed unless search matches annotation text when hideContextAnnotationRows=true", async function () {
 			Zotero.Prefs.set("hideContextAnnotationRows", true);
 
 			let item = await createDataObject('item', { title: "Item" });
@@ -197,22 +200,22 @@ describe("Zotero.ItemTree", function () {
 			let attachmentTwo = await importFileAttachment('test.pdf', { title: 'PDF test', parentItemID: item.id });
 			let highlightTwo = await createAnnotation('highlight', attachmentTwo, { comment: "Highlight te" });
 
-			// "te" search - all rows are visible
+			// Search matching attachment title only should keep attachment rows collapsed
-			await zp.itemsView.setFilter('search', "te");
+			await zp.itemsView.setFilter('search', "PDF test");
 
-			assert.isNumber(itemsView.getRowIndexByID(attachmentOne.id));
+			let attachmentTwoRow = itemsView.getRowIndexByID(attachmentTwo.id);
-			assert.isNumber(itemsView.getRowIndexByID(highlightOne.id));
+			assert.isNumber(attachmentTwoRow);
-			assert.isNumber(itemsView.getRowIndexByID(underlineOne.id));
+			assert.isFalse(itemsView.isContainerOpen(attachmentTwoRow));
-			assert.isNumber(itemsView.getRowIndexByID(attachmentTwo.id));
+			assert.isFalse(itemsView.getRowIndexByID(highlightTwo.id));
-			assert.isNumber(itemsView.getRowIndexByID(highlightTwo.id));
 
-			// "test" search - only annotations with "testing" remain
+			// Search matching annotation text should reveal matching annotation rows
-			await zp.itemsView.setFilter('search', "test");
+			await zp.itemsView.setFilter('search', "testing");
 
-			assert.isNumber(itemsView.getRowIndexByID(attachmentOne.id));
+			let attachmentOneRow = itemsView.getRowIndexByID(attachmentOne.id);
+			assert.isNumber(attachmentOneRow);
+			assert.isTrue(itemsView.isContainerOpen(attachmentOneRow));
 			assert.isNumber(itemsView.getRowIndexByID(underlineOne.id));
 			assert.isFalse(itemsView.getRowIndexByID(highlightOne.id));
-			assert.isNumber(itemsView.getRowIndexByID(attachmentTwo.id));
 			assert.isFalse(itemsView.getRowIndexByID(highlightTwo.id));
 		});
 	});
@@ -311,16 +314,28 @@ describe("Zotero.ItemTree", function () {
 	});
 
 	describe("Expand/Collapse all rows", function () {
-		let item1, item2, attachment1, attachment2;
+		let collection, item1, item2, emptyItem, attachment1, attachment2;
 
 		before(async () => {
+			collection = await createDataObject('collection');
 			// Top-level items, attachment child per each, one annotation per attachment
-			item1 = await createDataObject('item', { title: 'Item 1' });
+			item1 = await createDataObject('item', { title: 'Item 1', collections: [collection.id] });
-			item2 = await createDataObject('item', { title: 'Item 2' });
+			item2 = await createDataObject('item', { title: 'Item 2', collections: [collection.id] });
 			attachment1 = await importFileAttachment('test.pdf', { title: 'Attachment 1', parentItemID: item1.id });
 			attachment2 = await importFileAttachment('test.pdf', { title: 'Attachment 2', parentItemID: item2.id });
 			await createAnnotation('highlight', attachment1);
 			await createAnnotation('highlight', attachment2);
+			// An empty top-level item (no children) to test that it doesn't block level progression
+			emptyItem = await createDataObject('item', { title: 'Empty Item', collections: [collection.id] });
+			await select(win, collection);
+			itemsView = zp.itemsView;
+			await waitForItemsLoad(win);
+		});
+
+		beforeEach(async () => {
+			await select(win, collection);
+			itemsView = zp.itemsView;
+			await waitForItemsLoad(win);
 		});
 		
 		it("should expand all top-level rows when all rows are collapsed", async function () {
@@ -401,7 +416,6 @@ describe("Zotero.ItemTree", function () {
 
 			itemsView.collapseAllRows();
 
-
 			// All top-level items should be expanded
 			assert.isTrue(itemsView.isContainerOpen(itemsView.getRowIndexByID(item1.id)));
 			assert.isTrue(itemsView.isContainerOpen(itemsView.getRowIndexByID(item2.id)));
@@ -423,7 +437,124 @@ describe("Zotero.ItemTree", function () {
 		})
 	})
 	
-	describe.skip("#sort()", function () {
+	describe("#_saveOpenState() / #_restoreOpenState()", function () {
+		it("should restore open containers and keep closed containers closed", async function () {
+			let item1 = await createDataObject('item', { title: 'Item 1' });
+			let att1 = await importFileAttachment('test.pdf', { parentItemID: item1.id });
+			let ann1 = await createAnnotation('highlight', att1);
+			
+			let item2 = await createDataObject('item', { title: 'Item 2' });
+			await importFileAttachment('test.png', { parentItemID: item2.id });
+			
+			let item3 = await createDataObject('item', { title: 'Item 3' });
+			let att3 = await importFileAttachment('test.png', { parentItemID: item3.id });
+			
+			await waitForItemsLoad(win);
+			
+			let rowProvider = itemsView.rowProvider;
+			// Deep nesting for item1: open item1 -> attachment -> annotation path
+			assert.isTrue(rowProvider._expandToItem(ann1.id));
+			// Open item3 only
+			assert.isTrue(rowProvider._expandToItem(att3.id));
+			
+			let item1Row = itemsView.getRowIndexByID(item1.id);
+			let att1Row = itemsView.getRowIndexByID(att1.id);
+			let item2Row = itemsView.getRowIndexByID(item2.id);
+			let item3Row = itemsView.getRowIndexByID(item3.id);
+			assert.isTrue(itemsView.isContainerOpen(item1Row));
+			assert.isTrue(itemsView.isContainerOpen(att1Row));
+			assert.isFalse(itemsView.isContainerOpen(item2Row));
+			assert.isTrue(itemsView.isContainerOpen(item3Row));
+			
+			let openItemIDs = rowProvider._saveOpenState();
+			
+			// _saveOpenState closes top-level open containers
+			item1Row = itemsView.getRowIndexByID(item1.id);
+			item2Row = itemsView.getRowIndexByID(item2.id);
+			item3Row = itemsView.getRowIndexByID(item3.id);
+			assert.isFalse(itemsView.isContainerOpen(item1Row));
+			assert.isFalse(itemsView.isContainerOpen(item2Row));
+			assert.isFalse(itemsView.isContainerOpen(item3Row));
+			assert.isFalse(itemsView.getRowIndexByID(att1.id));
+			
+			rowProvider._restoreOpenState(openItemIDs);
+			
+			item1Row = itemsView.getRowIndexByID(item1.id);
+			att1Row = itemsView.getRowIndexByID(att1.id);
+			item2Row = itemsView.getRowIndexByID(item2.id);
+			item3Row = itemsView.getRowIndexByID(item3.id);
+			assert.isTrue(itemsView.isContainerOpen(item1Row));
+			assert.isTrue(itemsView.isContainerOpen(att1Row));
+			assert.isFalse(itemsView.isContainerOpen(item2Row));
+			assert.isTrue(itemsView.isContainerOpen(item3Row));
+		});
+	});
+	
+	describe("#toggleOpenState()", function () {
+		it("shouldn't scroll back to selected row when opening another container", async function () {
+			var collection = await createDataObject('collection');
+			await select(win, collection);
+			itemsView = zp.itemsView;
+			
+			var treebox = itemsView._treebox;
+			var numVisibleRows = treebox.getLastVisibleRow() - treebox.getFirstVisibleRow();
+			
+			function getTitle(i, max) {
+				return new String(new Array(max + 1).join(0) + i).slice(-1 * max);
+			}
+			
+			var num = numVisibleRows * 2 + 10;
+			var parentItem = await createDataObject('item', {
+				title: getTitle(0, num + 1),
+				collections: [collection.id]
+			});
+			await importFileAttachment('test.png', { parentItemID: parentItem.id });
+			
+			var itemIDs = [];
+			await Zotero.DB.executeTransaction(async function () {
+				for (let i = 1; i <= num; i++) {
+					let item = createUnsavedDataObject('item', {
+						title: getTitle(i, num + 1),
+						collections: [collection.id]
+					});
+					await item.save();
+					itemIDs.push(item.id);
+				}
+			});
+			await waitForItemsLoad(win);
+			
+			var parentRow = itemsView.getRowIndexByID(parentItem.id);
+			var selectedItemID;
+			var maxDistance = -1;
+			for (let id of itemIDs) {
+				let row = itemsView.getRowIndexByID(id);
+				let distance = Math.abs(row - parentRow);
+				if (distance > maxDistance) {
+					maxDistance = distance;
+					selectedItemID = id;
+				}
+			}
+			assert.isAbove(maxDistance, numVisibleRows);
+			
+			await itemsView.selectItem(selectedItemID);
+			assert.sameMembers(itemsView.getSelectedItems(true), [selectedItemID]);
+			
+			treebox.scrollToRow(parentRow);
+			var firstVisibleBefore = treebox.getFirstVisibleRow();
+			assert.isFalse(itemsView.tree.rowIsVisible(itemsView.getRowIndexByID(selectedItemID)));
+			assert.isFalse(itemsView.isContainerOpen(parentRow));
+			
+			await itemsView.toggleOpenState(parentRow);
+			await itemsView.waitForLoad();
+			
+			assert.isTrue(itemsView.isContainerOpen(itemsView.getRowIndexByID(parentItem.id)));
+			assert.sameMembers(itemsView.getSelectedItems(true), [selectedItemID]);
+			assert.equal(treebox.getFirstVisibleRow(), firstVisibleBefore);
+			assert.isFalse(itemsView.tree.rowIsVisible(itemsView.getRowIndexByID(selectedItemID)));
+		});
+	});
+	
+	describe("#sort()", function () {
 		it("should ignore invalid secondary-sort field", async function () {
 			await createDataObject('item', { title: 'A' });
 			await createDataObject('item', { title: 'A' });
@@ -453,6 +584,47 @@ describe("Zotero.ItemTree", function () {
 			assert.isFalse(e);
 			assert.equal(Zotero.Prefs.get('fallbackSort'), originalFallback);
 		});
+		
+		it("should preserve open container state when sorting", async function () {
+			let parentItem = await createDataObject('item', { title: 'Parent' });
+			let attachment = await importFileAttachment('test.pdf', { parentItemID: parentItem.id });
+			await createAnnotation('highlight', attachment);
+			
+			await waitForItemsLoad(win);
+			itemsView.expandAllRows(true);
+			await waitForItemsLoad(win);
+			
+			let parentRow = itemsView.getRowIndexByID(parentItem.id);
+			let attachmentRow = itemsView.getRowIndexByID(attachment.id);
+			assert.isTrue(itemsView.isContainerOpen(parentRow));
+			assert.isTrue(itemsView.isContainerOpen(attachmentRow));
+			
+			await itemsView.sort();
+			
+			parentRow = itemsView.getRowIndexByID(parentItem.id);
+			attachmentRow = itemsView.getRowIndexByID(attachment.id);
+			assert.isTrue(itemsView.isContainerOpen(parentRow));
+			assert.isTrue(itemsView.isContainerOpen(attachmentRow));
+		});
+		
+		it("should await sort context readiness before sorting", async function () {
+			let deferred = Zotero.Promise.defer();
+			let ensureStub = sinon.stub(itemsView, '_ensureSortContextReady').returns(deferred.promise);
+			let sortStub = sinon.stub(itemsView.rowProvider, 'sort');
+			
+			try {
+				let sortPromise = itemsView.sort();
+				await Zotero.Promise.delay(20);
+				assert.equal(sortStub.callCount, 0);
+				deferred.resolve();
+				await sortPromise;
+				assert.equal(sortStub.callCount, 1);
+			}
+			finally {
+				sortStub.restore();
+				ensureStub.restore();
+			}
+		});
 	});
 	
 	describe("#notify()", function () {
@@ -620,10 +792,8 @@ describe("Zotero.ItemTree", function () {
 			item.setField('title', 'no select on modify');
 			await item.saveTx();
 			
-			// itemSelected should have been called once (from 'selectEventsSuppressed = false'
+			// itemSelected should not have been called
-			// in notify()) as a no-op
+			assert.equal(win.ZoteroPane.itemSelected.callCount, 0);
-			assert.equal(win.ZoteroPane.itemSelected.callCount, 1);
-			assert.isFalse(await win.ZoteroPane.itemSelected.returnValues[0]);
 			
 			// Modified item should not be selected
 			assert.lengthOf(itemsView.getSelectedItems(), 0);
@@ -646,8 +816,8 @@ describe("Zotero.ItemTree", function () {
 			item.setField('title', 'maintain selection on modify');
 			await item.saveTx();
 			
-			// itemSelected should have been called once (from 'selectEventsSuppressed = false'
+			// itemSelected should have been called once from restoreSelection
-			// in notify()) as a no-op
+			// due to potential resort on modification
 			assert.equal(win.ZoteroPane.itemSelected.callCount, 1);
 			assert.isFalse(await win.ZoteroPane.itemSelected.returnValues[0]);
 			
@@ -755,9 +925,9 @@ describe("Zotero.ItemTree", function () {
 			assert.equal(itemsView.getRow(treebox.getFirstVisibleRow()).ref.id, firstVisibleItemID);
 		});
 		
-		it.skip("should keep first visible selected item in position when other items are added with skipSelect", function* () {
+		it("should keep first visible selected item in position when other items are added with skipSelect", async function () {
-			var collection = yield createDataObject('collection');
+			var collection = await createDataObject('collection');
-			yield select(win, collection);
+			await select(win, collection);
 			itemsView = zp.itemsView;
 			
 			var treebox = itemsView._treebox;
@@ -769,14 +939,14 @@ describe("Zotero.ItemTree", function () {
 			}
 			
 			var num = numVisibleRows + 10;
-			yield Zotero.DB.executeTransaction(async function () {
+			await Zotero.DB.executeTransaction(async function () {
 				for (let i = 0; i < num; i++) {
 					let title = getTitle(i, num);
 					let item = createUnsavedDataObject('item', { title });
 					item.addToCollection(collection.id);
 					await item.save();
 				}
-			}.bind(this));
+			});
 			
 			// Scroll halfway
 			treebox.scrollToRow(Math.round(num / 2) - Math.round(numVisibleRows / 2));
@@ -790,11 +960,11 @@ describe("Zotero.ItemTree", function () {
 			var item = createUnsavedDataObject(
 				'item', { title: getTitle(0, num), collections: [collection.id] }
 			);
-			yield item.saveTx({
+			await item.saveTx({
 				skipSelect: true
 			});
 			// Then add a few more in a transaction
-			yield Zotero.DB.executeTransaction(async function () {
+			await Zotero.DB.executeTransaction(async function () {
 				for (let i = 0; i < 3; i++) {
 					var item = createUnsavedDataObject(
 						'item', { title: getTitle(0, num), collections: [collection.id] }
@@ -803,7 +973,7 @@ describe("Zotero.ItemTree", function () {
 						skipSelect: true
 					});
 				}
-			}.bind(this));
+			});
 			
 			// Make sure the selected item is still at the same position
 			assert.equal(itemsView.getSelectedItems()[0], selectedItem);
@@ -1105,6 +1275,18 @@ describe("Zotero.ItemTree", function () {
 				assert.isTrue(itemsView.isContainerOpen(itemsView.getRowIndexByID(item2.id)));
 				assert.equal(noteRowIndex, secondItemRowIndex + 1);
 			});
+
+			it("should not expand an empty parent item when attachment is added", async function () {
+				let item2RowIndex = itemsView.getRowIndexByID(item2.id);
+				assert.isFalse(itemsView.isContainerOpen(item2RowIndex));
+
+				// Add attachment to item2
+				await importFileAttachment('test.png', { parentItemID: item2.id });
+
+				// Verify item2 is still collapsed
+				item2RowIndex = itemsView.getRowIndexByID(item2.id);
+				assert.isFalse(itemsView.isContainerOpen(item2RowIndex));
+			});
 		});
 		
 		describe("Recently Read", function () {
@@ -1447,7 +1629,34 @@ describe("Zotero.ItemTree", function () {
 				assert.isNumber(itemsView.getRowIndexByID(c1.treeViewID));
 				assert.isFalse(itemsView.getRowIndexByID(c2.treeViewID));
 				assert.isFalse(itemsView.getRowIndexByID(c3.treeViewID));
-			})
+			});
+
+			it("should assign collection/search row types in trash", async function () {
+				let collection = await createDataObject('collection', { deleted: true });
+				let search = await createDataObject('search', { deleted: true });
+
+				await selectTrash(win);
+
+				let collectionRowIndex = itemsView.getRowIndexByID(collection.treeViewID);
+				let searchRowIndex = itemsView.getRowIndexByID(search.treeViewID);
+				assert.isNumber(collectionRowIndex);
+				assert.isNumber(searchRowIndex);
+				assert.equal(itemsView.getRow(collectionRowIndex).type, 'collection');
+				assert.equal(itemsView.getRow(searchRowIndex).type, 'search');
+			});
+
+			it("should sort by hasAttachment in trash without crashing", async function () {
+				await createDataObject('collection', { deleted: true });
+				await createDataObject('search', { deleted: true });
+				await createDataObject('item', { deleted: true });
+				await selectTrash(win);
+
+				let columnIndex = itemsView._getColumns().findIndex(column => column.dataKey == 'hasAttachment');
+				assert.isAtLeast(columnIndex, 0);
+
+				await itemsView._handleColumnSort(columnIndex, 1);
+				assert.isAbove(itemsView.rowCount, 0);
+			});
 
 			it("should restore all subcollections when parent is restored", async function () {
 				var c1 = await createDataObject('collection', { deleted: true });
@@ -1723,8 +1932,6 @@ describe("Zotero.ItemTree", function () {
 			})
 			
 			await promise;
-			// Attachment add triggers multiple notifications and multiple select events
-			await itemsView.waitForSelect();
 			var items = itemsView.getSelectedItems();
 			var path = await items[0].getFilePathAsync();
 			assert.equal(
@@ -2193,6 +2400,133 @@ describe("Zotero.ItemTree", function () {
 		});
 	});
 	
+	describe("#_expandToItem()", function () {
+		it("should expand all ancestors for a nested annotation", async function () {
+			let parentItem = await createDataObject('item', { title: 'Parent Item' });
+			let attachment = await importFileAttachment('test.pdf', { parentItemID: parentItem.id });
+			let annotation = await createAnnotation('highlight', attachment);
+			await waitForItemsLoad(win);
+			
+			itemsView.collapseAllRows();
+			await waitForItemsLoad(win);
+			
+			let collapsedParentRow = itemsView.getRowIndexByID(parentItem.id);
+			assert.isNumber(collapsedParentRow);
+			assert.isFalse(itemsView.isContainerOpen(collapsedParentRow));
+			assert.isFalse(itemsView.getRowIndexByID(attachment.id));
+			assert.isFalse(itemsView.getRowIndexByID(annotation.id));
+			
+			let expanded = itemsView.rowProvider._expandToItem(annotation.id);
+			assert.isTrue(expanded);
+			
+			let parentRow = itemsView.getRowIndexByID(parentItem.id);
+			let attachmentRow = itemsView.getRowIndexByID(attachment.id);
+			assert.isTrue(itemsView.isContainerOpen(parentRow));
+			assert.isTrue(itemsView.isContainerOpen(attachmentRow));
+			assert.isNumber(itemsView.getRowIndexByID(annotation.id));
+		});
+	});
+	
+	describe("#setCollectionTreeRow()", function () {
+		it("should no-op when setting the same row", async function () {
+			let rowProvider = itemsView.rowProvider;
+			let currentRow = rowProvider.collectionTreeRow;
+			assert.ok(currentRow);
+			
+			let refreshSpy = sinon.spy(rowProvider, 'refresh');
+			
+			try {
+				await rowProvider.setCollectionTreeRow(currentRow);
+				assert.equal(refreshSpy.callCount, 0);
+			}
+			finally {
+				refreshSpy.restore();
+			}
+		});
+	});
+	
+	describe("#setFilter()", function () {
+		it("should refresh when search filter value changes", async function () {
+			let rowProvider = itemsView.rowProvider;
+			let refreshSpy = sinon.spy(rowProvider, 'refresh');
+			let setSearchStub = sinon.stub(rowProvider.collectionTreeRow, 'setSearch').returns(true);
+			
+			try {
+				await rowProvider.setFilter('search', 'changed-search');
+				assert.isTrue(setSearchStub.calledOnceWithExactly('changed-search'));
+				assert.isTrue(refreshSpy.calledOnceWithExactly({ restoreSelection: true }));
+			}
+			finally {
+				setSearchStub.restore();
+				refreshSpy.restore();
+			}
+		});
+		
+		it("should not refresh when filter value is unchanged", async function () {
+			let rowProvider = itemsView.rowProvider;
+			let refreshSpy = sinon.spy(rowProvider, 'refresh');
+			let setSearchStub = sinon.stub(rowProvider.collectionTreeRow, 'setSearch').returns(false);
+			
+			try {
+				await rowProvider.setFilter('search', 'unchanged-search');
+				assert.equal(refreshSpy.callCount, 0);
+			}
+			finally {
+				setSearchStub.restore();
+				refreshSpy.restore();
+			}
+		});
+	});
+	
+	describe("#_refresh()", function () {
+		it("should await sort context readiness before sorting", async function () {
+			let rowProvider = itemsView.rowProvider;
+			let deferred = Zotero.Promise.defer();
+			let ensureStub = sinon.stub(itemsView, '_ensureSortContextReady').returns(deferred.promise);
+			let sortSpy = sinon.spy(rowProvider, '_sort');
+			
+			try {
+				let refreshPromise = rowProvider._refresh();
+				await Zotero.Promise.delay(20);
+				assert.equal(sortSpy.callCount, 0);
+				deferred.resolve();
+				await refreshPromise;
+				assert.isTrue(sortSpy.called);
+			}
+			finally {
+				sortSpy.restore();
+				ensureStub.restore();
+			}
+		});
+	});
+	
+	describe("#handleRowModelUpdate()", function () {
+		it("should clear selection and return false when loading is true", async function () {
+			await itemsView.waitForLoad();
+			let item = await createDataObject('item');
+			await waitForItemsLoad(win);
+			
+			let row = itemsView.getRowIndexByID(item.id);
+			assert.isNumber(row);
+			itemsView.selection.select(row);
+			assert.equal(itemsView.selection.count, 1);
+			
+			let setMessageSpy = sinon.spy(itemsView, 'setItemsPaneMessage');
+			
+			try {
+				let done = await itemsView.handleRowModelUpdate([], { loading: true });
+				assert.isFalse(done);
+				assert.equal(itemsView.selection.count, 0);
+				assert.equal(itemsView.selection.focused, 0);
+				assert.isTrue(setMessageSpy.calledOnce);
+				assert.equal(setMessageSpy.firstCall.args[0], Zotero.getString('pane.items.loading'));
+			}
+			finally {
+				setMessageSpy.restore();
+				await itemsView.clearItemsPaneMessage();
+			}
+		});
+	});
 	
 	describe("#_restoreSelection()", function () {
 		it("should reselect collection in trash", async function () {
@@ -2214,9 +2548,31 @@ describe("Zotero.ItemTree", function () {
 			zp.itemsView._restoreSelection(selection);
 			assert.lengthOf(zp.itemsView.getSelectedObjects(), 2);
 		});
+		
+		it("should not expand collapsed parents when expandCollapsedParents is false", async function () {
+			let parentItem = await createDataObject('item', { title: 'Parent Item' });
+			let childAttachment = await importFileAttachment('test.png', { parentItemID: parentItem.id });
+			await waitForItemsLoad(win);
+			
+			await itemsView.selectItem(childAttachment.id);
+			let parentRow = itemsView.getRowIndexByID(parentItem.id);
+			assert.isTrue(itemsView.isContainerOpen(parentRow));
+			
+			itemsView.rowProvider._closeContainer(parentRow);
+			parentRow = itemsView.getRowIndexByID(parentItem.id);
+			assert.isFalse(itemsView.isContainerOpen(parentRow));
+			
+			itemsView.selection.clearSelection();
+			await itemsView._restoreSelection([childAttachment], false, false);
+			
+			parentRow = itemsView.getRowIndexByID(parentItem.id);
+			assert.isFalse(itemsView.isContainerOpen(parentRow));
+			assert.isFalse(itemsView.getRowIndexByID(childAttachment.id));
+			assert.sameMembers(itemsView.getSelectedItems(true), [parentItem.id]);
+		});
 	});
 
-	describe("#_renderPrimaryCell()", function () {
+	describe("primary cell rendering", function () {
 		async function getPrimaryCellContent(asHTML = false) {
 			let cellText;
 			do {
@@ -2296,6 +2652,7 @@ describe("Zotero.ItemTree", function () {
 				let annotationRowIndex = zp.itemsView.getRowIndexByID(annotation.id);
 				offset += 1;
 				assert.equal(annotationRowIndex, attachmentRowIndex + offset);
+				assert.equal(zp.itemsView.getRow(annotationRowIndex).type, 'annotation');
 			}
 		});
 
@@ -2304,9 +2661,17 @@ describe("Zotero.ItemTree", function () {
 			let itemAboveTwo = await createDataObject('item', { title: "BBB" });
 			let itemBelowOne = await createDataObject('item', { title: "ZZZ" });
 
-			// Initially, everything is sorted by title
+			// Ensure known starting state: primary sort by title ascending
 			var colIndex = itemsView.tree._getColumns().findIndex(column => column.dataKey == 'title');
+			for (let i = 0; i < 3; i++) {
+				let sortFields = itemsView.getSortFields();
+				if (sortFields[0] == 'title' && itemsView.getSortDirection(sortFields) == 1) {
+					break;
+				}
 				await zp.itemsView.tree._columns.toggleSort(colIndex);
+			}
+			assert.equal(itemsView.getSortField(), 'title');
+			assert.equal(itemsView.getSortDirection(itemsView.getSortFields()), 1);
 
 			// Expand annotations
 			var itemRowIndex = zp.itemsView.getRowIndexByID(toplevelItem.id);
@@ -2465,4 +2830,60 @@ describe("Zotero.ItemTree", function () {
 			});
 		});
 	});
+
+	describe("Search error handling", function () {
+		var rowProvider;
+
+		// Stub getSearchObject on a collectionTreeRow so that the search's .search() throws,
+		// simulating a broken saved search (e.g., "too many SQL variables").
+		function stubBrokenSearch(ctr) {
+			return sinon.stub(ctr, 'getSearchObject').resolves({
+				search: () => { throw new Error('simulated search failure'); }
+			});
+		}
+
+		beforeEach(async function () {
+			var search = await createDataObject('search');
+			await select(win, search);
+			itemsView = zp.itemsView;
+			rowProvider = itemsView.rowProvider;
+		});
+
+		it("should show load error message on search failure", async function () {
+			var stub = stubBrokenSearch(rowProvider.collectionTreeRow);
+			var setMessageSpy = sinon.spy(itemsView, 'setItemsPaneMessage');
+			try {
+				await rowProvider.refresh();
+				assert.isTrue(setMessageSpy.called);
+				assert.include(setMessageSpy.lastCall.args[0], Zotero.getString('pane.items.loadError'));
+				assert.equal(itemsView.rowCount, 0);
+			}
+			finally {
+				stub.restore();
+				setMessageSpy.restore();
+			}
+		});
+
+		it("should recover after switching to a working collection", async function () {
+			var stub = stubBrokenSearch(rowProvider.collectionTreeRow);
+			await rowProvider.refresh();
+			stub.restore();
+
+			await selectLibrary(win);
+			itemsView = zp.itemsView;
+			assert.isAbove(itemsView.rowCount, 0);
+			assert.isFalse(!!itemsView._itemsPaneMessage);
+		});
+
+		it("should not re-throw SearchError from refresh()", async function () {
+			var stub = stubBrokenSearch(rowProvider.collectionTreeRow);
+			try {
+				// refresh() should resolve, not reject
+				await rowProvider.refresh();
+			}
+			finally {
+				stub.restore();
+			}
+		});
+	});
 })

test/tests/itemTreeRowTest.js

@@ -0,0 +1,181 @@
+"use strict";
+
+describe("ItemTreeRow", function () {
+	var win;
+	var ItemTreeRow;
+	var ZoteroItemTreeRow;
+	var FileItemTreeRow;
+	var AnnotationItemTreeRow;
+	var CollectionItemTreeRow;
+	var SearchItemTreeRow;
+
+	before(async function () {
+		win = await loadZoteroPane();
+		({
+			ItemTreeRow,
+			ZoteroItemTreeRow,
+			FileItemTreeRow,
+			AnnotationItemTreeRow,
+			CollectionItemTreeRow,
+			SearchItemTreeRow,
+		} = win.require('zotero/itemTreeRow'));
+		await selectLibrary(win);
+	});
+
+	after(function () {
+		win.close();
+	});
+
+	it("should create row subclasses via factory", async function () {
+		let item = await createDataObject('item');
+		let attachment = await importFileAttachment('test.pdf', { parentItemID: item.id });
+		let annotation = await createAnnotation('highlight', attachment);
+		let collection = await createDataObject('collection');
+		let search = await createDataObject('search');
+
+		assert.instanceOf(ItemTreeRow.create(item, 0, false), ZoteroItemTreeRow);
+		assert.instanceOf(ItemTreeRow.create(attachment, 0, false), FileItemTreeRow);
+		assert.instanceOf(ItemTreeRow.create(annotation, 0, false), AnnotationItemTreeRow);
+		assert.instanceOf(ItemTreeRow.create(annotation, 0, false), ZoteroItemTreeRow);
+		assert.instanceOf(ItemTreeRow.create(collection, 0, false), CollectionItemTreeRow);
+		assert.instanceOf(ItemTreeRow.create(search, 0, false), SearchItemTreeRow);
+	});
+
+	it("should provide container and child behavior for regular items and file attachments", async function () {
+		let item = await createDataObject('item');
+		let note = await createDataObject('item', { itemType: 'note', parentID: item.id });
+		let attachment = await importFileAttachment('test.pdf', { parentItemID: item.id });
+		let annotation = await createAnnotation('highlight', attachment);
+
+		let itemRow = ItemTreeRow.create(item, 0, false);
+		assert.isTrue(itemRow.isContainer());
+		assert.isFalse(itemRow.isContainerOpen());
+		assert.isFalse(itemRow.isContainerEmpty({
+			searchMode: false,
+			searchItemIDs: new Set(),
+			includeTrashed: false,
+		}));
+		assert.sameMembers(
+			itemRow.getChildItems({ includeTrashed: false }).map(x => x.id),
+			[note.id, attachment.id]
+		);
+
+		let attachmentRow = ItemTreeRow.create(attachment, 1, false);
+		assert.isTrue(attachmentRow.isContainer());
+		assert.isFalse(attachmentRow.isContainerOpen());
+		assert.sameMembers(
+			attachmentRow.getChildItems({
+				searchMode: false,
+				searchItemIDs: new Set(),
+				includeTrashed: false,
+			}).map(x => x.id),
+			[annotation.id]
+		);
+
+		let annotationRow = ItemTreeRow.create(annotation, 2, false);
+		assert.isFalse(annotationRow.isContainer());
+	});
+
+	it("should expose attachment-state behavior by row type", async function () {
+		let item = await createDataObject('item');
+		let childAttachment = await importFileAttachment('test.pdf', { parentItemID: item.id });
+		let topLevelAttachment = await importFileAttachment('test.pdf');
+		let annotation = await createAnnotation('highlight', topLevelAttachment);
+		let note = await createDataObject('item', { itemType: 'note' });
+		let collection = await createDataObject('collection');
+		let search = await createDataObject('search');
+
+		let itemRow = ItemTreeRow.create(item, 0, false);
+		let childAttachmentRow = ItemTreeRow.create(childAttachment, 1, false);
+		let topLevelAttachmentRow = ItemTreeRow.create(topLevelAttachment, 0, false);
+		let annotationRow = ItemTreeRow.create(annotation, 1, false);
+		let noteRow = ItemTreeRow.create(note, 0, false);
+		let collectionRow = ItemTreeRow.create(collection, 0, false);
+		let searchRow = ItemTreeRow.create(search, 0, false);
+
+		assert.ok(itemRow.getBestAttachmentState()?.then);
+		assert.notOk(childAttachmentRow.getBestAttachmentState());
+		assert.ok(topLevelAttachmentRow.getBestAttachmentState()?.then);
+		assert.notOk(annotationRow.getBestAttachmentState());
+		assert.notOk(noteRow.getBestAttachmentState());
+		assert.notOk(collectionRow.getBestAttachmentState());
+		assert.notOk(searchRow.getBestAttachmentState());
+	});
+
+	it("should return localized type labels for all row types", async function () {
+		let item = await createDataObject('item', { itemType: 'book' });
+		let attachment = await importFileAttachment('test.pdf', { parentItemID: item.id });
+		let annotation = await createAnnotation('highlight', attachment);
+		let collection = await createDataObject('collection');
+		let search = await createDataObject('search');
+
+		let itemRow = ItemTreeRow.create(item, 0, false);
+		let attachmentRow = ItemTreeRow.create(attachment, 1, false);
+		let annotationRow = ItemTreeRow.create(annotation, 2, false);
+		let collectionRow = ItemTreeRow.create(collection, 0, false);
+		let searchRow = ItemTreeRow.create(search, 0, false);
+
+		assert.equal(itemRow.getTypeLabel(), Zotero.ItemTypes.getLocalizedString(item.itemTypeID));
+		assert.equal(attachmentRow.getTypeLabel(), Zotero.ItemTypes.getLocalizedString(attachment.itemTypeID));
+		assert.equal(annotationRow.getTypeLabel(), Zotero.ItemTypes.getLocalizedString(annotation.itemTypeID));
+		assert.equal(collectionRow.getTypeLabel(), Zotero.getString('search-conditions-collection'));
+		assert.equal(searchRow.getTypeLabel(), Zotero.getString('search-conditions-savedSearch'));
+	});
+
+	it("should return filename as display title for file attachment when pref is enabled", async function () {
+		let pref = Zotero.Prefs.get('showAttachmentFilenames');
+		let item = await createDataObject('item');
+		let attachment = await importFileAttachment('test.pdf', { parentItemID: item.id });
+		attachment.setField('title', 'Custom Attachment Title');
+		await attachment.saveTx();
+
+		try {
+			Zotero.Prefs.set('showAttachmentFilenames', false);
+			let withoutPref = ItemTreeRow.create(attachment, 1, false);
+			assert.equal(withoutPref.getDisplayTitle(), attachment.getDisplayTitle());
+
+			Zotero.Prefs.set('showAttachmentFilenames', true);
+			let withPref = ItemTreeRow.create(attachment, 1, false);
+			assert.notEqual(withPref.getDisplayTitle(), attachment.getDisplayTitle());
+			assert.equal(withPref.getDisplayTitle(), attachment.attachmentFilename);
+		}
+		finally {
+			Zotero.Prefs.set('showAttachmentFilenames', pref);
+		}
+	});
+
+	it("should render annotation row content with title and comment cells", async function () {
+		let item = await createDataObject('item');
+		let attachment = await importFileAttachment('test.pdf', { parentItemID: item.id });
+		let annotation = await createAnnotation('highlight', attachment, {
+			comment: 'Annotation comment',
+		});
+
+		let row = ItemTreeRow.create(annotation, 2, false);
+		let div = win.document.createElement('div');
+		let columns = [{ dataKey: 'title', className: 'title' }];
+		let calls = {
+			renderCell: 0,
+		};
+
+		let renderCtx = {
+			firstColumn: columns[0],
+			renderCell: () => {
+				calls.renderCell++;
+				let span = win.document.createElement('span');
+				span.className = 'cell title';
+				let text = win.document.createElement('span');
+				text.className = 'cell-text';
+				span.append(text);
+				return span;
+			},
+		};
+
+		row.renderRow(div, 0, columns, {}, renderCtx);
+
+		assert.equal(row.type, 'annotation');
+		assert.isTrue(div.classList.contains('annotation-row'));
+		assert.isAbove(calls.renderCell, 0);
+		assert.exists(div.querySelector('.annotation-comment'));
+	});
+});

test/tests/tagSelectorTest.js

@@ -782,4 +782,25 @@ describe("Tag Selector", function () {
 			assert.notInclude(getRegularTags(), 'automatic');
 		});
 	});
+
+	describe("Search error handling", function () {
+		it("should degrade gracefully when getTags() throws SearchError", async function () {
+			// _safeGetTags wraps collectionTreeRow.getTags(), which calls getSearchResults().
+			// If the underlying search query fails, getSearchResults() throws SearchError.
+			// The tag selector should catch this and return [] rather than throwing upwards and breaking the UI.
+			var collectionTreeRow = win.ZoteroPane.getCollectionTreeRow();
+			collectionTreeRow.clearCache();
+			var stub = sinon.stub(collectionTreeRow, 'getSearchObject').resolves({
+				search: () => { throw new Error('simulated search failure'); }
+			});
+			try {
+				var tags = await tagSelector._safeGetTags();
+				assert.isArray(tags);
+				assert.equal(tags.length, 0);
+			}
+			finally {
+				stub.restore();
+			}
+		});
+	});
 })

test/tests/virtualized-tableTest.js

@@ -0,0 +1,95 @@
+"use strict";
+
+describe("VirtualizedTable", function () {
+	let win, zp, itemsView;
+
+	before(async function () {
+		win = await loadZoteroPane();
+		zp = win.ZoteroPane;
+	});
+
+	beforeEach(async function () {
+		await selectLibrary(win);
+		itemsView = zp.itemsView;
+		await createDataObject('item');
+		await waitForItemsLoad(win);
+	});
+
+	after(function () {
+		win.close();
+	});
+
+	describe("#selectEventsSuppressed", function () {
+		it("should not trigger updates when set to false repeatedly", function () {
+			let selection = itemsView.selection;
+			selection.selectEventsSuppressed = false;
+
+			let updateSpy = sinon.spy(selection, '_updateTree');
+			let invalidateSpy = sinon.spy(itemsView.tree, 'invalidate');
+
+			try {
+				selection.selectEventsSuppressed = false;
+				assert.equal(updateSpy.callCount, 0);
+				assert.equal(invalidateSpy.callCount, 0);
+			}
+			finally {
+				updateSpy.restore();
+				invalidateSpy.restore();
+			}
+		});
+
+		it("should trigger updates when changed from true to false", function () {
+			let selection = itemsView.selection;
+			selection.selectEventsSuppressed = true;
+
+			let updateSpy = sinon.spy(selection, '_updateTree');
+			let invalidateSpy = sinon.spy(itemsView.tree, 'invalidate');
+
+			try {
+				selection.selectEventsSuppressed = false;
+				assert.equal(updateSpy.callCount, 1);
+				assert.equal(invalidateSpy.callCount, 1);
+			}
+			finally {
+				updateSpy.restore();
+				invalidateSpy.restore();
+			}
+		});
+	});
+
+	describe("VirtualizedTree rendering", function () {
+		it("should render tree indentation and ARIA attributes", async function () {
+			let parentItem = await createDataObject('item', { title: 'Parent Item' });
+			let attachment = await importFileAttachment('test.pdf', { parentItemID: parentItem.id });
+			let annotation = await createAnnotation('highlight', attachment);
+
+			await waitForItemsLoad(win);
+			await itemsView.selectItem(annotation.id);
+			
+			let parentIndex = itemsView.getRowIndexByID(parentItem.id);
+			let attachmentIndex = itemsView.getRowIndexByID(attachment.id);
+			let annotationIndex = itemsView.getRowIndexByID(annotation.id);
+			
+			let parentNode = itemsView.tree._renderItem(parentIndex);
+			let attachmentNode = itemsView.tree._renderItem(attachmentIndex);
+			let annotationNode = itemsView.tree._renderItem(annotationIndex);
+			
+			assert.equal(parentNode.getAttribute('role'), 'treeitem');
+			assert.equal(attachmentNode.getAttribute('role'), 'treeitem');
+			assert.equal(annotationNode.getAttribute('role'), 'treeitem');
+			
+			assert.equal(parentNode.getAttribute('aria-level'), '1');
+			assert.equal(attachmentNode.getAttribute('aria-level'), '2');
+			assert.equal(annotationNode.getAttribute('aria-level'), '3');
+			
+			assert.equal(parentNode.getAttribute('aria-expanded'), 'true');
+			assert.equal(attachmentNode.getAttribute('aria-expanded'), 'true');
+			assert.isNull(annotationNode.getAttribute('aria-expanded'));
+			
+			let getIndent = (node) => parseInt(node.querySelector('.cell-indent').style.paddingInlineStart || 0);
+			assert.equal(getIndent(parentNode), 0);
+			assert.equal(getIndent(attachmentNode), 16);
+			assert.equal(getIndent(annotationNode), 32);
+		});
+	});
+});