Clear stuck drag-and-drop row highlight in trees

If a drag ended without the tree receiving a dragleave, drop, or dragend
(e.g., a drag that left the window), the drop-target row kept its
highlight until restart, and, being index-based, could move to other
rows as the view changed.

Clear the indicator on the next mousemove and whenever the rows in
the items list change.

This will hopefully fix https://forums.zotero.org/discussion/133039/,
though I couldn't reproduce it.

(cherry picked from commit 896a3f829a)

chrome/content/zotero/collectionViewItemTree.jsx

@@ -1594,7 +1594,7 @@ class CollectionViewItemTree extends ItemTree {
 			row = 0;
 			orient = -1;
 		}
-		this._dropRow = null;
+		this._clearDropIndicator();
 		Zotero.DragDrop.currentDragSource = null;
 		// Use the effect set in onDragOver(), which the drop event's dropEffect may not reflect
 		// (see LibraryTreeView::setDropEffect())

chrome/content/zotero/itemTree.jsx

@@ -1149,22 +1149,10 @@ var ItemTree = class ItemTree extends LibraryTree {
 	}
 
 	componentDidMount() {
+		super.componentDidMount();
 		this._initialized = true;
 		this._loadingDeferredResolved = true;
 		this._itemTreeLoadingDeferred.resolve();
-		// Create an element where we can create drag images to be displayed next to the cursor while dragging
-		// since for multiple item drags we need to display all the elements
-		let elem = this._dragImageContainer = document.createElement("div");
-		elem.style.width = "100%";
-		elem.style.height = "2000px";
-		elem.style.position = "absolute";
-		elem.style.top = "-10000px";
-		elem.className = "drag-image-container";
-		this.domEl.appendChild(elem);
-	}
-	
-	componentWillUnmount() {
-		this.domEl.removeChild(this._dragImageContainer);
 	}
 	
 	
@@ -1258,6 +1246,9 @@ var ItemTree = class ItemTree extends LibraryTree {
 		loading: false,
 		message: null,
 	}) {
+		// Row indexes are changing, so clear any drop highlight, which is index-based
+		this._clearDropIndicator();
+
 		// Handle loading/message state
 		if (options.loading) {
 			options.message ||= Zotero.getString('pane.items.loading');

chrome/content/zotero/libraryTree.js

@@ -66,12 +66,29 @@ var LibraryTree = class LibraryTree extends React.Component {
 		elem.style.top = "-10000px";
 		elem.className = "drag-image-container";
 		this.domEl.appendChild(elem);
+		// Normal mousemove events are suppressed during native drag-and-drop, so a mousemove
+		// with a drop row still set means the drag ended without a dragleave/drop/dragend
+		// reaching the tree (e.g., a drag that left the window). Clear the leftover drop
+		// indicator when mouse movement resumes.
+		this.domEl.addEventListener('mousemove', this._clearDropIndicator, { passive: true });
 	}
 
 	componentWillUnmount() {
 		this.domEl.removeChild(this._dragImageContainer);
+		this.domEl.removeEventListener('mousemove', this._clearDropIndicator);
 	}
 
+	_clearDropIndicator = () => {
+		let dropRow = this._dropRow;
+		if (dropRow === null || dropRow === undefined) return;
+		this._dropRow = null;
+		// The row may no longer exist if rows were removed, in which case the next full
+		// render drops it
+		if (this.tree && dropRow < this.rowCount) {
+			this.tree.invalidateRow(dropRow);
+		}
+	};
+
 	componentDidCatch(error, info) {
 		// Async operations might attempt to update the react components
 		// after window close in tests, which will cause unnecessary crashing

test/tests/libraryTreeTest.js

@@ -27,6 +27,75 @@ describe("Zotero.LibraryTree", function() {
 		});
 	});
 	
+	describe("#_clearDropIndicator()", function () {
+		// Simulate a drag pausing over a row, highlighting it as a drop target. The tests then
+		// send no further drag events, as when a drag ends outside the window without the tree
+		// receiving a dragleave, drop, or dragend.
+		function simulateDragOverRow(view, index, draggedItemID) {
+			var rowEl = {
+				classList: { contains: () => false },
+				getBoundingClientRect: () => ({ y: 0, height: 100 })
+			};
+			Zotero.DragDrop.currentDragSource = view.collectionTreeRows[0];
+			view.onDragOver({
+				preventDefault: () => {},
+				stopPropagation: () => {},
+				currentTarget: rowEl,
+				target: rowEl,
+				// Middle of the row, so the drop targets the row itself
+				clientY: 50,
+				dataTransfer: {
+					dropEffect: 'copy',
+					effectAllowed: 'copyMove',
+					types: ['zotero/item'],
+					getData: type => type == 'zotero/item' ? String(draggedItemID) : ""
+				}
+			}, index);
+			Zotero.DragDrop.currentDragSource = null;
+		}
+
+		it("should clear a leftover drop highlight on mousemove", async function () {
+			var item = await createDataObject('item');
+			var note = new Zotero.Item('note');
+			note.setNote('A note');
+			await note.saveTx();
+			var view = zp.itemsView;
+			var row = view.getRowIndexByID(item.treeViewID);
+
+			simulateDragOverRow(view, row, note.id);
+			assert.equal(view._dropRow, row);
+			assert.ok(view.domEl.querySelector('.row.drop'));
+
+			view.domEl.dispatchEvent(new win.MouseEvent('mousemove', { bubbles: true }));
+
+			assert.isNull(view._dropRow);
+			assert.notOk(view.domEl.querySelector('.row.drop'));
+		});
+
+		it("should clear the drop highlight when the highlighted last row is removed", async function () {
+			var collection = await createDataObject('collection');
+			await select(win, collection);
+			await createDataObject('item', { collections: [collection.id] });
+			await createDataObject('item', { collections: [collection.id] });
+			var note = new Zotero.Item('note');
+			note.setNote('A note');
+			await note.saveTx();
+			var view = zp.itemsView;
+			var lastRow = view.rowCount - 1;
+			var lastItem = Zotero.Items.get(view.getRow(lastRow).id);
+
+			simulateDragOverRow(view, lastRow, note.id);
+			assert.equal(view._dropRow, lastRow);
+			assert.ok(view.domEl.querySelector('.row.drop'));
+
+			await Zotero.Items.trashTx([lastItem.id]);
+
+			assert.equal(view.rowCount, 1);
+			assert.isNull(view._dropRow);
+			assert.notOk(view.domEl.querySelector('.row.drop'));
+		});
+	});
+
 	describe("#_removeRow()", function () {
 		it("should remove the last row", async function () {
 			var collection = await createDataObject('collection');