suzanne.soy/zotero
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix citationByIndex[i].sortedItem errors during citation insertion

Browse Source 
Caused by inproper handling of copy-pasted citations in documents
This commit is contained in:
Adomas Venčkauskas 2018-03-26 15:31:26 +03:00
parent e746da4fab
commit 3445519714
2 changed files with 90 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

72
chrome/content/zotero/xpcom/integration.js
View File

@ -565,7 +565,7 @@ Zotero.Integration.Interface.prototype.editBibliography = Zotero.Promise.corouti
throw new Zotero.Exception.Alert("integration.error.mustInsertBibliography", throw new Zotero.Exception.Alert("integration.error.mustInsertBibliography",
[], "integration.error.title"); [], "integration.error.title");
} }
let bibliography = new Zotero.Integration.Bibliography(bibliographyField); let bibliography = new Zotero.Integration.Bibliography(bibliographyField, bibliographyField.unserialize());
var citationsMode = FORCE_CITATIONS_FALSE; var citationsMode = FORCE_CITATIONS_FALSE;
if(this._session.data.prefs.delayCitationUpdates) { if(this._session.data.prefs.delayCitationUpdates) {
// Refreshes citeproc state before proceeding // Refreshes citeproc state before proceeding
@ -604,7 +604,7 @@ Zotero.Integration.Interface.prototype.addEditBibliography = Zotero.Promise.coro
bibliographyField.clearCode(); bibliographyField.clearCode();
} }
let bibliography = new Zotero.Integration.Bibliography(bibliographyField); let bibliography = new Zotero.Integration.Bibliography(bibliographyField, bibliographyField.unserialize());
var citationsMode = FORCE_CITATIONS_FALSE; var citationsMode = FORCE_CITATIONS_FALSE;
if(this._session.data.prefs.delayCitationUpdates) { if(this._session.data.prefs.delayCitationUpdates) {
// Refreshes citeproc state before proceeding // 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]); let field = Zotero.Integration.Field.loadExisting(this._fields[i]);
if (field.type === INTEGRATION_TYPE_ITEM) { if (field.type === INTEGRATION_TYPE_ITEM) {
var noteIndex = field.getNoteIndex(), 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); yield this._session.addCitation(i, noteIndex, citation);
} else if (field.type === INTEGRATION_TYPE_BIBLIOGRAPHY) { } else if (field.type === INTEGRATION_TYPE_BIBLIOGRAPHY) {
@ -905,7 +906,8 @@ Zotero.Integration.Fields.prototype._processFields = Zotero.Promise.coroutine(fu
} }
} }
if (this._bibliographyFields.length) { 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(); yield this._session.bibliography.loadItemData();
} else { } else {
delete this._session.bibliography; 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) { Zotero.Integration.Fields.prototype.addEditCitation = Zotero.Promise.coroutine(function* (field) {
var newField; var newField;
var citation;
if (field) { if (field) {
field = Zotero.Integration.Field.loadExisting(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) { if (field.type != INTEGRATION_TYPE_ITEM) {
throw new Zotero.Exception.Alert("integration.error.notInCitation"); throw new Zotero.Exception.Alert("integration.error.notInCitation");
} }
citation = new Zotero.Integration.Citation(field, field.getNoteIndex(), field.unserialize());
} else { } else {
newField = true; newField = true;
field = new Zotero.Integration.CitationField(yield this.addField(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(); yield citation.prepareForEditing();
// ------------------- // -------------------
@ -1158,19 +1161,31 @@ Zotero.Integration.Fields.prototype.addEditCitation = Zotero.Promise.coroutine(f
}.bind(this)); }.bind(this));
} }
var previewFn = Zotero.Promise.coroutine(function* (citation) { var previewFn = async function (citation) {
let idx = yield fieldIndexPromise; let idx = await fieldIndexPromise;
yield citationsByItemIDPromise; await citationsByItemIDPromise;
var fields = await this.get();
var [citations, fieldToCitationIdxMapping, citationToFieldIdxMapping] = this._session.getCiteprocLists(); var [citations, fieldToCitationIdxMapping, citationToFieldIdxMapping] = this._session.getCiteprocLists(true);
let citationsPre = citations.slice(0, fieldToCitationIdxMapping[idx]); for (var prevIdx = idx-1; prevIdx >= 0; prevIdx--) {
let citationsPost = citations.slice(fieldToCitationIdxMapping[idx]+1); 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 { try {
return this._session.style.previewCitationCluster(citation, citationsPre, citationsPost, "rtf"); var result = this._session.style.previewCitationCluster(citation, citationsPre, citationsPost, "rtf");
} catch(e) { } catch(e) {
throw 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( var io = new Zotero.Integration.CitationEditInterface(
citation, this._session.style.opt.sort_citations, 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 // 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(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) { if(needNewID) {
Zotero.debug("Integration: "+citation.citationID+" ("+index+") needs new citationID"); Zotero.debug("Integration: "+citation.citationID+" ("+index+") needs new citationID");
citation.citationID = Zotero.randomString(); citation.citationID = Zotero.randomString();
@ -1537,17 +1559,21 @@ Zotero.Integration.Session.prototype.addCitation = Zotero.Promise.coroutine(func
this.newIndices[index] = true; this.newIndices[index] = true;
} }
Zotero.debug("Integration: Adding citationID "+citation.citationID); 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 citations = [];
var fieldToCitationIdxMapping = []; var fieldToCitationIdxMapping = {};
var citationToFieldIdxMapping = {}; var citationToFieldIdxMapping = {};
var i = 0; var i = 0;
// This relies on the order of citationsByIndex keys being stable and sorted in ascending order // 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 // Which it seems to currently be true for every modern JS engine, so we're probably fine
for(let idx in this.citationsByIndex) { for(let idx in this.citationsByIndex) {
if (excludeNew && this.newIndices[idx]) {
i++;
continue;
}
citations.push([this.citationsByIndex[idx].citationID, this.citationsByIndex[idx].properties.noteIndex]); citations.push([this.citationsByIndex[idx].citationID, this.citationsByIndex[idx].properties.noteIndex]);
fieldToCitationIdxMapping[i] = idx; fieldToCitationIdxMapping[i] = idx;
citationToFieldIdxMapping[idx] = i++; citationToFieldIdxMapping[idx] = i++;
@ -2266,8 +2292,10 @@ Zotero.Integration.BibliographyField = class extends Zotero.Integration.Field {
}; };
Zotero.Integration.Citation = class { Zotero.Integration.Citation = class {
constructor(citationField, noteIndex) { constructor(citationField, noteIndex, data) {
let data = citationField.unserialize(); if (!data) {
data = {citationItems: [], properties: {}};
}
this.citationID = data.citationID; this.citationID = data.citationID;
this.citationItems = data.citationItems; this.citationItems = data.citationItems;
this.properties = data.properties; this.properties = data.properties;
@ -2511,9 +2539,9 @@ Zotero.Integration.Citation = class {
}; };
Zotero.Integration.Bibliography = class { Zotero.Integration.Bibliography = class {
constructor(bibliographyField) { constructor(bibliographyField, data) {
this._field = bibliographyField; this._field = bibliographyField;
this.data = bibliographyField.unserialize(); this.data = data;
this.uncitedItemIDs = new Set(); this.uncitedItemIDs = new Set();
this.omittedItemIDs = new Set(); this.omittedItemIDs = new Set();

53
test/tests/integrationTest.js
View File

@ -277,11 +277,12 @@ describe("Zotero.Integration", function () {
function setAddEditItems(items) { function setAddEditItems(items) {
if (items.length == undefined) items = [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) { io.citation.citationItems = items.map(function(item) {
item = Zotero.Cite.getItem(item.id); item = Zotero.Cite.getItem(item.id);
return {id: item.id, uris: item.cslURIs, itemData: item.cslItemData}; return {id: item.id, uris: item.cslURIs, itemData: item.cslItemData};
}); });
await io.previewFn(io.citation);
io._acceptDeferred.resolve(() => {}); io._acceptDeferred.resolve(() => {});
}; };
} }
@ -306,21 +307,15 @@ describe("Zotero.Integration", function () {
return applications[docID]; return applications[docID];
}); });
displayDialogStub = sinon.stub(Zotero.Integration, 'displayDialog', displayDialogStub = sinon.stub(Zotero.Integration, 'displayDialog');
// @TODO: This sinon.stub syntax is deprecated! displayDialogStub.callsFake(async function(dialogName, prefs, io) {
// 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) {
Zotero.debug(`Display dialog: ${dialogName}`, 2); Zotero.debug(`Display dialog: ${dialogName}`, 2);
var ioResult = dialogResults[dialogName.substring(dialogName.lastIndexOf('/')+1, dialogName.length-4)]; var ioResult = dialogResults[dialogName.substring(dialogName.lastIndexOf('/')+1, dialogName.length-4)];
if (typeof ioResult == 'function') { if (typeof ioResult == 'function') {
ioResult(dialogName, io); await ioResult(dialogName, io);
} else { } else {
Object.assign(io, ioResult); Object.assign(io, ioResult);
} }
return Zotero.Promise.resolve();
}); });
addEditCitationSpy = sinon.spy(Zotero.Integration.Interface.prototype, 'addEditCitation'); addEditCitationSpy = sinon.spy(Zotero.Integration.Interface.prototype, 'addEditCitation');
@ -371,7 +366,7 @@ describe("Zotero.Integration", function () {
Zotero.Styles.get(style.styleID).remove(); Zotero.Styles.get(style.styleID).remove();
} catch (e) {} } catch (e) {}
initDoc(docID, {style}); initDoc(docID, {style});
displayDialogStub.reset(); displayDialogStub.resetHistory();
displayAlertStub.reset(); 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() { describe('when delayCitationUpdates is set', function() {
it('should insert a citation with wave underlining', function* (){ it('should insert a citation with wave underlining', function* (){
yield insertMultipleCitations.call(this); yield insertMultipleCitations.call(this);
@ -631,7 +658,7 @@ describe("Zotero.Integration", function () {
}); });
it('should insert bibliography if no bibliography field present', function* () { it('should insert bibliography if no bibliography field present', function* () {
displayDialogStub.reset(); displayDialogStub.resetHistory();
yield execCommand('addEditBibliography', docID); yield execCommand('addEditBibliography', docID);
assert.isFalse(displayDialogStub.called); assert.isFalse(displayDialogStub.called);
var biblPresent = false; var biblPresent = false;
@ -647,7 +674,7 @@ describe("Zotero.Integration", function () {
it('should display the edit bibliography dialog if bibliography present', function* () { it('should display the edit bibliography dialog if bibliography present', function* () {
yield execCommand('addEditBibliography', docID); yield execCommand('addEditBibliography', docID);
displayDialogStub.reset(); displayDialogStub.resetHistory();
yield execCommand('addEditBibliography', docID); yield execCommand('addEditBibliography', docID);
assert.isTrue(displayDialogStub.calledOnce); assert.isTrue(displayDialogStub.calledOnce);
assert.isTrue(displayDialogStub.lastCall.args[0].includes('editBibliographyDialog')); assert.isTrue(displayDialogStub.lastCall.args[0].includes('editBibliographyDialog'));