From 78c3d5808bdd8018d46e4bd312577520b77bf6b5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Adomas=20Ven=C4=8Dkauskas?= <adomas.ven@gmail.com>
Date: Mon, 8 Apr 2019 16:43:29 +0300
Subject: [PATCH] Fix citeproc errors due to improper citeproc state updates

The error is triggered upon initial interaction with a doc after Zotero
restart or if new external citations (copied into the document) are
peresnt and `session.updateSession()` is called without a subsequent
`session.updateDocument()` call. `session.updateSession()` is called
without a subsequent `session.updateDocument()` call every time the
user cancels a citation insert.

More specifically, `session.updateSession()` is called every time a
citation dialog is invoked. It retrieves all citations and writes them
into a local `session.citationsByIndex` object. Moreover, it marks
each citation that hasn't seen before in a `session.newIndices` object.
`session.newIndices` is there to ensure that we load every new citation
into citeproc upon document update. This object is built by marking any
citation that does not appear in the previous invocation's list of
citations as new. However, if the document is never updated (because the
user cancels the insertion) then the new indices are not loaded
into citeproc. This commit fixes that, by excluding citeproc unloaded
items from the previous invocation's citation list.
---
 chrome/content/zotero/xpcom/integration.js | 22 +++++++++------
 test/tests/integrationTest.js              | 32 ++++++++++++++++++++--
 2 files changed, 44 insertions(+), 10 deletions(-)

diff --git a/chrome/content/zotero/xpcom/integration.js b/chrome/content/zotero/xpcom/integration.js
index e27034acbf..1e37684303 100644
--- a/chrome/content/zotero/xpcom/integration.js
+++ b/chrome/content/zotero/xpcom/integration.js
@@ -1381,6 +1381,19 @@ Zotero.Integration.Session.prototype.resetRequest = function(doc) {
 	
 	this.bibliographyHasChanged = false;
 	this.bibliographyDataHasChanged = false;
+
+	// When processing citations this list will be checked for citations that are new to the document
+	// (i.e. copied from somewhere else) and marked as newIndices to be processed with citeproc if
+	// not present
+	this.oldCitations = new Set();
+	for (let i in this.citationsByIndex) {
+		// But ignore indices from this.newIndices. If any are present it means that the last
+		// call to this.updateSession() was never followed up with this.updateDocument()
+		// i.e. the operation was user cancelled
+		if (i in this.newIndices) continue;
+		this.oldCitations.add(this.citationsByIndex[i].citationID);
+	}
+	
 	// After adding fields to the session
 	// citations that are new to the document will be marked
 	// as new,  so that they are correctly loaded into and processed with citeproc
@@ -1390,15 +1403,8 @@ Zotero.Integration.Session.prototype.resetRequest = function(doc) {
 	this.updateIndices = {};
 	// Citations that require updating in the document will be marked in
 	// processIndices
+	
 	this.processIndices = {};
-
-	// When processing citations this list will be checked for citations that are new to the document
-	// (i.e. copied from somewhere else) and marked as newIndices to be processed with citeproc if
-	// not present
-	this.oldCitations = new Set();
-	for (let i in this.citationsByIndex) {
-		this.oldCitations.add(this.citationsByIndex[i].citationID);
-	}
 	this.citationsByItemID = {};
 	this.citationsByIndex = {};
 	this.documentCitationIDs = {};
diff --git a/test/tests/integrationTest.js b/test/tests/integrationTest.js
index a537425a72..4b757dd85c 100644
--- a/test/tests/integrationTest.js
+++ b/test/tests/integrationTest.js
@@ -138,7 +138,7 @@ describe("Zotero.Integration", function () {
 		/**
 		 * Deletes this field and its contents.
 		 */
-		delete: function() {this.doc.fields.filter((field) => field !== this)},
+		delete: function() {this.doc.fields = this.doc.fields.filter((field) => field !== this)},
 		/**
 		 * Removes this field, but maintains the field's contents.
 		 */
@@ -268,7 +268,10 @@ describe("Zotero.Integration", function () {
 				item = Zotero.Cite.getItem(item.id);
 				return {id: item.id, uris: item.cslURIs, itemData: item.cslItemData};
 			});
-			await io.previewFn(io.citation);
+			try {
+				await io.previewFn(io.citation);
+			}
+			catch (e) {}
 			io._acceptDeferred.resolve(() => {});
 		};
 	}
@@ -715,6 +718,31 @@ describe("Zotero.Integration", function () {
 						stubUpdateDocument.restore();
 					}
 				});
+				
+				it('should successfully insert a citation after canceled citation insert', async function () {
+					var docID = this.test.fullTitle();
+					if (!(docID in applications)) initDoc(docID);
+					var doc = applications[docID].doc;
+
+					setAddEditItems(testItems[0]);
+					await execCommand('addEditCitation', docID);
+					assert.equal(doc.fields.length, 1);
+					doc.fields.push(new DocumentPluginDummy.Field(doc));
+					// Add a "citation copied from somewhere else"
+					// the content doesn't really matter, just make sure that the citationID is different
+					var newCitationID = Zotero.Utilities.randomString();
+					doc.fields[1].code = doc.fields[0].code;
+					doc.fields[1].code = doc.fields[1].code.replace(/"citationID":"[A-Za-z0-9^"]*"/,
+						`"citationID":"${newCitationID}"`);
+					doc.fields[1].text = doc.fields[0].text;
+					
+					setAddEditItems([]);
+					await execCommand('addEditCitation', docID);
+
+					setAddEditItems(testItems[1]);
+					await execCommand('addEditCitation', docID);
+					assert.notEqual(doc.fields[2].code, "TEMP");
+				});
 			});
 			
 			describe('when delayCitationUpdates is set', function() {