commit 34455197149ce15ea24156804e8412fcf2b7b9f5
parent e746da4faba46533f35db7cbe04c91c757134a44
Author: Adomas Venčkauskas <adomas.ven@gmail.com>
Date: Mon, 26 Mar 2018 15:31:26 +0300
Fix citationByIndex[i].sortedItem errors during citation insertion
Caused by inproper handling of copy-pasted citations in documents
Diffstat:
2 files changed, 90 insertions(+), 35 deletions(-)
diff --git a/chrome/content/zotero/xpcom/integration.js b/chrome/content/zotero/xpcom/integration.js
@@ -565,7 +565,7 @@ Zotero.Integration.Interface.prototype.editBibliography = Zotero.Promise.corouti
throw new Zotero.Exception.Alert("integration.error.mustInsertBibliography",
[], "integration.error.title");
}
- let bibliography = new Zotero.Integration.Bibliography(bibliographyField);
+ let bibliography = new Zotero.Integration.Bibliography(bibliographyField, bibliographyField.unserialize());
var citationsMode = FORCE_CITATIONS_FALSE;
if(this._session.data.prefs.delayCitationUpdates) {
// Refreshes citeproc state before proceeding
@@ -604,7 +604,7 @@ Zotero.Integration.Interface.prototype.addEditBibliography = Zotero.Promise.coro
bibliographyField.clearCode();
}
- let bibliography = new Zotero.Integration.Bibliography(bibliographyField);
+ let bibliography = new Zotero.Integration.Bibliography(bibliographyField, bibliographyField.unserialize());
var citationsMode = FORCE_CITATIONS_FALSE;
if(this._session.data.prefs.delayCitationUpdates) {
// Refreshes citeproc state before proceeding
@@ -893,7 +893,8 @@ Zotero.Integration.Fields.prototype._processFields = Zotero.Promise.coroutine(fu
let field = Zotero.Integration.Field.loadExisting(this._fields[i]);
if (field.type === INTEGRATION_TYPE_ITEM) {
var noteIndex = field.getNoteIndex(),
- citation = new Zotero.Integration.Citation(field, noteIndex);
+ data = field.unserialize(),
+ citation = new Zotero.Integration.Citation(field, noteIndex, data);
yield this._session.addCitation(i, noteIndex, citation);
} else if (field.type === INTEGRATION_TYPE_BIBLIOGRAPHY) {
@@ -905,7 +906,8 @@ Zotero.Integration.Fields.prototype._processFields = Zotero.Promise.coroutine(fu
}
}
if (this._bibliographyFields.length) {
- this._session.bibliography = new Zotero.Integration.Bibliography(this._bibliographyFields[0]);
+ var data = this._bibliographyFields[0].unserialize()
+ this._session.bibliography = new Zotero.Integration.Bibliography(this._bibliographyFields[0], data);
yield this._session.bibliography.loadItemData();
} else {
delete this._session.bibliography;
@@ -1118,6 +1120,7 @@ Zotero.Integration.Fields.prototype._updateDocument = async function(forceCitati
*/
Zotero.Integration.Fields.prototype.addEditCitation = Zotero.Promise.coroutine(function* (field) {
var newField;
+ var citation;
if (field) {
field = Zotero.Integration.Field.loadExisting(field);
@@ -1125,13 +1128,13 @@ Zotero.Integration.Fields.prototype.addEditCitation = Zotero.Promise.coroutine(f
if (field.type != INTEGRATION_TYPE_ITEM) {
throw new Zotero.Exception.Alert("integration.error.notInCitation");
}
+ citation = new Zotero.Integration.Citation(field, field.getNoteIndex(), field.unserialize());
} else {
newField = true;
field = new Zotero.Integration.CitationField(yield this.addField(true));
- field.clearCode();
+ citation = new Zotero.Integration.Citation(field);
}
- var citation = new Zotero.Integration.Citation(field);
yield citation.prepareForEditing();
// -------------------
@@ -1158,19 +1161,31 @@ Zotero.Integration.Fields.prototype.addEditCitation = Zotero.Promise.coroutine(f
}.bind(this));
}
- var previewFn = Zotero.Promise.coroutine(function* (citation) {
- let idx = yield fieldIndexPromise;
- yield citationsByItemIDPromise;
+ var previewFn = async function (citation) {
+ let idx = await fieldIndexPromise;
+ await citationsByItemIDPromise;
+ var fields = await this.get();
- var [citations, fieldToCitationIdxMapping, citationToFieldIdxMapping] = this._session.getCiteprocLists();
- let citationsPre = citations.slice(0, fieldToCitationIdxMapping[idx]);
- let citationsPost = citations.slice(fieldToCitationIdxMapping[idx]+1);
+ var [citations, fieldToCitationIdxMapping, citationToFieldIdxMapping] = this._session.getCiteprocLists(true);
+ for (var prevIdx = idx-1; prevIdx >= 0; prevIdx--) {
+ if (prevIdx in fieldToCitationIdxMapping) break;
+ }
+ for (var nextIdx = idx+1; nextIdx < fields.length; nextIdx++) {
+ if (nextIdx in fieldToCitationIdxMapping) break;
+ }
+ let citationsPre = citations.slice(0, fieldToCitationIdxMapping[prevIdx]+1);
+ let citationsPost = citations.slice(fieldToCitationIdxMapping[nextIdx]);
try {
- return this._session.style.previewCitationCluster(citation, citationsPre, citationsPost, "rtf");
+ var result = this._session.style.previewCitationCluster(citation, citationsPre, citationsPost, "rtf");
} catch(e) {
throw e;
+ } finally {
+ // CSL.previewCitationCluster() sets citationID, which means that we do not mark it
+ // as a new citation in Session.addCitation() if the ID is still present
+ delete citation.citationID;
}
- }.bind(this));
+ return result;
+ }.bind(this);
var io = new Zotero.Integration.CitationEditInterface(
citation, this._session.style.opt.sort_citations,
@@ -1528,8 +1543,15 @@ Zotero.Integration.Session.prototype.addCitation = Zotero.Promise.coroutine(func
}
// We need a new ID if there's another citation with the same citation ID in this document
- var needNewID = !citation.citationID || this.documentCitationIDs[citation.citationID];
+ var duplicateIndex = this.documentCitationIDs[citation.citationID];
+ var needNewID = !citation.citationID || duplicateIndex != undefined;
if(needNewID || this.regenAll) {
+ if (duplicateIndex != undefined) {
+ // If this is a duplicate, we need to mark both citations as "new"
+ // since we do not know which one was the "original" one
+ // and either one may need to be updated
+ this.newIndices[duplicateIndex] = true;
+ }
if(needNewID) {
Zotero.debug("Integration: "+citation.citationID+" ("+index+") needs new citationID");
citation.citationID = Zotero.randomString();
@@ -1537,17 +1559,21 @@ Zotero.Integration.Session.prototype.addCitation = Zotero.Promise.coroutine(func
this.newIndices[index] = true;
}
Zotero.debug("Integration: Adding citationID "+citation.citationID);
- this.documentCitationIDs[citation.citationID] = citation.citationID;
+ this.documentCitationIDs[citation.citationID] = index;
});
-Zotero.Integration.Session.prototype.getCiteprocLists = function() {
+Zotero.Integration.Session.prototype.getCiteprocLists = function(excludeNew) {
var citations = [];
- var fieldToCitationIdxMapping = [];
+ var fieldToCitationIdxMapping = {};
var citationToFieldIdxMapping = {};
var i = 0;
// This relies on the order of citationsByIndex keys being stable and sorted in ascending order
// Which it seems to currently be true for every modern JS engine, so we're probably fine
for(let idx in this.citationsByIndex) {
+ if (excludeNew && this.newIndices[idx]) {
+ i++;
+ continue;
+ }
citations.push([this.citationsByIndex[idx].citationID, this.citationsByIndex[idx].properties.noteIndex]);
fieldToCitationIdxMapping[i] = idx;
citationToFieldIdxMapping[idx] = i++;
@@ -2266,8 +2292,10 @@ Zotero.Integration.BibliographyField = class extends Zotero.Integration.Field {
};
Zotero.Integration.Citation = class {
- constructor(citationField, noteIndex) {
- let data = citationField.unserialize();
+ constructor(citationField, noteIndex, data) {
+ if (!data) {
+ data = {citationItems: [], properties: {}};
+ }
this.citationID = data.citationID;
this.citationItems = data.citationItems;
this.properties = data.properties;
@@ -2511,9 +2539,9 @@ Zotero.Integration.Citation = class {
};
Zotero.Integration.Bibliography = class {
- constructor(bibliographyField) {
+ constructor(bibliographyField, data) {
this._field = bibliographyField;
- this.data = bibliographyField.unserialize();
+ this.data = data;
this.uncitedItemIDs = new Set();
this.omittedItemIDs = new Set();
diff --git a/test/tests/integrationTest.js b/test/tests/integrationTest.js
@@ -277,11 +277,12 @@ describe("Zotero.Integration", function () {
function setAddEditItems(items) {
if (items.length == undefined) items = [items];
- dialogResults.quickFormat = function(dialogName, io) {
+ dialogResults.quickFormat = async function(dialogName, io) {
io.citation.citationItems = items.map(function(item) {
item = Zotero.Cite.getItem(item.id);
return {id: item.id, uris: item.cslURIs, itemData: item.cslItemData};
});
+ await io.previewFn(io.citation);
io._acceptDeferred.resolve(() => {});
};
}
@@ -306,21 +307,15 @@ describe("Zotero.Integration", function () {
return applications[docID];
});
- displayDialogStub = sinon.stub(Zotero.Integration, 'displayDialog',
- // @TODO: This sinon.stub syntax is deprecated!
- // However when using callsFake tests won't work. This is due to a
- // possible bug that reset() erases callsFake.
- // @NOTE: https://github.com/sinonjs/sinon/issues/1341
- // displayDialogStub.callsFake(function(doc, dialogName, prefs, io) {
- function(dialogName, prefs, io) {
+ displayDialogStub = sinon.stub(Zotero.Integration, 'displayDialog');
+ displayDialogStub.callsFake(async function(dialogName, prefs, io) {
Zotero.debug(`Display dialog: ${dialogName}`, 2);
var ioResult = dialogResults[dialogName.substring(dialogName.lastIndexOf('/')+1, dialogName.length-4)];
if (typeof ioResult == 'function') {
- ioResult(dialogName, io);
+ await ioResult(dialogName, io);
} else {
Object.assign(io, ioResult);
}
- return Zotero.Promise.resolve();
});
addEditCitationSpy = sinon.spy(Zotero.Integration.Interface.prototype, 'addEditCitation');
@@ -371,7 +366,7 @@ describe("Zotero.Integration", function () {
Zotero.Styles.get(style.styleID).remove();
} catch (e) {}
initDoc(docID, {style});
- displayDialogStub.reset();
+ displayDialogStub.resetHistory();
displayAlertStub.reset();
});
@@ -577,6 +572,38 @@ describe("Zotero.Integration", function () {
});
});
+ describe('when there are duplicated citations (copy-paste)', function() {
+ it('should resolve duplicate citationIDs and mark both as new citations', 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);
+ // Add a duplicate
+ doc.fields.push(new DocumentPluginDummy.Field(doc));
+ doc.fields[1].code = doc.fields[0].code;
+ doc.fields[1].text = doc.fields[0].text;
+
+ var originalUpdateDocument = Zotero.Integration.Fields.prototype.updateDocument;
+ var stubUpdateDocument = sinon.stub(Zotero.Integration.Fields.prototype, 'updateDocument');
+ try {
+ var indicesLength;
+ stubUpdateDocument.callsFake(function() {
+ indicesLength = Object.keys(Zotero.Integration.currentSession.newIndices).length;
+ return originalUpdateDocument.apply(this, arguments);
+ });
+
+ setAddEditItems(testItems[1]);
+ await execCommand('addEditCitation', docID);
+ assert.equal(indicesLength, 3);
+ } finally {
+ stubUpdateDocument.restore();
+ }
+ });
+ });
+
describe('when delayCitationUpdates is set', function() {
it('should insert a citation with wave underlining', function* (){
yield insertMultipleCitations.call(this);
@@ -631,7 +658,7 @@ describe("Zotero.Integration", function () {
});
it('should insert bibliography if no bibliography field present', function* () {
- displayDialogStub.reset();
+ displayDialogStub.resetHistory();
yield execCommand('addEditBibliography', docID);
assert.isFalse(displayDialogStub.called);
var biblPresent = false;
@@ -647,7 +674,7 @@ describe("Zotero.Integration", function () {
it('should display the edit bibliography dialog if bibliography present', function* () {
yield execCommand('addEditBibliography', docID);
- displayDialogStub.reset();
+ displayDialogStub.resetHistory();
yield execCommand('addEditBibliography', docID);
assert.isTrue(displayDialogStub.calledOnce);
assert.isTrue(displayDialogStub.lastCall.args[0].includes('editBibliographyDialog'));
