Improve counting of file attachments for Show File and Export PDFs

If a parent item and its primary attachment are selected, it should
still say "Show File" rather than "Show Files". This adds the
questionably named `Zotero.Items.numDistinctFileAttachmentsForLabel()`
to try to figure out if we're operating on 0, 1, or >1 items.

Follow-up to #4124

chrome/content/zotero/locateMenu.js

@@ -602,15 +602,15 @@ var Zotero_LocateMenu = new function() {
 		this.className = "zotero-menuitem-show-file";
 		this.hideInToolbar = true;
 		this.l10nId = "menu-show-file";
-		this.l10nArgs = { count: 0 };
+		this.l10nArgs = { count: 1 };
 		
 		this.canHandleItem = function (item) {
 			return ZoteroPane.canShowItemInFilesystem(item);
 		};
 		
 		this.updateMenuItem = function (items) {
-			let count = items.filter(item => ZoteroPane.canShowItemInFilesystem(item))
-				.length;
+			// We only care about showing 1 or many
+			let count = Zotero.Items.numDistinctFileAttachmentsForLabel(items) > 1 ? 2 : 1;
 			this.l10nArgs = { count };
 		};
 		

chrome/content/zotero/standalone/standalone.js

@@ -176,28 +176,25 @@ const ZoteroStandalone = new function() {
 			}
 		}
 		
+		let selectedItems = ZoteroPane.getSelectedItems();
+		
 		let showFileMenuitem = document.getElementById('menu_showFile');
-		let numFiles = ZoteroPane.getSelectedItems()
-			.filter(item => ZoteroPane.canShowItemInFilesystem(item))
-			.length;
+		let numFiles = Zotero.Items.numDistinctFileAttachmentsForLabel(selectedItems);
 		showFileMenuitem.disabled = !numFiles;
 		document.l10n.setArgs(showFileMenuitem, {
-			count: numFiles
+			// We only care about showing 1 or many
+			count: numFiles > 1 ? 2 : 1
 		});
 
 		// TEMP: Quick implementation
 		try {
 			let menuitem = document.getElementById('menu_export_files');
+			// Library tab
 			if (!reader) {
-				let numFiles = ZoteroPane.getSelectedItems().reduce((num, item) => {
-					if (item.isPDFAttachment()) {
-						return num + 1;
-					}
-					if (item.isRegularItem()) {
-						return num + item.numPDFAttachments();
-					}
-					return num;
-				}, 0);
+				let numFiles = Zotero.Items.numDistinctFileAttachmentsForLabel(
+					selectedItems,
+					item => item.isPDFAttachment()
+				);
 				if (numFiles) {
 					menuitem.hidden = false;
 					menuitem.label = Zotero.getString(
@@ -208,6 +205,7 @@ const ZoteroStandalone = new function() {
 					menuitem.hidden = true;
 				}
 			}
+			// Reader tab
 			else {
 				menuitem.hidden = true;
 			}

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

@@ -1829,6 +1829,61 @@ Zotero.Items = function() {
 	};
 	
 	
+	/**
+	 * Returns a rough count (0, 1, or 2) of the number of file attachments implied by the passed
+	 * array of items (which can include both parent and child items) in order to display a menu
+	 * label (e.g., "Show File" or "Show Files")
+	 *
+	 * @param {[Zotero.Item]}
+	 * @param {Function} filter - An additional filter function to run on file attachment items to
+	 *     determine if they qualify
+	 * @return {Integer} - 0, 1, or 2, where 2 means >1
+	 */
+	this.numDistinctFileAttachmentsForLabel = function (items, filter = item => item.isFileAttachment()) {
+		const MAX_ITEMS = 2;
+		var num = 0;
+		var foundKey;
+		for (let item of items) {
+			if (item.isRegularItem()) {
+				// Ideally we want to avoid counting a parent item and its primary attachment as
+				// multiple files, but getBestAttachment() is asynchronous and we need to do this
+				// synchronously, so try to use the cached best-attachment state
+				let { key } = item.getBestAttachmentStateCached();
+				if (key) {
+					if (foundKey) {
+						if (key == foundKey) {
+							continue;
+						}
+						return MAX_ITEMS;
+					}
+					foundKey = key;
+					num++;
+				}
+				// If we don't have a cached primary attachment, the best we can do is count the
+				// parent item if it has any file attachments. Since we're not recording the actual
+				// attachment being counted, this might result in returning MAX_ITEMS even if only
+				// the parent item and primary attachment are selected.
+				else if (item.getAttachments().map(itemID => Zotero.Items.get(itemID)).some(filter)) {
+					foundKey = item.key;
+					num++;
+				}
+			}
+			else if (filter(item)) {
+				if (foundKey) {
+					if (item.key == foundKey) {
+						continue;
+					}
+					// More than 1 attachment found
+					return MAX_ITEMS;
+				}
+				foundKey = item.key;
+				num++;
+			}
+		}
+		return num;
+	};
+	
+	
 	/*
 	 * Generate SQL to retrieve firstCreator field
 	 *

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

@@ -14,7 +14,6 @@ menu-show-file =
     .label = { PLATFORM() ->
         [macos] Show in Finder
         *[other] { $count ->
-            [0] Show File
             [one] Show File
             *[other] Show Files
         }

test/tests/itemsTest.js

@@ -1324,6 +1324,43 @@ describe("Zotero.Items", function () {
 		});
 	});
 	
+	
+	describe("#numDistinctFileAttachmentsForLabel()", function () {
+		it("should count parent item and best attachment as one item when best-attachment state is cached", async function () {
+			var item1 = await createDataObject('item');
+			var attachment1 = await importFileAttachment('test.png', { parentItemID: item1.id });
+			var attachment2 = await importFileAttachment('test.png', { parentItemID: item1.id });
+			
+			function getNum() {
+				return Zotero.Items.numDistinctFileAttachmentsForLabel(zp.getSelectedItems());
+			}
+			
+			zp.itemsView.selection.clearSelection();
+			assert.equal(getNum(), 0);
+			
+			// Uncached best-attachment state
+			await zp.selectItems([item1.id]);
+			assert.equal(getNum(), 1);
+			await zp.selectItems([item1.id, attachment1.id]);
+			// Should count parent item and best attachment as two item when uncached
+			assert.equal(getNum(), 2);
+			await zp.selectItems([item1.id, attachment1.id, attachment2.id]);
+			// Max is 2
+			assert.equal(getNum(), 2);
+			
+			await item1.getBestAttachment();
+			
+			// Cached best-attachment state
+			await zp.selectItems([item1.id]);
+			assert.equal(getNum(), 1);
+			await zp.selectItems([item1.id, attachment1.id]);
+			// Should count parent item and best attachment as one item when cached
+			assert.equal(getNum(), 1);
+			await zp.selectItems([item1.id, attachment1.id, attachment2.id]);
+			assert.equal(getNum(), 2);
+		});
+	});
+	
 	describe("#_loadChildItems()", function () {
 		it("should mark child items as loaded for an attachment", async function () {
 			var attachment = await importPDFAttachment();