Set automatic title on first child attachment of each type (#4237)

chrome/content/zotero/itemTree.jsx

@@ -2496,6 +2496,8 @@ var ItemTree = class ItemTree extends LibraryTree {
 					}
 
 					if (item) {
+						item.setFirstAttachmentTitle();
+						await item.saveTx();
 						addedItems.push(item);
 					}
 				}

chrome/content/zotero/xpcom/data/item.js

@@ -2488,15 +2488,20 @@ Zotero.Item.prototype.numNonHTMLFileAttachments = function () {
 };
 
 
-Zotero.Item.prototype.numPDFAttachments = function () {
+Zotero.Item.prototype.numFileAttachmentsWithContentType = function (contentType) {
 	this._requireData('childItems');
 	return this.getAttachments()
 		.map(itemID => Zotero.Items.get(itemID))
-		.filter(item => item.isFileAttachment() && item.attachmentContentType == 'application/pdf')
+		.filter(item => item.isFileAttachment() && item.attachmentContentType == contentType)
 		.length;
 };
 
 
+Zotero.Item.prototype.numPDFAttachments = function () {
+	return this.numFileAttachmentsWithContentType('application/pdf');
+};
+
+
 Zotero.Item.prototype.getFile = function () {
 	Zotero.debug("Zotero.Item.prototype.getFile() is deprecated -- use getFilePath[Async]()", 2);
 	
@@ -3858,6 +3863,40 @@ Zotero.Item.prototype.clearBestAttachmentState = function () {
 }
 
 
+Zotero.Item.prototype._getDefaultTitleForAttachmentContentType = function () {
+	switch (this.attachmentContentType) {
+		case 'application/pdf':
+			return Zotero.getString('fileTypes.pdf');
+		case 'application/epub+zip':
+			return Zotero.getString('fileTypes.ebook');
+		case 'text/html':
+			return Zotero.getString('fileTypes.webpage');
+		default:
+			return null;
+	}
+};
+
+
+Zotero.Item.prototype.setFirstAttachmentTitle = function () {
+	if (!this.isAttachment()) {
+		throw new Error("setFirstAttachmentTitle() can only be called on attachment items");
+	}
+	if (!this.isFileAttachment() || !this.parentItemID) {
+		return;
+	}
+	let isFirstOfType = this.parentItem.numFileAttachmentsWithContentType(this.attachmentContentType) <= 1;
+	if (!isFirstOfType) {
+		return;
+	}
+	let defaultTitle = this._getDefaultTitleForAttachmentContentType();
+	if (defaultTitle === null) {
+		// Keep existing title
+		return;
+	}
+	this.setField('title', defaultTitle);
+};
+
+
 ////////////////////////////////////////////////////////
 //
 //

chrome/content/zotero/xpcom/recognizeDocument.js

@@ -200,11 +200,9 @@ Zotero.RecognizeDocument = new function () {
 		try {
 			let currentFilename = attachment.attachmentFilename;
 			if (currentFilename != originalFilename) {
-				let renamed = await attachment.renameAttachmentFile(originalFilename);
-				if (renamed) {
-					attachment.setField('title', originalTitle);
-				}
+				await attachment.renameAttachmentFile(originalFilename);
 			}
+			attachment.setField('title', originalTitle);
 		}
 		catch (e) {
 			Zotero.logError(e);
@@ -302,11 +300,12 @@ Zotero.RecognizeDocument = new function () {
 			if (result !== true) {
 				throw new Error("Error renaming " + path);
 			}
-			// Rename attachment title
-			attachment.setField('title', newName);
-			await attachment.saveTx();
 		}
 		
+		// Rename attachment title
+		attachment.setFirstAttachmentTitle();
+		await attachment.saveTx();
+
 		try {
 			let win = Zotero.getMainWindow();
 			if (selectParent && win && win.Zotero_Tabs.selectedID == 'zotero-pane') {

chrome/content/zotero/zoteroPane.js

@@ -4297,18 +4297,23 @@ var ZoteroPane = new function()
 		});
 	});
 	
-	
-	this.addAttachmentFromDialog = async function (link, parentItemID) {
+	/**
+	 * @param {Boolean} [link]
+	 * @param {Number} [parentItemID]
+	 * @param {nsIFile[]} [files] Used instead of showing a file picker - for tests
+	 * @returns {Promise<Zotero.Item[] | null>}
+	 */
+	this.addAttachmentFromDialog = async function (link, parentItemID, files = null) {
 		if (!this.canEdit()) {
 			this.displayCannotEditLibraryMessage();
-			return;
+			return null;
 		}
 		
 		var collectionTreeRow = this.getCollectionTreeRow();
 		if (link) {
 			if (collectionTreeRow.isWithinGroup()) {
 				Zotero.alert(null, "", "Linked files cannot be added to group libraries.");
-				return;
+				return null;
 			}
 			else if (collectionTreeRow.isPublications()) {
 				Zotero.alert(
@@ -4316,7 +4321,7 @@ var ZoteroPane = new function()
 					Zotero.getString('general.error'),
 					Zotero.getString('publications.error.linkedFilesCannotBeAdded')
 				);
-				return;
+				return null;
 			}
 		}
 		
@@ -4328,15 +4333,17 @@ var ZoteroPane = new function()
 		
 		var libraryID = collectionTreeRow.ref.libraryID;
 		
-		var fp = new FilePicker();
-		fp.init(window, Zotero.getString('pane.item.attachments.select'), fp.modeOpenMultiple);
-		fp.appendFilters(fp.filterAll);
-		
-		if (await fp.show() != fp.returnOK) {
-			return;
+		if (!files) {
+			var fp = new FilePicker();
+			fp.init(window, Zotero.getString('pane.item.attachments.select'), fp.modeOpenMultiple);
+			fp.appendFilters(fp.filterAll);
+
+			if (await fp.show() != fp.returnOK) {
+				return null;
+			}
+
+			files = fp.files;
 		}
-		
-		var files = fp.files;
 		var addedItems = [];
 		var collection;
 		var fileBaseName;
@@ -4403,6 +4410,13 @@ var ZoteroPane = new function()
 				});
 			}
 			
+			try {
+				item.setFirstAttachmentTitle();
+				await item.saveTx();
+			}
+			catch (e) {
+				Zotero.logError(e);
+			}
 			addedItems.push(item);
 		}
 		
@@ -4410,6 +4424,8 @@ var ZoteroPane = new function()
 		if (!parentItemID) {
 			Zotero.RecognizeDocument.autoRecognizeItems(addedItems);
 		}
+		
+		return addedItems;
 	};
 	
 	
