From ec5419f80eec8f04edcb7625bdfdfa12be3eb230 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Mon, 3 Aug 2026 15:45:30 -0400 Subject: [PATCH] Search: Allow binding a group whose conditions match at any level Binding is meaningful for a condition that matches at every level -- a tag bound to an attachment means the tag is on the attachment -- but a group carrying one lost the binding as soon as the search was serialized, so "items with an attachment tagged foo" couldn't be built. (cherry picked from commit 9da57a9fe3fd06d92afef972db91536967cd68b9) --- .../content/zotero/elements/zoteroSearch.js | 29 ++++++---- test/tests/advancedSearchTest.js | 53 +++++++++++++++++++ 2 files changed, 73 insertions(+), 9 deletions(-) diff --git a/chrome/content/zotero/elements/zoteroSearch.js b/chrome/content/zotero/elements/zoteroSearch.js index 59d3440993..22db3012b4 100644 --- a/chrome/content/zotero/elements/zoteroSearch.js +++ b/chrome/content/zotero/elements/zoteroSearch.js @@ -603,6 +603,8 @@ // Zotero.Search.combineConditions), so treat it as 'item' here. let bindableBelow = resultLevel == 'any' ? 'item' : resultLevel; let counts = {}; + // Conditions that match at every level (a tag, say) can be bound to any of them + let anyLevel = 0; for (let row of this.conditionsContainer.children) { // Skip a row just added via "+" until the user engages with it, so adding a row // doesn't immediately suggest grouping it @@ -610,19 +612,29 @@ continue; } let level = row.conditionLevel; - if (Zotero.Search._isAncestorLevel(bindableBelow, level)) { + if (level == 'any') { + anyLevel++; + } + else if (Zotero.Search._isAncestorLevel(bindableBelow, level)) { counts[level] = (counts[level] || 0) + 1; } } - let levels = Object.keys(counts); - // Drop a stored binding once no condition at its level remains - if (this._resultLevel != 'any' && !levels.includes(this._resultLevel)) { + // The levels this group can bind to at all: one of its conditions matches there, or + // -- for a condition that matches anywhere -- any level below the result level + let optionLevels = ['attachment', 'note', 'annotation'].filter(l => counts[l] + || (anyLevel && Zotero.Search._isAncestorLevel(bindableBelow, l))); + // Drop a stored binding once it isn't one of them, as when the result level moves + // down to the bound level and binding there stops meaning anything + if (this._resultLevel != 'any' && !optionLevels.includes(this._resultLevel)) { this._resultLevel = 'any'; } - // Binding is offered once some level has 2+ conditions, and an existing binding - // stays visible (and clearable) even when its group no longer qualifies, so it - // can't invisibly constrain the group from a hidden menu - if (this._resultLevel == 'any' && !levels.some(l => counts[l] >= 2)) { + // Binding is offered once it would mean something: a level shared by 2+ conditions + // ties them to one entity, and a condition that matches at any level is narrowed to + // the bound one. An existing binding stays visible (and clearable) even when its + // group no longer qualifies, so it can't invisibly constrain the group from a + // hidden menu. + if (this._resultLevel == 'any' && !anyLevel + && !optionLevels.some(l => counts[l] >= 2)) { this.bindingMenu.hidden = true; // Plain group: "Match [all] of the following:" (the suffix carries the colon) this.querySelector('.join-mode-suffix').hidden = false; @@ -634,7 +646,6 @@ // Rebuild the popup only when its option set changes. Rebuilding it on every refresh // would replace the menuitems mid-selection -- when the change came from this menu // itself -- and wedge the drop-down. - let optionLevels = ['attachment', 'note', 'annotation'].filter(l => levels.includes(l)); let key = optionLevels.join(','); if (key !== this._bindingMenuKey) { this._bindingMenuKey = key; diff --git a/test/tests/advancedSearchTest.js b/test/tests/advancedSearchTest.js index d4e654826f..fdffee8023 100644 --- a/test/tests/advancedSearchTest.js +++ b/test/tests/advancedSearchTest.js @@ -1538,6 +1538,59 @@ describe("Advanced Search", function () { ['title', 'groupStart', 'joinMode', 'tag', 'tag', 'groupEnd']); }); + it("should keep a binding that scopes a condition matching at any level", async function () { + // An item whose attachment has the tag, and one where the tag is on a + // note instead + var hit = await createDataObject('item'); + var attachment = await importFileAttachment('test.png', { parentID: hit.id }); + attachment.setTags([{ tag: 'zbound' }]); + await attachment.saveTx(); + var miss = await createDataObject('item'); + var note = new Zotero.Item('note'); + note.parentID = miss.id; + note.setNote('note'); + note.setTags([{ tag: 'zbound' }]); + await note.saveTx(); + + var s = new Zotero.Search(); + s.libraryID = Zotero.Libraries.userLibraryID; + s.addCondition('resultLevel', 'item'); + s.addCondition('groupStart', 'true', ''); + s.addCondition('resultLevel', 'attachment'); + s.addCondition('tag', 'is', 'zbound'); + s.addCondition('groupEnd', 'true', ''); + pane.search = s; + searchBox.updateSearch(); + + var group = conditions.querySelector('search-condition-group'); + assert.equal(group.resultLevel, 'attachment'); + // The tag has to be on the attachment, not anywhere in the item + var ids = await searchBox.search.search(); + assert.include(ids, hit.id); + assert.notInclude(ids, miss.id); + }); + + it("should drop a binding that the result level makes meaningless", function () { + var s = new Zotero.Search(); + s.libraryID = Zotero.Libraries.userLibraryID; + s.addCondition('resultLevel', 'attachment'); + s.addCondition('groupStart', 'true', ''); + s.addCondition('resultLevel', 'attachment'); + s.addCondition('tag', 'is', 'zbound'); + s.addCondition('groupEnd', 'true', ''); + pane.search = s; + searchBox.updateSearch(); + + // Results are attachments, so binding the group to the attachment says + // nothing the result level doesn't + var group = conditions.querySelector('search-condition-group'); + assert.equal(group.resultLevel, 'any'); + assert.deepEqual( + Object.values(searchBox.search.getConditions()).map(c => c.condition), + ['resultLevel', 'groupStart', 'tag', 'groupEnd'] + ); + }); + it("should wrap a condition in a new group in its place", function () { var s = new Zotero.Search(); s.libraryID = Zotero.Libraries.userLibraryID;