diff --git a/chrome/content/zotero/xpcom/data/items.js b/chrome/content/zotero/xpcom/data/items.js index 2af24fff3b..d214a4717e 100644 --- a/chrome/content/zotero/xpcom/data/items.js +++ b/chrome/content/zotero/xpcom/data/items.js @@ -1060,7 +1060,7 @@ Zotero.Items = function() { } if (!masterAttachmentID || mergedMasterAttachments.has(masterAttachmentID)) { - Zotero.debug(`No unmerged match for attachment ${otherAttachment.id} in master item - moving`); + Zotero.debug(`No unmerged match for attachment ${otherAttachment.key} in master item - moving`); otherAttachment.parentItemID = item.id; await otherAttachment.save(); continue; @@ -1070,14 +1070,22 @@ Zotero.Items = function() { let masterAttachment = await this.getAsync(masterAttachmentID); if (masterAttachment.attachmentContentType !== otherAttachment.attachmentContentType) { - Zotero.debug(`Master attachment ${masterAttachmentID} matches ${otherAttachment.id}, ` + Zotero.debug(`Master attachment ${masterAttachment.key} matches ${otherAttachment.key}, ` + 'but content types differ - moving'); otherAttachment.parentItemID = item.id; await otherAttachment.save(); continue; } - Zotero.debug(`Master attachment ${masterAttachmentID} matches ${otherAttachment.id} - merging`); + if (masterAttachment.attachmentLinkMode !== otherAttachment.attachmentLinkMode) { + Zotero.debug(`Master attachment ${masterAttachment.key} matches ${otherAttachment.key}, ` + + 'but link modes differ - moving'); + otherAttachment.parentItemID = item.id; + await otherAttachment.save(); + continue; + } + + Zotero.debug(`Master attachment ${masterAttachment.key} matches ${otherAttachment.key} - merging`); await this.moveChildItems(otherAttachment, masterAttachment, true); await this._moveEmbeddedNote(otherAttachment, masterAttachment); await this._moveRelations(otherAttachment, masterAttachment); @@ -1125,7 +1133,7 @@ Zotero.Items = function() { ); if (!masterAttachment) { - Zotero.debug(`No match for web attachment ${otherAttachment.id} in master item - moving`); + Zotero.debug(`No match for web attachment ${otherAttachment.key} in master item - moving`); otherAttachment.parentItemID = item.id; await otherAttachment.save(); continue; diff --git a/test/tests/itemsTest.js b/test/tests/itemsTest.js index 4ea5cf2f60..f4f7655fd3 100644 --- a/test/tests/itemsTest.js +++ b/test/tests/itemsTest.js @@ -819,6 +819,51 @@ describe("Zotero.Items", function () { assert.lengthOf(rels, 2); assert.sameMembers(rels, [attachment2URI, attachment3URI]); }); + + it("should not merge attachments with different content types", async function () { + let item1 = await createDataObject('item', { setTitle: true }); + let attachment1 = await importPDFAttachment(item1); + + let item2 = item1.clone(); + await item2.saveTx(); + let attachment2 = await importPDFAttachment(item2); + attachment2.attachmentContentType = 'text/plain'; + await attachment2.saveTx(); + + await Zotero.Items.merge(item1, [item2]); + + assert.isFalse(item1.deleted); + assert.isFalse(attachment1.deleted); + assert.equal(item1.numAttachments(true), 2); + assert.isTrue(item2.deleted); + assert.isFalse(attachment2.deleted); + assert.equal(attachment2.parentItemID, item1.id); + }); + + it("should not merge attachments with different link modes", async function () { + let file = getTestDataDirectory(); + file.append('test.pdf'); + + let item1 = await createDataObject('item', { setTitle: true }); + let attachment1 = await Zotero.Attachments.linkFromFile({ + file, + parentItemID: item1.id + }); + + let item2 = item1.clone(); + await item2.saveTx(); + let attachment2 = await importPDFAttachment(item2); + + await Zotero.Items.merge(item1, [item2]); + + assert.equal(await attachment1.attachmentHash, await attachment2.attachmentHash); + assert.isFalse(item1.deleted); + assert.isFalse(attachment1.deleted); + assert.equal(item1.numAttachments(true), 2); + assert.isTrue(item2.deleted); + assert.isFalse(attachment2.deleted); + assert.equal(attachment2.parentItemID, item1.id); + }); })