@@ -5324,6 +5340,13 @@ var ZoteroPane = new function()
 				this.createEmptyParent(item);
 			}
 		}
+
+		await Zotero.DB.executeTransaction(async () => {
+			for (let item of items) {
+				item.setFirstAttachmentTitle();
+				await item.save();
+			}
+		});
 	};
 	
 	

chrome/locale/en-US/zotero/zotero.properties

@@ -645,7 +645,7 @@ creatorTypes.reviewedAuthor		= Reviewed Author
 creatorTypes.cosponsor			= Cosponsor
 creatorTypes.bookAuthor			= Book Author
 
-fileTypes.webpage				= Web Page
+fileTypes.webpage				= Webpage
 fileTypes.image					= Image
 fileTypes.pdf					= PDF
 fileTypes.audio					= Audio

test/tests/itemTreeTest.js

@@ -1331,9 +1331,7 @@ describe("Zotero.ItemTree", function() {
 			var itemIDs = await promise;
 			var item = Zotero.Items.get(itemIDs[0]);
 			assert.equal(item.parentItemID, parentItem.id);
-			var title = item.getField('title');
 			var path = await item.getFilePathAsync();
-			assert.equal(title, parentTitle + '.pdf');
 			assert.equal(OS.Path.basename(path), parentTitle + '.pdf');
 		});
 		
