diff --git a/chrome/content/zotero/itemTree.jsx b/chrome/content/zotero/itemTree.jsx index 8eb0a46d65..4223f30161 100644 --- a/chrome/content/zotero/itemTree.jsx +++ b/chrome/content/zotero/itemTree.jsx @@ -2575,6 +2575,10 @@ var ItemTree = class ItemTree extends LibraryTree { } } + // If we have more than one file, we only want to call setAutoAttachmentTitle() + // at the end, once the attachments know whether they have siblings + let delaySetAutoAttachmentTitle = data.length > 1; + for (var i=0; i item.id)); diff --git a/chrome/content/zotero/zoteroPane.js b/chrome/content/zotero/zoteroPane.js index 4850b26c4a..b1c2f8ad62 100644 --- a/chrome/content/zotero/zoteroPane.js +++ b/chrome/content/zotero/zoteroPane.js @@ -4725,72 +4725,98 @@ var ZoteroPane = new function () { files = fp.files; } var addedItems = []; + var notifierQueue = new Zotero.Notifier.Queue(); var collection; var fileBaseName; - if (parentItemID) { - // If only one item is being added, automatic renaming is enabled, and the parent item - // doesn't have any other non-HTML file attachments, rename the file. - // This should be kept in sync with itemTreeView::drop(). - if (files.length == 1 && Zotero.Attachments.shouldAutoRenameFile(link, libraryID)) { - let parentItem = Zotero.Items.get(parentItemID); - if (!parentItem.numNonHTMLFileAttachments()) { - fileBaseName = await Zotero.Attachments.getRenamedFileBaseNameIfAllowedType( - parentItem, files[0] - ); - } - } - } - // If not adding to an item, add to the current collection - else { - collection = this.getSelectedCollection(true); - } - for (let file of files) { - let item; - - if (link) { - // Rename linked file, with unique suffix if necessary - try { - if (fileBaseName) { - let ext = Zotero.File.getExtension(file); - let newName = await Zotero.File.rename( - file, - fileBaseName + (ext ? '.' + ext : ''), - { - unique: true - } + try { + if (parentItemID) { + // If only one item is being added, automatic renaming is enabled, and the parent item + // doesn't have any other non-HTML file attachments, rename the file. + // This should be kept in sync with itemTreeView::drop(). + if (files.length == 1 && Zotero.Attachments.shouldAutoRenameFile(link, libraryID)) { + let parentItem = Zotero.Items.get(parentItemID); + if (!parentItem.numNonHTMLFileAttachments()) { + fileBaseName = await Zotero.Attachments.getRenamedFileBaseNameIfAllowedType( + parentItem, files[0] ); - // Update path in case the name was changed to be unique - file = PathUtils.join(PathUtils.parent(file), newName); } } - catch (e) { - Zotero.logError(e); - } - - item = await Zotero.Attachments.linkFromFile({ - file, - parentItemID, - collections: collection ? [collection] : undefined - }); } + // If not adding to an item, add to the current collection else { - if (file.endsWith(".lnk")) { - let win = Services.wm.getMostRecentWindow("navigator:browser"); - win.ZoteroPane.displayCannotAddShortcutMessage(file); - continue; - } - - item = await Zotero.Attachments.importFromFile({ - file, - libraryID, - fileBaseName, - parentItemID, - collections: collection ? [collection] : undefined - }); + collection = this.getSelectedCollection(true); } - - addedItems.push(item); + + // If we have more than one file, we only want to call setAutoAttachmentTitle() + // at the end, once the attachments know whether they have siblings + let delaySetAutoAttachmentTitle = files.length > 1; + + for (let file of files) { + let item; + + if (link) { + // Rename linked file, with unique suffix if necessary + try { + if (fileBaseName) { + let ext = Zotero.File.getExtension(file); + let newName = await Zotero.File.rename( + file, + fileBaseName + (ext ? '.' + ext : ''), + { + unique: true + } + ); + // Update path in case the name was changed to be unique + file = PathUtils.join(PathUtils.parent(file), newName); + } + } + catch (e) { + Zotero.logError(e); + } + + item = await Zotero.Attachments.linkFromFile({ + file, + title: delaySetAutoAttachmentTitle ? '' : undefined, + parentItemID, + collections: collection ? [collection] : undefined, + saveOptions: { + notifierQueue + }, + }); + } + else { + if (file.endsWith(".lnk")) { + let win = Services.wm.getMostRecentWindow("navigator:browser"); + win.ZoteroPane.displayCannotAddShortcutMessage(file); + continue; + } + + item = await Zotero.Attachments.importFromFile({ + file, + libraryID, + fileBaseName, + title: delaySetAutoAttachmentTitle ? '' : undefined, + parentItemID, + collections: collection ? [collection] : undefined, + saveOptions: { + notifierQueue + }, + }); + } + + addedItems.push(item); + } + + if (delaySetAutoAttachmentTitle) { + for (let item of addedItems) { + item.setAutoAttachmentTitle(); + await item.saveTx(); + } + } + } + finally { + await Zotero.Notifier.commit(notifierQueue); } // Select added child attachments diff --git a/test/tests/itemTreeTest.js b/test/tests/itemTreeTest.js index 0fee9ae82c..3375211e3c 100644 --- a/test/tests/itemTreeTest.js +++ b/test/tests/itemTreeTest.js @@ -1402,8 +1402,7 @@ describe("Zotero.ItemTree", function () { mozItemCount: 2, }) - var item1 = Zotero.Items.get((await promise)[0]); - var item2 = Zotero.Items.get((await waitForItemEvent('add'))[0]); + var [item1, item2] = Zotero.Items.get(await promise); var progressWindow = await recognizerPromise; progressWindow.close(); @@ -1607,6 +1606,7 @@ describe("Zotero.ItemTree", function () { var parentRow = view.getRowIndexByID(parentItem.id); var originalFileName = 'empty.pdf'; + var originalFilenameWithoutExtension = 'empty'; var file = getTestDataDirectory(); file.append(originalFileName); @@ -1629,6 +1629,10 @@ describe("Zotero.ItemTree", function () { assert.equal(item.parentItemID, parentItem.id); var path = await item.getFilePathAsync(); assert.equal(OS.Path.basename(path), originalFileName); + + for (let item of Zotero.Items.get(itemIDs)) { + assert.equal(item.getField('title'), originalFilenameWithoutExtension); + } }); it("should set an automatic title on the first file attachment of each supported type", async function () { diff --git a/test/tests/zoteroPaneTest.js b/test/tests/zoteroPaneTest.js index 480da6704a..addf50de46 100644 --- a/test/tests/zoteroPaneTest.js +++ b/test/tests/zoteroPaneTest.js @@ -1567,7 +1567,26 @@ describe("ZoteroPane", function () { assert.equal(parentItem.getAttachments().length, 4); assert.equal(epubAttachment.getField('title'), Zotero.getString('file-type-ebook')); }); + + it("shouldn't set type-based titles when multiple attachments of the same type are added at once", async function () { + let parentItem = await createDataObject('item'); + // Add two PDF attachments at once, which will get titles based on their filenames + let file = getTestDataDirectory(); + file.append('test.pdf'); + let [pdfAttachment1, pdfAttachment2] = await zp.addAttachmentFromDialog(false, parentItem.id, [file.path, file.path]); + assert.equal(parentItem.getAttachments().length, 2); + assert.equal(pdfAttachment1.getField('title'), 'test'); + assert.equal(pdfAttachment2.getField('title'), 'test'); + + // Add an EPUB attachment, which will get a default title + file = getTestDataDirectory(); + file.append('stub.epub'); + let [epubAttachment] = await zp.addAttachmentFromDialog(false, parentItem.id, [file.path]); + assert.equal(parentItem.getAttachments().length, 3); + assert.equal(epubAttachment.getField('title'), Zotero.getString('file-type-ebook')); + }); + it("should select added file attachment", async function () { let parentItem = await createDataObject('item');