mirrors/zotero
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages 1 Wiki Activity Actions

Merge: Keep external annotations on master, don't erase on other

Browse source 
PDFWorker only re-imports external annotations when the file changes on
disk. Keep annotation items corresponding to external annotations so
that they don't disappear after the merge (and then come back when the
file is edited).

Tweaks behavior introduced in 2aa34a6.

https://forums.zotero.org/discussion/113943/zotero-7-beta-merging-pdf-files-leaves-ghost-external-annotations
This commit is contained in:
Abe Jellinek 2024-04-25 16:08:45 -04:00
parent 1c05dafedd
commit a5393ca0e5
5 changed files with 52 additions and 11 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

10
chrome/content/zotero/xpcom/data/items.js
View file

@ -959,7 +959,6 @@ Zotero.Items = function() {
let annotations = fromItem.getAnnotations(includeTrashed);
for (let annotation of annotations) {
if (annotation.annotationIsExternal) {
await annotation.erase();
continue;
}
annotation.parentItemID = toItem.id;
@ -967,15 +966,6 @@ Zotero.Items = function() {
}
}
if (toItem.isFileAttachment()) {
let annotations = toItem.getAnnotations(includeTrashed);
for (let annotation of annotations) {
if (annotation.annotationIsExternal) {
await annotation.erase();
}
}
}
// TODO: Other things as necessary
};

BIN
test/tests/data/duplicatesMerge_annotated_2.pdf
View file

Binary file not shown.

BIN
test/tests/data/duplicatesMerge_annotated_3.pdf Normal file
View file

Binary file not shown.

2
test/tests/itemTest.js
View file

@ -1626,7 +1626,7 @@ describe("Zotero.Item", function () {
});
it("should recognize a strikeout annotation", async function () {
let attachment = await importFileAttachment('duplicatesMerge_annotated_2.pdf');
let attachment = await importFileAttachment('duplicatesMerge_annotated_3.pdf');
assert.isTrue(await attachment.hasEmbeddedAnnotations());
});

51
test/tests/itemsTest.js
View file

@ -934,6 +934,16 @@ describe("Zotero.Items", function () {
let item2 = item1.clone();
await item2.saveTx();
let attachment2 = await importFileAttachment('duplicatesMerge_annotated_2.pdf', { parentID: item2.id });
// Import external annotations non-destructively
await Zotero.PDFWorker.import(attachment1.id, true);
await Zotero.PDFWorker.import(attachment2.id, true);
assert.lengthOf(attachment1.getAnnotations(), 1);
assert.lengthOf(attachment2.getAnnotations(), 1);
assert.isTrue(attachment1.getAnnotations()[0].annotationIsExternal);
assert.isTrue(attachment2.getAnnotations()[0].annotationIsExternal);
assert.isTrue(await attachment2.hasEmbeddedAnnotations()); // Unsupported attachment remains embedded
await Zotero.Items.merge(item1, [item2]);
@ -943,6 +953,47 @@ describe("Zotero.Items", function () {
assert.isTrue(item2.deleted);
assert.isFalse(attachment2.deleted);
assert.equal(attachment2.parentItemID, item1.id);
assert.lengthOf(attachment1.getAnnotations(), 1);
assert.lengthOf(attachment2.getAnnotations(), 1);
assert.isTrue(attachment1.getAnnotations()[0].annotationIsExternal);
assert.isTrue(attachment2.getAnnotations()[0].annotationIsExternal);
});
it("should merge imported annotations into PDF with remaining unimported annotations", async function () {
let item1 = await createDataObject('item', { setTitle: true });
let attachment1 = await importFileAttachment('duplicatesMerge_annotated_1.pdf', { parentID: item1.id });
let item2 = item1.clone();
await item2.saveTx();
let attachment2 = await importFileAttachment('duplicatesMerge_annotated_2.pdf', { parentID: item2.id });
// Import external annotations non-destructively
await Zotero.PDFWorker.import(attachment1.id, true);
await Zotero.PDFWorker.import(attachment2.id, true);
assert.isTrue(attachment1.getAnnotations()[0].annotationIsExternal);
assert.isTrue(attachment2.getAnnotations()[0].annotationIsExternal);
// Import external annotations *destructively*
await Zotero.PDFWorker.import(attachment1.id, true, '', true);
await Zotero.PDFWorker.import(attachment2.id, true, '', true);
assert.lengthOf(attachment1.getAnnotations(), 1);
assert.lengthOf(attachment2.getAnnotations(), 1);
assert.isFalse(attachment1.getAnnotations()[0].annotationIsExternal);
assert.isFalse(attachment2.getAnnotations()[0].annotationIsExternal);
assert.isTrue(await attachment2.hasEmbeddedAnnotations()); // Unsupported annotation remains embedded
await Zotero.Items.merge(item1, [item2]);
assert.isTrue(attachment1.deleted);
assert.isFalse(attachment2.deleted);
assert.lengthOf(attachment1.getAnnotations(), 0);
assert.lengthOf(attachment2.getAnnotations(), 2);
assert.isFalse(attachment2.getAnnotations()[0].annotationIsExternal);
assert.isFalse(attachment2.getAnnotations()[1].annotationIsExternal);
});
it("should merge a non-master PDF without embedded annotations into a master PDF with embedded annotations", async function () {