@@ -1378,9 +1376,7 @@ describe("Zotero.ItemTree", function() {
 			var itemIDs = await promise;
 			var item = Zotero.Items.get(itemIDs[0]);
 			assert.equal(item.parentItemID, parentItem.id);
-			var title = item.getField('title');
 			var path = await item.getFilePathAsync();
-			assert.equal(title, parentTitle + '.pdf');
 			assert.equal(OS.Path.basename(path), parentTitle + '.pdf');
 		});
 		
@@ -1425,9 +1421,7 @@ describe("Zotero.ItemTree", function() {
 			var itemIDs = await promise;
 			var item = Zotero.Items.get(itemIDs[0]);
 			assert.equal(item.parentItemID, parentItem.id);
-			var title = item.getField('title');
 			var path = await item.getFilePathAsync();
-			assert.equal(title, 'empty.pdf');
 			assert.equal(OS.Path.basename(path), 'empty.pdf');
 		});
 		
@@ -1464,10 +1458,8 @@ describe("Zotero.ItemTree", function() {
 			var itemIDs = await promise;
 			var item = Zotero.Items.get(itemIDs[0]);
 			assert.equal(item.parentItemID, parentItem.id);
-			var title = item.getField('title');
 			var path = await item.getFilePathAsync();
 			// Should match original filename, not parent title
-			assert.equal(title, originalFileName);
 			assert.equal(OS.Path.basename(path), originalFileName);
 		});
 		
@@ -1506,9 +1498,7 @@ describe("Zotero.ItemTree", function() {
 			var itemIDs = await promise;
 			var item = Zotero.Items.get(itemIDs[0]);
 			assert.equal(item.parentItemID, parentItem.id);
-			var title = item.getField('title');
 			var path = await item.getFilePathAsync();
-			assert.equal(title, originalFileName);
 			assert.equal(OS.Path.basename(path), originalFileName);
 		});
 		
@@ -1543,11 +1533,56 @@ describe("Zotero.ItemTree", function() {
 			var itemIDs = await promise;
 			var item = Zotero.Items.get(itemIDs[0]);
 			assert.equal(item.parentItemID, parentItem.id);
-			var title = item.getField('title');
 			var path = await item.getFilePathAsync();
-			assert.equal(title, originalFileName);
 			assert.equal(OS.Path.basename(path), originalFileName);
 		});
+
+		it("should set an automatic title on the first file attachment of each supported type", async function () {
+			let view = zp.itemsView;
+			let parentItem = await createDataObject('item');
+			let parentRow = view.getRowIndexByID(parentItem.id);
+
+			// Add a link attachment, which won't affect renaming
+			await Zotero.Attachments.linkFromURL({
+				url: 'https://example.com/',
+				parentItemID: parentItem.id,
+			});
+
+			let file = getTestDataDirectory();
+			file.append('test.pdf');
+
+			let dataTransfer = {
+				dropEffect: 'copy',
+				effectAllowed: 'copy',
+				types: {
+					contains: function (type) {
+						return type == 'application/x-moz-file';
+					}
+				},
+				mozItemCount: 1,
+				mozGetDataAt: function (type, i) {
+					if (type == 'application/x-moz-file' && i == 0) {
+						return file;
+					}
+				}
+			};
+
+			let promise = waitForItemEvent('add');
+			drop(parentRow, 0, dataTransfer);
+
+			// Add a PDF attachment, which will be renamed
+			let pdfAttachment1 = Zotero.Items.get((await promise)[0]);
+			assert.equal(pdfAttachment1.parentItemID, parentItem.id);
+			assert.equal(pdfAttachment1.getField('title'), Zotero.getString('fileTypes.pdf'));
+
+			promise = waitForItemEvent('add');
+			drop(parentRow, 0, dataTransfer);
+
+			// Add a second, which won't
+			let pdfAttachment2 = Zotero.Items.get((await promise)[0]);
+			assert.equal(pdfAttachment2.parentItemID, parentItem.id);
+			assert.equal(pdfAttachment2.getField('title'), 'test.pdf');
+		});
 	});
 	
 	

test/tests/recognizeDocumentTest.js

