From 1b9811c31de63102e5ef29bd5e4cedf880cba2ed Mon Sep 17 00:00:00 2001
From: Dan Stillman <dstillman@zotero.org>
Date: Sat, 6 Oct 2018 01:38:32 -0400
Subject: [PATCH] Fix test failures after 18f79f9796

---
 chrome/content/zotero/xpcom/attachments.js    |  5 ++--
 .../zotero/xpcom/progressQueueDialog.js       | 23 +++++++++++--------
 test/tests/attachmentsTest.js                 | 22 ++++++++++++------
 3 files changed, 30 insertions(+), 20 deletions(-)

diff --git a/chrome/content/zotero/xpcom/attachments.js b/chrome/content/zotero/xpcom/attachments.js
index cc827f35f5..d37e2d93ce 100644
--- a/chrome/content/zotero/xpcom/attachments.js
+++ b/chrome/content/zotero/xpcom/attachments.js
@@ -1228,20 +1228,19 @@ Zotero.Attachments = new function(){
 				// We reached the end of the queue
 				if (!current) {
 					// If all entries are resolved, we're done
-					if (queue.every(x => x instanceof Zotero.Item || x.result === false)) {
+					if (queue.every(x => x.result instanceof Zotero.Item || x.result === false)) {
 						resolve();
 						return;
 					}
 					
 					// Otherwise, wait until the next time a pending request is ready to process
-					// and restart
+					// and restart. If no pending requests, they're all in progress.
 					let nextStart = queue
 						.map(x => x.result === null && getDomainInfo(x.domain).nextRequestTime)
 						.filter(x => x)
 						.reduce((accumulator, currentValue) => {
 							return currentValue < accumulator ? currentValue : accumulator;
 						});
-					
 					i = 0;
 					setTimeout(processNextItem, Math.max(0, nextStart - Date.now()));
 					return;
diff --git a/chrome/content/zotero/xpcom/progressQueueDialog.js b/chrome/content/zotero/xpcom/progressQueueDialog.js
index 985426a886..78f63685f5 100644
--- a/chrome/content/zotero/xpcom/progressQueueDialog.js
+++ b/chrome/content/zotero/xpcom/progressQueueDialog.js
@@ -69,9 +69,14 @@ Zotero.ProgressQueueDialog = function (progressQueue) {
 		_showMinimize = show;
 	};
 	
-	function close() {
+	this.close = function () {
+		// In case close() is called before open()
+		if (!_progressWindow) {
+			return;
+		}
 		_progressWindow.close();
-	}
+		_progressQueue.cancel();
+	};
 	
 	function _getImageByStatus(status) {
 		if (status === Zotero.ProgressQueue.ROW_PROCESSING) {
@@ -140,20 +145,18 @@ Zotero.ProgressQueueDialog = function (progressQueue) {
 		
 		_progressIndicator = _progressWindow.document.getElementById('progress-indicator');
 		_progressWindow.document.getElementById('cancel-button')
-			.addEventListener('command', function () {
-				close();
-				_progressQueue.cancel();
+			.addEventListener('command', () => {
+				this.close();
 			}, false);
 		
 		_progressWindow.document.getElementById('minimize-button')
 			.addEventListener('command', function () {
-				close();
+				_progressWindow.close();
 			}, false);
 		
 		_progressWindow.document.getElementById('close-button')
-			.addEventListener('command', function () {
-				close();
-				_progressQueue.cancel();
+			.addEventListener('command', () => {
+				this.close();
 			}, false);
 		
 		_progressWindow.addEventListener('keypress', function (e) {
@@ -162,7 +165,7 @@ Zotero.ProgressQueueDialog = function (progressQueue) {
 				if (_progressQueue.getTotal() === _progressQueue.getProcessedTotal()) {
 					_progressQueue.cancel();
 				}
-				close();
+				_progressWindow.close();
 			}
 		});
 		
diff --git a/test/tests/attachmentsTest.js b/test/tests/attachmentsTest.js
index ac96e48b05..d97897dec5 100644
--- a/test/tests/attachmentsTest.js
+++ b/test/tests/attachmentsTest.js
@@ -583,6 +583,12 @@ describe("Zotero.Attachments", function() {
 				httpd.stop(() => resolve());
 			});
 			Zotero.Prefs.clear('findPDFs.resolvers');
+			
+			// Close progress dialog after each run
+			var queue = Zotero.ProgressQueues.get('findPDF');
+			if (queue) {
+				queue.getDialog().close();
+			}
 		}.bind(this));
 		
 		after(() => {
@@ -751,8 +757,9 @@ describe("Zotero.Attachments", function() {
 			
 			assert.isTrue(requestStub.calledTwice);
 			assert.isAbove(requestStubCallTimes[1] - requestStubCallTimes[0], 1000);
-			// Make sure there's an attachment for every item
-			assert.lengthOf(attachments.filter(x => x), 2);
+			// Make sure both items have attachments
+			assert.equal(item1.numAttachments(), 1);
+			assert.equal(item2.numAttachments(), 1);
 		});
 		
 		it("should wait between requests that resolve to the same domain", async function () {
@@ -793,9 +800,9 @@ describe("Zotero.Attachments", function() {
 			// 'website' requests should be a second apart
 			assert.isAbove(requestStubCallTimes[5] - requestStubCallTimes[1], 1000);
 			
-			assert.instanceOf(attachments[0], Zotero.Item);
-			assert.isFalse(attachments[1]);
-			assert.instanceOf(attachments[2], Zotero.Item);
+			assert.equal(item1.numAttachments(), 1);
+			assert.equal(item2.numAttachments(), 0);
+			assert.equal(item3.numAttachments(), 1);
 		});
 		
 		it("should wait between requests to the same domain after a 429", async function () {
@@ -818,8 +825,9 @@ describe("Zotero.Attachments", function() {
 			assert.equal(requestStub.getCall(1).args[1], pageURL9);
 			assert.equal(requestStub.getCall(2).args[1], pageURL3);
 			assert.isAbove(requestStubCallTimes[1] - requestStubCallTimes[0], 2000);
-			// Make sure there's an attachment for every item
-			assert.lengthOf(attachments.filter(x => x), 2);
+			// Make sure both items have attachments
+			assert.equal(item1.numAttachments(), 1);
+			assert.equal(item2.numAttachments(), 1);
 		});
 		
 		it("should handle a custom resolver in HTML mode", async function () {