From 557a69c4c91f7466488f7e0e56e98fe548978cee Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Tue, 16 Jun 2026 13:47:21 -0400 Subject: [PATCH] Handle multiple selection in collection-tree menus and delete prompts Hide the delete actions for a mixed-type selection (and bail in deleteSelectedCollection), so the delete confirmation is always for a homogeneous collection or saved-search selection, with count-aware Fluent strings. Drop the type noun from the Export/Generate Report/Create Bibliography labels, since those act on the references. Moves the affected strings to Fluent and removes the now-unused .properties keys. --- chrome/content/zotero/zoteroPane.js | 71 +++++++++++++------- chrome/locale/en-US/zotero/zotero.ftl | 69 +++++++++++++++++-- chrome/locale/en-US/zotero/zotero.properties | 22 ------ test/tests/collectionTreeTest.js | 25 +++++++ 4 files changed, 138 insertions(+), 49 deletions(-) diff --git a/chrome/content/zotero/zoteroPane.js b/chrome/content/zotero/zoteroPane.js index 33e34ca6d9..1cbbcc316a 100644 --- a/chrome/content/zotero/zoteroPane.js +++ b/chrome/content/zotero/zoteroPane.js @@ -2572,7 +2572,7 @@ var ZoteroPane = new function () { }; - this.deleteSelectedCollection = function (deleteItems) { + this.deleteSelectedCollection = async function (deleteItems) { var collectionTreeRows = this.getCollectionTreeRows(); if (!collectionTreeRows.length) { return; @@ -2613,22 +2613,35 @@ var ZoteroPane = new function () { this.displayCannotEditLibraryMessage(); return; } - + + // A mixed-type selection (e.g., a collection and a saved search) has no + // coherent confirmation, so only delete a homogeneous selection + if (collectionTreeRows.length > 1 + && !collectionTreeRows.every(r => r.type === collectionTreeRows[0].type)) { + return; + } + var ps = Services.prompt; buttonFlags = ps.BUTTON_POS_0 * ps.BUTTON_TITLE_IS_STRING + ps.BUTTON_POS_1 * ps.BUTTON_TITLE_CANCEL; var title, message; + var count = collectionTreeRows.length; // Work out the required title and message if (collectionTreeRows[0].isCollection()) { if (deleteItems) { - title = Zotero.getString('pane.collections.deleteWithItems.title'); - message = Zotero.getString('pane.collections.deleteWithItems'); + [title, message] = await document.l10n.formatValues([ + { id: 'collections-delete-with-items-title', args: { count } }, + { id: 'collections-delete-with-items-message', args: { count } }, + ]); } else { - title = Zotero.getString('pane.collections.delete.title'); - message = Zotero.getString('pane.collections.delete') - + "\n\n" - + Zotero.getString('pane.collections.delete.keepItems'); + let keepItems; + [title, message, keepItems] = await document.l10n.formatValues([ + { id: 'collections-delete-title', args: { count } }, + { id: 'collections-delete-message', args: { count } }, + { id: 'collections-delete-keep-items', args: { count } }, + ]); + message = message + "\n\n" + keepItems; } } else if (collectionTreeRows[0].isFeed()) { @@ -2636,8 +2649,10 @@ var ZoteroPane = new function () { message = Zotero.getString('pane.feed.deleteWithItems'); } else if (collectionTreeRows[0].isSearch()) { - title = Zotero.getString('pane.collections.deleteSearch.title'); - message = Zotero.getString('pane.collections.deleteSearch'); + [title, message] = await document.l10n.formatValues([ + { id: 'collections-delete-search-title', args: { count } }, + { id: 'collections-delete-search-message', args: { count } }, + ]); } // Display prompt @@ -3694,21 +3709,26 @@ var ZoteroPane = new function () { } // Adjust labels - document.l10n.setAttributes(m.editSelectedCollection, 'collections-menu-rename-collection'); + document.l10n.setAttributes(m.editSelectedCollection, 'collections-menu-rename'); document.l10n.setAttributes(m.moveCollection, 'collections-menu-move-collection'); document.l10n.setAttributes(m.copyCollection, 'collections-menu-copy-collection'); - m.deleteCollection.setAttribute('label', Zotero.getString('pane.collections.menu.delete.collection')); - m.deleteCollectionAndItems.setAttribute('label', Zotero.getString('pane.collections.menu.delete.collectionAndItems')); - m.exportCollection.setAttribute('label', Zotero.getString('pane.collections.menu.export.collection')); - m.createBibCollection.setAttribute('label', Zotero.getString('pane.collections.menu.createBib.collection')); - m.loadReport.setAttribute('label', Zotero.getString('pane.collections.menu.generateReport.collection')); + document.l10n.setAttributes(m.deleteCollection, 'collections-menu-delete', { count: collectionTreeRows.length }); + document.l10n.setAttributes(m.deleteCollectionAndItems, 'collections-menu-delete-with-items', { count: collectionTreeRows.length }); + document.l10n.setAttributes(m.exportCollection, 'collections-menu-export'); + document.l10n.setAttributes(m.createBibCollection, 'collections-menu-create-bibliography'); + document.l10n.setAttributes(m.loadReport, 'collections-menu-generate-report'); // New Subcollection and Rename act on a single collection, so hide them // when more than one row is selected if (collectionTreeRows.length > 1) { show = show.filter(id => id != 'newSubcollection' && id != 'editSelectedCollection'); } + // A mixed-type selection has no coherent delete confirmation, so hide the + // delete actions unless every selected row is a collection + if (!collectionTreeRows.every(r => r.isCollection())) { + show = show.filter(id => id != 'deleteCollection' && id != 'deleteCollectionAndItems'); + } // Hide move/copy when collections span multiple libraries, and disable // the report (its URL is scoped to a single library) @@ -3735,7 +3755,7 @@ var ZoteroPane = new function () { // Adjust labels m.refreshFeed.setAttribute('label', Zotero.getString('pane.collections.menu.refresh.feed')); m.markReadFeed.setAttribute('label', Zotero.getString('pane.collections.menu.markAsRead.feed')); - m.deleteCollectionAndItems.setAttribute('label', Zotero.getString('pane.collections.menu.delete.feedAndItems')); + document.l10n.setAttributes(m.deleteCollectionAndItems, 'collections-menu-unsubscribe'); } else if (collectionTreeRows.some(row => row.isFeeds())) { show = [ @@ -3770,14 +3790,19 @@ var ZoteroPane = new function () { } // Adjust labels - document.l10n.setAttributes(m.editSelectedCollection, 'collections-menu-edit-saved-search'); - m.duplicate.setAttribute('label', Zotero.getString('pane.collections.menu.duplicate.savedSearch')); + document.l10n.setAttributes(m.editSelectedCollection, 'collections-menu-edit-search'); + document.l10n.setAttributes(m.duplicate, 'collections-menu-duplicate-search'); m.duplicate.classList.add('zotero-menuitem-duplicate-saved-search'); m.duplicate.classList.remove('zotero-menuitem-duplicate-collection'); - m.deleteCollection.setAttribute('label', Zotero.getString('pane.collections.menu.delete.savedSearch')); - m.exportCollection.setAttribute('label', Zotero.getString('pane.collections.menu.export.savedSearch')); - m.createBibCollection.setAttribute('label', Zotero.getString('pane.collections.menu.createBib.savedSearch')); - m.loadReport.setAttribute('label', Zotero.getString('pane.collections.menu.generateReport.savedSearch')); + document.l10n.setAttributes(m.deleteCollection, 'collections-menu-delete-search', { count: collectionTreeRows.length }); + document.l10n.setAttributes(m.exportCollection, 'collections-menu-export'); + document.l10n.setAttributes(m.createBibCollection, 'collections-menu-create-bibliography'); + document.l10n.setAttributes(m.loadReport, 'collections-menu-generate-report'); + + // Hide delete for a mixed-type selection (see the collection branch) + if (!collectionTreeRows.every(r => r.isSearch())) { + show = show.filter(id => id != 'deleteCollection'); + } } else if (collectionTreeRows[0].isTrash()) { show = ['emptyTrash']; diff --git a/chrome/locale/en-US/zotero/zotero.ftl b/chrome/locale/en-US/zotero/zotero.ftl index c62f071a36..2f8451e910 100644 --- a/chrome/locale/en-US/zotero/zotero.ftl +++ b/chrome/locale/en-US/zotero/zotero.ftl @@ -195,15 +195,76 @@ collections-menu-show-recently-read = .label = Show { recently-read } item-menu-remove-from-recently-read = .label = Remove from { recently-read }… -collections-menu-rename-collection = - .label = Rename Collection +collections-menu-rename = + .label = Rename edit-saved-search = Edit Saved Search -collections-menu-edit-saved-search = - .label = { edit-saved-search } +collections-menu-edit-search = + .label = Edit Search +collections-menu-duplicate-search = + .label = Duplicate Search collections-menu-move-collection = .label = Move To collections-menu-copy-collection = .label = Copy To +collections-menu-export = + .label = Export… +collections-menu-generate-report = + .label = Generate Report… +collections-menu-create-bibliography = + .label = Create Bibliography… +collections-menu-unsubscribe = + .label = Unsubscribe… +collections-menu-delete = + .label = { $count -> + [one] Delete Collection… + *[other] Delete Collections… + } +collections-menu-delete-with-items = + .label = { $count -> + [one] Delete Collection and Items… + *[other] Delete Collections and Items… + } +collections-menu-delete-search = + .label = { $count -> + [one] Delete Search… + *[other] Delete Searches… + } + +collections-delete-title = + { $count -> + [one] Delete Collection + *[other] Delete Collections + } +collections-delete-message = + { $count -> + [one] Are you sure you want to delete this collection? + *[other] Are you sure you want to delete { $count } collections? + } +collections-delete-keep-items = + { $count -> + [one] Items within this collection will not be deleted. + *[other] Items within these collections will not be deleted. + } +collections-delete-with-items-title = + { $count -> + [one] Delete Collection and Items + *[other] Delete Collections and Items + } +collections-delete-with-items-message = + { $count -> + [one] Are you sure you want to delete this collection and move all items within it to the Trash? + *[other] Are you sure you want to delete { $count } collections and move all items within them to the Trash? + } +collections-delete-search-title = + { $count -> + [one] Delete Search + *[other] Delete Searches + } +collections-delete-search-message = + { $count -> + [one] Are you sure you want to delete this search? + *[other] Are you sure you want to delete { $count } searches? + } item-creator-moveDown = .label = Move Down diff --git a/chrome/locale/en-US/zotero/zotero.properties b/chrome/locale/en-US/zotero/zotero.properties index 81dff45f67..a3511726cb 100644 --- a/chrome/locale/en-US/zotero/zotero.properties +++ b/chrome/locale/en-US/zotero/zotero.properties @@ -251,16 +251,9 @@ date.relative.yearsAgo.one = 1 year ago date.relative.yearsAgo.multiple = %S years ago pane.collections.title = Collections -pane.collections.delete.title = Delete Collection -pane.collections.delete = Are you sure you want to delete the selected collection? -pane.collections.delete.keepItems = Items within this collection will not be deleted. -pane.collections.deleteWithItems.title = Delete Collection and Items -pane.collections.deleteWithItems = Are you sure you want to delete the selected collection and move all items within it to the Trash? pane.feed.deleteWithItems.title = Unsubscribe pane.feed.deleteWithItems = Are you sure you want to unsubscribe from this feed? -pane.collections.deleteSearch.title = Delete Search -pane.collections.deleteSearch = Are you sure you want to delete the selected search? pane.collections.emptyTrash = Are you sure you want to permanently remove items in the Trash? pane.collections.newSavedSeach = New Saved Search pane.collections.savedSearchName = Enter a name for this saved search: @@ -278,24 +271,9 @@ pane.collections.duplicate = Duplicate Items pane.collections.removeLibrary = Remove Library pane.collections.removeLibrary.text = Are you sure you want to permanently remove “%S” from this computer? -pane.collections.menu.duplicate.savedSearch = Duplicate Saved Search pane.collections.menu.remove.library = Remove Library… -pane.collections.menu.delete.collection = Delete Collection… -pane.collections.menu.delete.collectionAndItems = Delete Collection and Items… -pane.collections.menu.delete.savedSearch = Delete Saved Search… -pane.collections.menu.delete.feedAndItems = Unsubscribe from Feed… -pane.collections.menu.export.collection = Export Collection… -pane.collections.menu.export.savedSearch = Export Saved Search… -pane.collections.menu.export.feed = Export Feed… -pane.collections.menu.createBib.collection = Create Bibliography from Collection… -pane.collections.menu.createBib.savedSearch = Create Bibliography from Saved Search… -pane.collections.menu.createBib.feed = Create Bibliography from Feed… pane.collections.showCollectionInLibrary = Show Collection in Library -pane.collections.menu.generateReport.collection = Generate Report from Collection… -pane.collections.menu.generateReport.savedSearch = Generate Report from Saved Search… -pane.collections.menu.generateReport.feed = Generate Report from Feed… - pane.collections.menu.refresh.feed = Refresh Feed pane.collections.menu.refresh.allFeeds = Refresh All Feeds pane.collections.menu.markAsRead.feed = Mark Feed as Read diff --git a/test/tests/collectionTreeTest.js b/test/tests/collectionTreeTest.js index c3f9e03ee3..d00bf6fa2c 100644 --- a/test/tests/collectionTreeTest.js +++ b/test/tests/collectionTreeTest.js @@ -798,6 +798,31 @@ describe("Zotero.CollectionTree", function () { }); }); + describe("#deleteSelectedCollection()", function () { + it("shouldn't delete a mixed-type selection", async function () { + let collection = await createDataObject('collection'); + let search = await createDataObject('search'); + await cv.selectByID("C" + collection.id); + cv.selection.toggleSelect(cv.getRowIndexByID("S" + search.id)); + await zp.onCollectionSelected(); + assert.equal(cv.selection.count, 2); + + let stub = sinon.stub().returns(0); + let promptService = win.Services.prompt; + win.Services.prompt = { confirmEx: stub }; + try { + await zp.deleteSelectedCollection(false); + } + finally { + win.Services.prompt = promptService; + } + + assert.isFalse(stub.called, "Mixed selection shouldn't prompt or delete"); + assert.isFalse(collection.deleted); + assert.isFalse(search.deleted); + }); + }); + describe("#onDrop()", function () { /** * Simulate a drag and drop