From 0ee3e4052e4e783792c99fb5ca9193b6ad41bd0c Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Thu, 20 Aug 2026 15:36:55 -0400 Subject: [PATCH] Don't prompt to save an unchanged saved search https://forums.zotero.org/discussion/133302/zotero-10-questions-about-search --- .../zotero/elements/advancedSearchPane.js | 36 ++++++++++++++ chrome/content/zotero/zoteroPane.js | 6 +-- test/tests/advancedSearchTest.js | 48 +++++++++++++++++++ 3 files changed, 87 insertions(+), 3 deletions(-) diff --git a/chrome/content/zotero/elements/advancedSearchPane.js b/chrome/content/zotero/elements/advancedSearchPane.js index 65b701cb3b..c3be83b149 100644 --- a/chrome/content/zotero/elements/advancedSearchPane.js +++ b/chrome/content/zotero/elements/advancedSearchPane.js @@ -140,6 +140,42 @@ this._search.addCondition('title', 'contains', ''); } this._searchElem.search = this._search; + this._loadedState = this.type === 'saved' ? this._getState() : null; + } + + /** + * The editor's current name and conditions, for comparison against the state the + * search was loaded with + * + * The search is serialized through the editor, since rendering an existing search + * can normalize it (e.g., folding a legacy 'noChildren' condition into the result + * level), and the normalized form is what a save would write. + */ + _getState() { + this._searchElem.updateSearch(); + return { + name: this._nameField.value, + conditions: this._search.toJSON().conditions + }; + } + + /** + * Whether the name or conditions have been edited since the search was loaded, + * for type "saved" + */ + get hasChanges() { + if (this.type !== 'saved') { + throw new Error('hasChanges is unsupported for temporary search'); + } + if (!this._loadedState) { + return false; + } + let state = this._getState(); + return state.name !== this._loadedState.name + || state.conditions.length !== this._loadedState.conditions.length + || state.conditions.some((condition, i) => !Zotero.Searches.conditionEquals( + condition, this._loadedState.conditions[i] + )); } _ensureSearch() { diff --git a/chrome/content/zotero/zoteroPane.js b/chrome/content/zotero/zoteroPane.js index be39e1475d..69e0eb2753 100644 --- a/chrome/content/zotero/zoteroPane.js +++ b/chrome/content/zotero/zoteroPane.js @@ -1921,15 +1921,15 @@ var ZoteroPane = new function () { /** * Prompt to save changes in the open saved-search editor and close it * - * If the search being edited was deleted or trashed, the editor is closed without - * a prompt, since there's nothing left to save the changes to. + * If the editor has no changes to save, or if the search being edited was deleted or + * trashed, leaving nothing to save the changes to, the editor is closed without a prompt. * * @return {Promise} - False if the user chose Cancel, leaving the editor open */ this._confirmCloseSavedSearchEditor = async function () { let deck = document.getElementById('zotero-advanced-search-pane-deck'); let editedSearch = Zotero.Searches.get(deck.pane.editedSearchID); - if (!editedSearch || editedSearch.deleted) { + if (!editedSearch || editedSearch.deleted || !deck.pane.hasChanges) { await deck.pane.cancel(); return true; } diff --git a/test/tests/advancedSearchTest.js b/test/tests/advancedSearchTest.js index 8854d1def5..52471d59af 100644 --- a/test/tests/advancedSearchTest.js +++ b/test/tests/advancedSearchTest.js @@ -698,6 +698,49 @@ describe("Advanced Search", function () { await selectLibrary(win); }); + it("should close the saved-search editor without prompting when nothing was changed", async function () { + var saved = await createDataObject('search', { name: "UnchangedEditing" }); + await select(win, saved); + await zp.setSavedSearchEditorState('open'); + assert.equal(deck.selectedSearchType, 'saved'); + + let stub = sinon.stub().returns(1); // Cancel + let promptService = win.Services.prompt; + win.Services.prompt = { confirmEx: stub }; + try { + await selectLibrary(win); + assert.equal(stub.callCount, 0); + assert.equal(deck.state, 'closed'); + } + finally { + win.Services.prompt = promptService; + } + + await saved.eraseTx(); + }); + + it("should prompt when only the saved search's name was changed", async function () { + var saved = await createDataObject('search', { name: "RenameWhileEditing" }); + await select(win, saved); + await zp.setSavedSearchEditorState('open'); + + deck.pane.querySelector('#saved-search-name').value = "RenameWhileEditing 2"; + + let stub = sinon.stub().returns(0); // Save + let promptService = win.Services.prompt; + win.Services.prompt = { confirmEx: stub }; + try { + await selectLibrary(win); + assert.equal(stub.callCount, 1); + assert.equal(saved.name, "RenameWhileEditing 2"); + } + finally { + win.Services.prompt = promptService; + } + + await saved.eraseTx(); + }); + it("should revert to the edited search without re-prompting when canceling", async function () { var search1 = await createDataObject('search', { name: "CancelEditing1" }); var search2 = await createDataObject('search', { name: "CancelEditing2" }); @@ -707,6 +750,11 @@ describe("Advanced Search", function () { await zp.setSavedSearchEditorState('open'); assert.equal(deck.selectedSearchType, 'saved'); + // Edit the editor's working copy, so that closing it prompts + var searchBox = deck.pane.querySelector('zoterosearch'); + searchBox.querySelector('.conditions').firstChild.querySelector('#valuefield').value = 'edited'; + searchBox.updateSearch(); + // zoteroPane.js uses the pane window's Services, so stub there. Set it before // touching the selection so no prompt can reach the real (modal) service. let stub = sinon.stub().returns(1); // Cancel