@@ -66,6 +66,12 @@ describe("Document Recognition", function() {
 				attachment.attachmentFilename,
 				Zotero.Attachments.getFileBaseNameFromItem(item) + '.pdf'
 			);
+			
+			// The title should have changed
+			assert.equal(
+				attachment.getField('title'),
+				Zotero.getString('fileTypes.pdf')
+			);
 		});
 		
 		it("should recognize a PDF by arXiv ID", async function () {
@@ -93,6 +99,12 @@ describe("Document Recognition", function() {
 			while (progressWindow.document.getElementById("label").value != completeStr) {
 				await Zotero.Promise.delay(20);
 			}
+
+			// The title should have changed
+			assert.equal(
+				attachment.getField('title'),
+				Zotero.getString('fileTypes.pdf')
+			);
 		});
 		
 		it("should put new item in same collection", async function () {
@@ -260,6 +272,12 @@ describe("Document Recognition", function() {
 				attachment.attachmentFilename,
 				Zotero.Attachments.getFileBaseNameFromItem(item) + '.pdf'
 			);
+
+			// The title should have changed
+			assert.equal(
+				attachment.getField('title'),
+				Zotero.getString('fileTypes.pdf')
+			);
 		});
 		
 		it("shouldn't rename a linked file attachment using parent metadata if pref disabled", async function () {
@@ -295,6 +313,12 @@ describe("Document Recognition", function() {
 			
 			// The file should not have been renamed
 			assert.equal(attachment.attachmentFilename, 'test.pdf');
+
+			// The title should have changed
+			assert.equal(
+				attachment.getField('title'),
+				Zotero.getString('fileTypes.pdf')
+			);
 		});
 	});
 
@@ -343,6 +367,12 @@ describe("Document Recognition", function() {
 				Zotero.Attachments.getFileBaseNameFromItem(item) + '.epub'
 			);
 
+			// The title should have changed
+			assert.equal(
+				attachment.getField('title'),
+				Zotero.getString('fileTypes.ebook')
+			);
+
 			translateStub.restore();
 		});
 
@@ -374,6 +404,12 @@ describe("Document Recognition", function() {
 				attachment.attachmentFilename,
 				Zotero.Attachments.getFileBaseNameFromItem(item) + '.epub'
 			);
+
+			// The title should have changed
+			assert.equal(
+				attachment.getField('title'),
+				Zotero.getString('fileTypes.ebook')
+			);
 		});
 
 		it("should use metadata from EPUB when search returns item with different ISBN", async function () {

test/tests/zoteroPaneTest.js

@@ -1649,4 +1649,35 @@ describe("ZoteroPane", function() {
 			assert.equal(doc.activeElement.id, "zotero-tb-add");
 		});
 	});
+	
+	describe("#addAttachmentFromDialog()", function () {
+		it("should set an automatic title on the first file attachment of each supported type", async function () {
+			let parentItem = await createDataObject('item');
+			
+			// Add a link attachment, which won't affect renaming
+			await Zotero.Attachments.linkFromURL({
+				url: 'https://example.com/',
+				parentItemID: parentItem.id,
+			});
+			
+			// Add a PDF attachment, which will be renamed
+			let file = getTestDataDirectory();
+			file.append('test.pdf');
+			let [pdfAttachment1] = await zp.addAttachmentFromDialog(false, parentItem.id, [file.path]);
+			assert.equal(parentItem.getAttachments().length, 2);
+			assert.equal(pdfAttachment1.getField('title'), Zotero.getString('fileTypes.pdf'));
+			
+			// Add a second, which won't
+			let [pdfAttachment2] = await zp.addAttachmentFromDialog(false, parentItem.id, [file.path]);
+			assert.equal(parentItem.getAttachments().length, 3);
+			assert.equal(pdfAttachment2.getField('title'), 'test.pdf');
+			
+			// Add an EPUB attachment, which will be renamed
+			file = getTestDataDirectory();
+			file.append('stub.epub');
+			let [epubAttachment] = await zp.addAttachmentFromDialog(false, parentItem.id, [file.path]);
+			assert.equal(parentItem.getAttachments().length, 4);
+			assert.equal(epubAttachment.getField('title'), Zotero.getString('fileTypes.ebook'));
+		});
+	});
 })