Fix a few test failures/warnings

And simplify tree view load event handling, which may or may not have
been contributing to intermittent test failures, but is cleaner this way
regardless.

chrome/content/zotero/xpcom/collectionTreeView.js

@@ -909,7 +909,7 @@ Zotero.CollectionTreeView.prototype.selectLibrary = Zotero.Promise.coroutine(fun
 	var row = this._rowMap['L' + libraryID];
 	if (row !== undefined) {
 		this._treebox.ensureRowIsVisible(row);
-		this.selectWait(row);
+		yield this.selectWait(row);
 		return true;
 	}
 	
@@ -1621,7 +1621,9 @@ Zotero.CollectionTreeView.prototype.drop = Zotero.Promise.coroutine(function* (r
 				yield linkedItem.loadCollections();
 				linkedItem.setCollections();
 				linkedItem.deleted = false;
-				yield linkedItem.save();
+				yield linkedItem.save({
+					skipSelect: true
+				});
 			}
 			return linkedItem.id;
 			
@@ -1691,7 +1693,9 @@ Zotero.CollectionTreeView.prototype.drop = Zotero.Promise.coroutine(function* (r
 			}
 		}
 		
-		var newItemID = yield newItem.save();
+		var newItemID = yield newItem.save({
+			skipSelect: true
+		});
 		
 		// Record link
 		yield newItem.addLinkedItem(item);
@@ -1710,7 +1714,9 @@ Zotero.CollectionTreeView.prototype.drop = Zotero.Promise.coroutine(function* (r
 			for each(var note in notes) {
 				let newNote = yield note.clone(targetLibraryID);
 				newNote.parentID = newItemID;
-				yield newNote.save()
+				yield newNote.save({
+					skipSelect: true
+				})
 				
 				yield newNote.addLinkedItem(note);
 			}

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

@@ -528,14 +528,16 @@ Zotero.DataObject.prototype._addLinkedObject = Zotero.Promise.coroutine(function
 	if (this.libraryID == userLibraryID || object.libraryID != userLibraryID) {
 		this.addRelation(predicate, objectURI);
 		yield this.save({
-			skipDateModifiedUpdate: true
+			skipDateModifiedUpdate: true,
+			skipSelect: true
 		});
 	}
 	else {
 		yield object.loadRelations();
 		object.addRelation(predicate, thisURI);
 		yield object.save({
-			skipDateModifiedUpdate: true
+			skipDateModifiedUpdate: true,
+			skipSelect: true
 		});
 	}
 	

chrome/content/zotero/xpcom/libraryTreeView.js

@@ -41,7 +41,7 @@ Zotero.LibraryTreeView.prototype = {
 		if (event == 'load') {
 			// If already initialized run now
 			if (this._initialized) {
-				listener();
+				listener.call(this);
 			}
 			else {
 				this._listeners[event].push(listener);
@@ -60,7 +60,7 @@ Zotero.LibraryTreeView.prototype = {
 		if (!this._listeners[event]) return;
 		var listener;
 		while (listener = this._listeners[event].shift()) {
-			yield Zotero.Promise.resolve(listener());
+			yield Zotero.Promise.resolve(listener.call(this));
 		}
 	}),
 	

chrome/content/zotero/zoteroPane.js

@@ -1123,9 +1123,7 @@ var ZoteroPane = new function()
 				//
 				// TODO: Cancel loading
 				let deferred = Zotero.Promise.defer();
-				this.itemsView.addEventListener('load', function () {
+				this.itemsView.addEventListener('load', () => deferred.resolve());
-					deferred.resolve();
-				});
 				if (deferred.promise.isPending()) {
 					Zotero.debug("Waiting for items view " + this.itemsView.id + " to finish loading");
 					yield deferred.promise;
@@ -1192,13 +1190,6 @@ var ZoteroPane = new function()
 			this.itemsView.onError = function () {
 				ZoteroPane_Local.displayErrorMessage();
 			};
-			// If any queued load listeners, set them to run when the tree is ready
-			if (this._listeners.itemsLoaded) {
-				let listener;
-				while (listener = this._listeners.itemsLoaded.shift()) {
-					this.itemsView.addEventListener('load', listener);
-				}
-			}
 			this.itemsView.addEventListener('load', this.setTagScope);
 			document.getElementById('zotero-items-tree').view = this.itemsView;
 			
@@ -2047,81 +2038,48 @@ var ZoteroPane = new function()
 			throw new Error("Collections view not loaded");
 		}
 		
-		var self = this;
+		var cv = this.collectionsView;
-		var deferred = Zotero.Promise.defer();
-		this.collectionsView.addEventListener('load', function () {
-			Zotero.spawn(function* () {
-				try {
-					self.addEventListener('itemsLoaded', function () {
-						Zotero.spawn(function* () {
-							var selected = yield self.itemsView.selectItem(itemID, expand);
-							if (!selected) {
-								if (item.deleted) {
-									Zotero.debug("Item is deleted; switching to trash");
-									yield self.collectionsView.selectTrash(item.libraryID);
-								}
-								else {
-									Zotero.debug("Item was not selected; switching to library");
-									yield self.collectionsView.selectLibrary(item.libraryID);
-								}
-								yield self.itemsView.selectItem(itemID, expand);
-							}
-							deferred.resolve(true);
-						})
-						.catch(function(e) {
-							deferred.reject(e);
-						});
-					});
 		
-					var currentLibraryID = self.getSelectedLibraryID();
+		var deferred = Zotero.Promise.defer();
-					// If in a different library
+		cv.addEventListener('load', () => deferred.resolve());
-					if (item.libraryID != currentLibraryID) {
+		yield deferred.promise;
-						Zotero.debug("Library ID differs; switching library");
+		
-						yield self.collectionsView.selectLibrary(item.libraryID);
+		var currentLibraryID = this.getSelectedLibraryID();
-					}
+		// If in a different library
-					// Force switch to library view
+		if (item.libraryID != currentLibraryID) {
-					else if (!self.collectionsView.selectedTreeRow.isLibrary() && inLibrary) {
+			Zotero.debug("Library ID differs; switching library");
-						Zotero.debug("Told to select in library; switching to library");
+			yield cv.selectLibrary(item.libraryID);
-						yield self.collectionsView.selectLibrary(item.libraryID);
+		}
-					}
+		// Force switch to library view
-				}
+		else if (!cv.selectedTreeRow.isLibrary() && inLibrary) {
-				catch (e) {
+			Zotero.debug("Told to select in library; switching to library");
-					deferred.reject(e);
+			yield cv.selectLibrary(item.libraryID);
-				}
+		}
-			})
+		
-		});
+		deferred = Zotero.Promise.defer();
+		this.itemsView.addEventListener('load', () => deferred.resolve());
+		yield deferred.promise;
+		
+		var selected = yield this.itemsView.selectItem(itemID, expand);
+		if (!selected) {
+			if (item.deleted) {
+				Zotero.debug("Item is deleted; switching to trash");
+				yield cv.selectTrash(item.libraryID);
+			}
+			else {
+				Zotero.debug("Item was not selected; switching to library");
+				yield cv.selectLibrary(item.libraryID);
+			}
+			
+			deferred = Zotero.Promise.defer();
+			this.itemsView.addEventListener('load', () => deferred.resolve());
+			yield deferred.promise;
+			
+			yield this.itemsView.selectItem(itemID, expand);
+		}
 		
 		// open Zotero pane
 		this.show();
-		
-		return deferred.promise;
-	});
-	
-	
-	this.addEventListener = function (event, listener) {
-		if (event == 'itemsLoaded') {
-			if (this.itemsView) {
-				this.itemsView.addEventListener('load', listener);
-			}
-			else {
-				if (!this._listeners.itemsLoaded) {
-					this._listeners.itemsLoaded = [];
-				}
-				this._listeners.itemsLoaded.push(listener);
-			}
-		}
-	};
-	
-	
-	this._runListeners = Zotero.Promise.coroutine(function* (event) {
-		if (!this._listeners[event]) {
-			return;
-		}
-		
-		var listener;
-		while (listener = this._listeners[event].shift()) {
-			yield Zotero.Promise.resolve(listener());
-		}
 	});
 	
 	

test/content/support.js

@@ -95,21 +95,20 @@ var selectLibrary = Zotero.Promise.coroutine(function* (win, libraryID) {
 	yield waitForItemsLoad(win);
 });
 
-var waitForItemsLoad = function (win, collectionRowToSelect) {
+var waitForItemsLoad = Zotero.Promise.coroutine(function* (win, collectionRowToSelect) {
-	var resolve;
-	var promise = new Zotero.Promise(() => resolve = arguments[0]);
 	var zp = win.ZoteroPane;
 	var cv = zp.collectionsView;
-	cv.addEventListener('load', function () {
+	
-		if (collectionRowToSelect !== undefined) {
+	var deferred = Zotero.Promise.defer();
-			cv.selection.select(collectionRowToSelect);
+	cv.addEventListener('load', () => deferred.resolve());
-		}
+	yield deferred.promise;
-		zp.addEventListener('itemsLoaded', function () {
+	if (collectionRowToSelect !== undefined) {
-			resolve();
+		yield cv.selectWait(collectionRowToSelect);
-		});
+	}
-	});
+	deferred = Zotero.Promise.defer();
-	return promise;
+	zp.itemsView.addEventListener('load', () => deferred.resolve());
-}
+	return deferred.promise;
+});
 
 /**
  * Waits for a single item event. Returns a promise for the item ID(s).

test/tests/collectionTreeViewTest.js

@@ -54,7 +54,7 @@ describe("Zotero.CollectionTreeView", function() {
 			var collection1 = yield createDataObject('collection');
 			var collection2 = createUnsavedDataObject('collection');
 			collection2.parentID = collection1.id;
-			yield collection2.save({
+			yield collection2.saveTx({
 				skipSelect: true
 			});
 			var row = cv.getRowIndexByID("C" + collection1.id);
@@ -279,6 +279,7 @@ describe("Zotero.CollectionTreeView", function() {
 			var ids = yield drop("C" + collection.id, [item.id], deferred.promise);
 			
 			Zotero.Notifier.unregisterObserver(observerID);
+			
 			yield collectionsView.selectCollection(collection.id);
 			yield waitForItemsLoad(win);
 			
@@ -289,7 +290,7 @@ describe("Zotero.CollectionTreeView", function() {
 		})
 		
 		it("should copy an item with an attachment to a group", function* () {
-			var group = yield getGroup();
+			var group = yield createGroup();
 			
 			var item = yield createDataObject('item', false, { skipSelect: true });
 			var file = getTestDataDirectory();
@@ -310,15 +311,27 @@ describe("Zotero.CollectionTreeView", function() {
 			yield collectionsView.selectLibrary(group.libraryID);
 			yield waitForItemsLoad(win);
 			
-			var itemsView = win.ZoteroPane.itemsView
+			// Check parent
+			var itemsView = win.ZoteroPane.itemsView;
 			assert.equal(itemsView.rowCount, 1);
 			var treeRow = itemsView.getRow(0);
 			assert.equal(treeRow.ref.libraryID, group.libraryID);
 			assert.equal(treeRow.ref.id, ids[0]);
-			
 			// New item should link back to original
 			var linked = yield item.getLinkedItem(group.libraryID);
 			assert.equal(linked.id, treeRow.ref.id);
+			
+			// Check attachment
+			assert.isTrue(itemsView.isContainer(0));
+			yield itemsView.toggleOpenState(0);
+			assert.equal(itemsView.rowCount, 2);
+			treeRow = itemsView.getRow(1);
+			assert.equal(treeRow.ref.id, ids[1]);
+			// New attachment should link back to original
+			linked = yield attachment.getLinkedItem(group.libraryID);
+			assert.equal(linked.id, treeRow.ref.id);
+			
+			yield group.erase()
 		})
 		
 		it("should not copy an item or its attachment to a group twice", function* () {
@@ -334,7 +347,7 @@ describe("Zotero.CollectionTreeView", function() {
 			});
 			var attachmentTitle = Zotero.Utilities.randomString();
 			attachment.setField('title', attachmentTitle);
-			yield attachment.save();
+			yield attachment.saveTx();
 			
 			var ids = yield drop("L" + group.libraryID, [item.id]);
 			assert.isFalse(yield canDrop("L" + group.libraryID, [item.id]));
@@ -350,7 +363,7 @@ describe("Zotero.CollectionTreeView", function() {
 			var droppedItem = yield item.getLinkedItem(group.libraryID);
 			droppedItem.setCollections([collection.id]);
 			droppedItem.deleted = true;
-			yield droppedItem.save();
+			yield droppedItem.saveTx();
 			
 			// Add observer to wait for collection add
 			var deferred = Zotero.Promise.defer();