diff --git a/chrome/content/zotero/elements/advancedSearchPane.js b/chrome/content/zotero/elements/advancedSearchPane.js
index 5a2a648a38..93669520e5 100644
--- a/chrome/content/zotero/elements/advancedSearchPane.js
+++ b/chrome/content/zotero/elements/advancedSearchPane.js
@@ -127,6 +127,9 @@
}
else {
this._search = new Zotero.Search();
+ // Default a fresh search to top-level items, so a condition on a child
+ // (e.g. attachment content) maps up to its item without any grouping
+ this._search.addCondition('resultLevel', 'item');
this._search.addCondition('title', 'contains', '');
}
this._searchElem.search = this._search;
diff --git a/chrome/content/zotero/elements/zoteroSearch.js b/chrome/content/zotero/elements/zoteroSearch.js
index 164603992e..9ae1de255d 100644
--- a/chrome/content/zotero/elements/zoteroSearch.js
+++ b/chrome/content/zotero/elements/zoteroSearch.js
@@ -38,9 +38,11 @@
class ZoteroSearch extends SearchElementBase {
content = MozXULElement.parseXULToFragment(`
+
-
+
+
`, ['chrome://zotero/locale/zotero.dtd', 'chrome://zotero/locale/searchbox.dtd']);
@@ -65,9 +67,20 @@
init() {
this.rootGroup = this.querySelector('search-condition-group[root]');
this.addEventListener('keypress', event => this.handleKeyPress(event));
- // Re-evaluate which remove buttons are enabled as the conditions change
- this.addEventListener('input', () => this.updateRemoveButtons());
- this.addEventListener('command', () => this.updateRemoveButtons());
+ // Re-evaluate which remove buttons are enabled and which groups can bind to the
+ // same descendant as the conditions change
+ this.addEventListener('input', () => {
+ this.updateRemoveButtons();
+ this.updateBindingMenus();
+ this.updateBindingHint();
+ this.updateLevelWarning();
+ });
+ this.addEventListener('command', () => {
+ this.updateRemoveButtons();
+ this.updateBindingMenus();
+ this.updateBindingHint();
+ this.updateLevelWarning();
+ });
}
// Build the condition tree (root group, nested groups, and search-global
@@ -75,9 +88,11 @@
renderConditions() {
var root = this.rootGroup;
- for (let name of ['recursive', 'noChildren', 'includeParentsAndChildren']) {
- this.querySelector('#' + name + 'Checkbox').checked = false;
- }
+ this.querySelector('#recursiveCheckbox').checked = false;
+ // 'Include parent and child items' is a legacy hack subsumed by result levels;
+ // its checkbox is shown only for an existing search that still carries the flag
+ this.querySelector('#includeParentsAndChildrenCheckbox').checked = false;
+ this.querySelector('#search-legacy-options').hidden = true;
root.clear();
@@ -90,16 +105,35 @@
let condition = conditions[id];
switch (condition.condition) {
case 'recursive':
+ this.querySelector('#recursiveCheckbox').checked = condition.operator == 'true';
+ continue;
+
+ // Legacy "show only top-level items" is exactly result level = item, so
+ // fold it into the root result level rather than a checkbox
case 'noChildren':
+ if (condition.operator == 'true') {
+ root.resultLevel = 'item';
+ }
+ continue;
+
+ // Legacy include-parents-and-children: keep it editable for searches that
+ // have it, but don't offer it on new ones
case 'includeParentsAndChildren':
- this.querySelector('#' + condition.condition + 'Checkbox').checked
+ this.querySelector('#includeParentsAndChildrenCheckbox').checked
= condition.operator == 'true';
+ if (condition.operator == 'true') {
+ this.querySelector('#search-legacy-options').hidden = false;
+ }
continue;
case 'joinMode':
stack[stack.length - 1].joinMode = condition.operator;
continue;
+ case 'resultLevel':
+ stack[stack.length - 1].resultLevel = condition.operator;
+ continue;
+
case 'groupStart': {
let group = document.createXULElement('search-condition-group');
stack[stack.length - 1].conditionsContainer.appendChild(group);
@@ -124,6 +158,73 @@
}
this.updateRemoveButtons();
+ this.updateBindingMenus();
+ this.updateBindingHint();
+ this.updateLevelWarning();
+ }
+
+ // Refresh each nested group's same-entity binding menu (visibility and options)
+ updateBindingMenus() {
+ for (let group of this.querySelectorAll('search-condition-group')) {
+ group.updateBindingMenu();
+ }
+ }
+
+ // Refresh every group's level warning. Each group flags only its own conditions, so the
+ // message sits on the group whose conditions actually conflict and speaks to that group's
+ // controls (see SearchConditionGroup.updateLevelWarning).
+ updateLevelWarning() {
+ for (let group of this.querySelectorAll('search-condition-group')) {
+ group.updateLevelWarning();
+ }
+ }
+
+ // Offer to bind ungrouped sibling conditions at the root. The root can't bind itself
+ // (it returns the result level), so 2+ of its conditions sharing a level below the
+ // result level are wrappable into a "same attachment" group -- surfaced as a hint
+ // with a button per such level. Nested groups use their own binding menu instead.
+ updateBindingHint() {
+ var hint = this.querySelector('#search-binding-hint');
+ var resultLevel = this.rootGroup.resultLevel;
+ var counts = {};
+ for (let row of this.rootGroup.conditionsContainer.children) {
+ // Skip rows the user hasn't filled in yet, so a freshly added (or just-retyped)
+ // condition doesn't trigger the hint until it actually has a value
+ if (row.localName != 'zoterosearchcondition' || !row.isPopulated()) {
+ continue;
+ }
+ let level = row.conditionLevel;
+ if (Zotero.Search._isAncestorLevel(resultLevel, level)) {
+ counts[level] = (counts[level] || 0) + 1;
+ }
+ }
+ var levels = ['attachment', 'note', 'annotation'].filter(l => counts[l] >= 2);
+ // Only rebuild when the set of bindable levels changes, so re-running this on every
+ // keystroke doesn't recreate the rows and make the hint flicker.
+ let key = levels.join(',');
+ if (key === this._bindingHintKey) {
+ return;
+ }
+ this._bindingHintKey = key;
+ hint.replaceChildren();
+ if (!levels.length) {
+ hint.hidden = true;
+ return;
+ }
+ // One self-contained line per level: a statement naming that level and a button to
+ // group its conditions into one entity. Separate lines when 2+ levels each qualify.
+ for (let level of levels) {
+ let row = document.createXULElement('hbox');
+ row.setAttribute('align', 'center');
+ let label = document.createXULElement('label');
+ label.setAttribute('data-l10n-id', 'advanced-search-binding-hint-' + level);
+ let button = document.createXULElement('button');
+ button.setAttribute('data-l10n-id', 'advanced-search-bind-same-' + level);
+ button.addEventListener('command', () => this.rootGroup.bindSameEntity(level));
+ row.append(label, button);
+ hint.append(row);
+ }
+ hint.hidden = false;
}
// Regenerate the search's flat condition list from the current tree. The DOM is
@@ -137,14 +238,23 @@
var flat = [];
this.collectGroup(this.rootGroup, flat, true);
- // Search-global options
- for (let name of ['recursive', 'noChildren', 'includeParentsAndChildren']) {
- if (this.querySelector('#' + name + 'Checkbox').checked) {
- flat.push({ condition: name, operator: 'true', value: null });
- }
+ // Search-global options. noChildren is no longer emitted here -- it's carried by
+ // the result level (resultLevel = item). includeParentsAndChildren is emitted only when
+ // its legacy checkbox is present and still checked, so unchecking it drops it.
+ if (this.querySelector('#recursiveCheckbox').checked) {
+ flat.push({ condition: 'recursive', operator: 'true', value: null });
+ }
+ if (this.querySelector('#includeParentsAndChildrenCheckbox').checked) {
+ flat.push({ condition: 'includeParentsAndChildren', operator: 'true', value: null });
}
this.rebuildConditions(flat);
+
+ // Any mutation runs through here (including paths whose menus stopPropagation, like
+ // changing or removing a condition), so refresh the derived UI from one place
+ this.updateBindingMenus();
+ this.updateBindingHint();
+ this.updateLevelWarning();
}
// Append a group's serialized form to `flat`. The root contributes its
@@ -157,6 +267,11 @@
if (group.joinMode == 'any') {
flat.push({ condition: 'joinMode', operator: 'any', value: null });
}
+ // A concrete result level is emitted as a marker inside the group, like joinMode; 'any'
+ // (the default) is omitted
+ if (group.resultLevel && group.resultLevel != 'any') {
+ flat.push({ condition: 'resultLevel', operator: group.resultLevel, value: null });
+ }
for (let child of group.conditionsContainer.children) {
if (child.localName == 'zoterosearchcondition') {
let data = child.getConditionData();
@@ -299,6 +414,16 @@
content = MozXULElement.parseXULToFragment(`
+
+
+
`, ['chrome://zotero/locale/zotero.dtd', 'chrome://zotero/locale/searchbox.dtd']);
init() {
this.joinMenu = this.querySelector('.join-mode-menu');
+ this.resultLevelMenu = this.querySelector('.result-level-menu');
+ this.bindingMenu = this.querySelector('.binding-menu');
this.conditionsContainer = this.querySelector('.conditions');
+ // The group's own warning element, stashed at init to avoid re-querying.
+ this.levelWarning = this.querySelector('.level-warning');
+
+ // The result level is tracked here and reflected to whichever control is active: the root's
+ // result-level menu ("Find ..."), or a nested group's binding menu ("... in the
+ // same attachment"). collectGroup/renderConditions read and write `resultLevel`.
+ this._resultLevel = 'any';
+
+ // The root surfaces the result-level menu; a nested group hides it and instead
+ // shows a binding menu (built on demand by updateBindingMenu) when it holds
+ // conditions that can be bound to the same descendant.
+ this.resultLevelMenu.value = 'any';
+ var scopePrefix = this.querySelector('.result-level-prefix');
+ if (this.isRoot) {
+ // Read as one sentence: "Find [Top-level items] matching [all] of the following:"
+ scopePrefix.setAttribute('data-l10n-id', 'advanced-search-result-level-prefix-root');
+ this.querySelector('.join-mode-prefix').setAttribute('data-l10n-id', 'advanced-search-join-prefix-root');
+ this.resultLevelControl = this.resultLevelMenu;
+ }
+ else {
+ // Nested: "Match [all] of the following:" -- the result level lives on the
+ // root. The binding menu (and its hiding of the suffix) is set up in
+ // updateBindingMenu().
+ this.resultLevelMenu.hidden = true;
+ scopePrefix.hidden = true;
+ this.resultLevelControl = this.bindingMenu;
+ }
+
+ // Keep the stored result level in sync when the user changes the control (the
+ // command target may be the menulist or a menuitem inside it), so a nested
+ // binding that later hides still round-trips its last value
+ this.addEventListener('command', (event) => {
+ if (this.resultLevelControl && this.resultLevelControl.contains(event.target)) {
+ this._resultLevel = this.resultLevelControl.value || 'any';
+ }
+ });
// At init the group has no nested groups yet, so these resolve to its own
// caption buttons
this.addConditionButton = this.querySelector('.add-condition');
@@ -357,8 +527,221 @@
this.joinMenu.value = val;
}
+ // The group's result level: 'any' (no level constraint -- mixed result for the root,
+ // plain grouping for a nested group) or a concrete 'item'/'attachment'/'note'/
+ // 'annotation' level for cross-level mapping. The active control's current
+ // selection is the source of truth; fall back to the stored value for a nested
+ // binding menu that's hidden (it has no options to read).
+ get resultLevel() {
+ if (this.resultLevelControl && !this.resultLevelControl.hidden) {
+ return this.resultLevelControl.value || 'any';
+ }
+ return this._resultLevel;
+ }
+
+ set resultLevel(val) {
+ this._resultLevel = val || 'any';
+ // Reflect to the active control if it currently offers a matching option; the
+ // binding menu's options are (re)built by updateBindingMenu()
+ let popup = this.resultLevelControl && this.resultLevelControl.querySelector('menupopup');
+ if (popup && [...popup.children].some(item => item.value == this._resultLevel)) {
+ this.resultLevelControl.value = this._resultLevel;
+ }
+ }
+
+ // Build the nested-group binding menu ("... in the same attachment"), shown only
+ // when binding is meaningful: 2+ conditions sharing a level below the result level.
+ updateBindingMenu() {
+ if (this.isRoot) {
+ return;
+ }
+ let resultLevel = 'any';
+ if (this.searchElement && this.searchElement.rootGroup) {
+ resultLevel = this.searchElement.rootGroup.resultLevel;
+ }
+ // Count this group's direct condition rows by level, keeping only levels below the
+ // result level -- those are what a group can bind to the same entity
+ let counts = {};
+ for (let row of this.conditionsContainer.children) {
+ // Skip rows the user hasn't filled in yet, so a freshly added (or just-retyped)
+ // condition doesn't trigger the hint until it actually has a value
+ if (row.localName != 'zoterosearchcondition' || !row.isPopulated()) {
+ continue;
+ }
+ let level = row.conditionLevel;
+ if (Zotero.Search._isAncestorLevel(resultLevel, level)) {
+ counts[level] = (counts[level] || 0) + 1;
+ }
+ }
+ let levels = Object.keys(counts);
+ // Binding is only meaningful when some level has 2+ conditions. When it isn't,
+ // hide the menu but preserve any stored level (a single descendant condition
+ // maps the same way bound or not, so it round-trips losslessly).
+ if (!levels.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;
+ this.querySelector('.join-mode-following').hidden = true;
+ this._bindingMenuKey = null;
+ return;
+ }
+ // Drop a stored binding whose level is no longer offered
+ if (this._resultLevel != 'any' && !levels.includes(this._resultLevel)) {
+ this._resultLevel = 'any';
+ }
+
+ // 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;
+ let popup = this.bindingMenu.querySelector('menupopup');
+ popup.replaceChildren();
+ let separate = document.createXULElement('menuitem');
+ separate.setAttribute('value', 'any');
+ separate.setAttribute('data-l10n-id', 'advanced-search-binding-separate');
+ popup.append(separate);
+ for (let level of optionLevels) {
+ let item = document.createXULElement('menuitem');
+ item.setAttribute('value', level);
+ item.setAttribute('data-l10n-id', 'advanced-search-binding-same-' + level);
+ popup.append(item);
+ }
+ }
+ this.bindingMenu.hidden = false;
+ // Bound group: "Match [all] of the following in the same attachment". The binding
+ // phrase ends the caption, so swap the legacy "of the following:" (with its colon)
+ // for the colon-less "of the following" that precedes the binding menu.
+ this.querySelector('.join-mode-suffix').hidden = true;
+ this.querySelector('.join-mode-following').hidden = false;
+ this.bindingMenu.value = this._resultLevel;
+ }
+
+ // The level this group's conditions are actually matched at: its own result level (the
+ // result type for the root, the binding for a nested group) if set, otherwise the level
+ // it inherits from its enclosing group. Mirrors the engine, where an unbound
+ // ("separately") group maps its conditions to the parent's level rather than
+ // combining them at no level.
+ effectiveLevel() {
+ if (this.resultLevel != 'any') {
+ return this.resultLevel;
+ }
+ let parent = this.parentElement && this.parentElement.closest('search-condition-group');
+ return parent ? parent.effectiveLevel() : this.resultLevel;
+ }
+
+ // Warn when this group's own conditions can never combine: a child whose level can't
+ // reach the group's effective level, or -- with no level anywhere up the chain (a mixed
+ // result type) -- an "all" of children on different item-hierarchy branches. Each group
+ // flags only its own conditions, so the message sits where the problem is.
+ updateLevelWarning() {
+ let ownLevel = this.resultLevel;
+ let level = this.effectiveLevel();
+ // This group's direct children's levels: a populated condition row's own level (an
+ // empty row doesn't warn until it's filled in) or a nested group's binding. 'any'
+ // combines with anything, so drop it.
+ let childLevels = [...this.conditionsContainer.children].map((child) => {
+ if (child.localName == 'zoterosearchcondition') {
+ return child.isPopulated() ? child.conditionLevel : null;
+ }
+ if (child.localName == 'search-condition-group') {
+ return child.resultLevel;
+ }
+ return null;
+ }).filter(l => l && l != 'any');
+
+ let messageID = null;
+ let args = null;
+ let resultTypeArgs = () => {
+ let item = this.resultLevelMenu.querySelector('menuitem[value="item"]');
+ return { topLevelItems: item ? item.getAttribute('label') : 'top-level items' };
+ };
+ if (level != 'any') {
+ // A child that can't reach the effective level can never match here
+ if (childLevels.some(l => !this.levelsCombine(l, level))) {
+ if (!this.isRoot && ownLevel != 'any') {
+ // This group's own binding is the constraint, so "match separately" fixes it
+ messageID = 'advanced-search-group-warning-unreachable';
+ args = { entity: ownLevel };
+ }
+ else {
+ // The result type (this group's, or one it inherits) is the constraint
+ messageID = 'advanced-search-level-warning-unreachable';
+ args = resultTypeArgs();
+ }
+ }
+ }
+ else if (this.joinMode == 'all' && new Set(childLevels).size >= 2) {
+ // No level anywhere up the chain (mixed result type): ANDing conditions on
+ // different branches can never all match
+ let anyItem = this.joinMenu.querySelector('menuitem[value="any"]');
+ let matchAny = anyItem ? anyItem.getAttribute('label') : 'any';
+ if (this.isRoot) {
+ messageID = 'advanced-search-level-warning-mixed';
+ args = { matchAny, ...resultTypeArgs() };
+ }
+ else {
+ // Reachable only when the result type is "any", so setting one fixes it too
+ messageID = 'advanced-search-group-warning-mixed';
+ args = { matchAny, ...resultTypeArgs() };
+ }
+ }
+
+ // Only touch the DOM when the message changes, so re-running on every keystroke
+ // doesn't re-translate the string and make the warning flicker.
+ let key = messageID ? messageID + '\n' + JSON.stringify(args) : '';
+ if (key === this._levelWarningKey) {
+ return;
+ }
+ this._levelWarningKey = key;
+ if (messageID) {
+ document.l10n.setAttributes(this.levelWarning.querySelector('description'), messageID, args);
+ }
+ this.levelWarning.hidden = !messageID;
+ }
+
+ // Two levels combine if one is an ancestor of the other (or equal); 'any' matches any.
+ levelsCombine(a, b) {
+ return a == 'any' || b == 'any' || a == b
+ || Zotero.Search._isAncestorLevel(a, b) || Zotero.Search._isAncestorLevel(b, a);
+ }
+
+ // Wrap this group's direct condition rows that match `level` into a new child group
+ // bound to that level ("the same attachment"). Used by the discoverability hint.
+ // Rows are rebuilt from their data rather than moved, since detaching a custom element
+ // wipes its contents.
+ bindSameEntity(level) {
+ let rows = [...this.conditionsContainer.children].filter(
+ row => row.localName == 'zoterosearchcondition' && row.conditionLevel == level);
+ if (rows.length < 2) {
+ return;
+ }
+ let newGroup = document.createXULElement('search-condition-group');
+ this.conditionsContainer.insertBefore(newGroup, rows[0]);
+ for (let row of rows) {
+ let data = row.getConditionData();
+ let ref;
+ if (data) {
+ let [condition, mode] = Zotero.SearchConditions.parseCondition(data.condition);
+ ref = { id: undefined, condition, mode, operator: data.operator, value: data.value, required: false };
+ }
+ newGroup.addCondition(ref);
+ row.remove();
+ }
+ newGroup.resultLevel = level;
+
+ let search = this.searchElement;
+ search.updateSearch();
+ search.updateRemoveButtons();
+ search.updateBindingMenus();
+ search.updateBindingHint();
+ }
+
clear() {
this.joinMode = 'all';
+ this.resultLevel = 'any';
while (this.conditionsContainer.firstChild) {
this.conditionsContainer.removeChild(this.conditionsContainer.firstChild);
}
@@ -757,6 +1140,14 @@
document.l10n.setAttributes(valueMenu, 'advanced-search-condition-input', { label: valueMenu.label });
}
this.updateMenuCheckboxesRecursive(operatorsList, operatorsList.selectedItem.getAttribute('value'));
+
+ // Changing the condition or operator is a mutation like add/remove, so rebuild the
+ // search and refresh the derived UI (binding hint, level warning) right away. The
+ // condition/operator menus stopPropagation, so this won't happen via event bubbling.
+ // (updateSearch no-ops during the initial render via its own guard.)
+ if (this.parent) {
+ this.parent.updateSearch();
+ }
}
createValueMenu(rows) {
@@ -1079,6 +1470,13 @@
newGroup.conditionsContainer.firstElementChild.querySelector('#conditionsmenu').focus();
}
+ // The item level this condition matches at ('item' by default), used to decide
+ // cross-level binding in a group
+ get conditionLevel() {
+ let data = this.selectedCondition && Zotero.SearchConditions.get(this.selectedCondition);
+ return (data && data.level) || 'item';
+ }
+
// Whether a value has been entered, used to decide whether the last
// remaining condition can be cleared back to the default state
isPopulated() {
diff --git a/chrome/locale/en-US/zotero/zotero.ftl b/chrome/locale/en-US/zotero/zotero.ftl
index b26551557a..173874f7d3 100644
--- a/chrome/locale/en-US/zotero/zotero.ftl
+++ b/chrome/locale/en-US/zotero/zotero.ftl
@@ -909,6 +909,57 @@ advanced-search-group-btn =
.tooltiptext = Add Condition Group
advanced-search-remove-group-btn =
.tooltiptext = Remove Group
+advanced-search-result-level-menu =
+ .aria-label = Result type
+advanced-search-result-level-prefix-root =
+ .value = Find
+advanced-search-join-prefix-root =
+ .value = matching
+advanced-search-result-level-any =
+ .label = any items
+advanced-search-result-level-item =
+ .label = top-level items
+advanced-search-result-level-attachment =
+ .label = attachments
+advanced-search-result-level-note =
+ .label = notes
+advanced-search-result-level-annotation =
+ .label = annotations
+advanced-search-binding-menu =
+ .aria-label = Match against the same item
+advanced-search-binding-separate =
+ .label = separately
+advanced-search-binding-same-attachment =
+ .label = in the same attachment
+advanced-search-binding-same-note =
+ .label = in the same note
+advanced-search-binding-same-annotation =
+ .label = in the same annotation
+# Shown before the binding menu so a bound group reads "Match all of the following in the same
+# annotation" (the legacy "of the following:" -- with the colon -- is kept for a plain group)
+advanced-search-of-the-following =
+ .value = of the following
+advanced-search-binding-hint-attachment =
+ .value = These conditions can match separate attachments.
+advanced-search-binding-hint-note =
+ .value = These conditions can match separate notes.
+advanced-search-binding-hint-annotation =
+ .value = These conditions can match separate annotations.
+advanced-search-level-warning-mixed = These conditions cannot all match the same item, so this search will never return results. Try matching “{ $matchAny }” of them, or set the result type to “{ $topLevelItems }”.
+advanced-search-level-warning-unreachable = This search has a condition that cannot apply to the chosen result type. Set the result type to “{ $topLevelItems }” or remove the incompatible condition.
+advanced-search-group-warning-unreachable =
+ A condition here cannot be in the same { $entity ->
+ [attachment] attachment
+ [note] note
+ *[annotation] annotation
+ }. Match these separately or remove the incompatible condition.
+advanced-search-group-warning-mixed = These conditions cannot all match the same item, so this group will never match. Try matching “{ $matchAny }” of them, or set the result type to “{ $topLevelItems }”.
+advanced-search-bind-same-attachment =
+ .label = Match the same attachment
+advanced-search-bind-same-note =
+ .label = Match the same note
+advanced-search-bind-same-annotation =
+ .label = Match the same annotation
advanced-search-conditions-menu =
.aria-label = Search condition
.label = { $label }
diff --git a/scss/elements/_advancedSearchPane.scss b/scss/elements/_advancedSearchPane.scss
index c538e82509..bf16a6919b 100644
--- a/scss/elements/_advancedSearchPane.scss
+++ b/scss/elements/_advancedSearchPane.scss
@@ -12,7 +12,7 @@ advanced-search-deck {
advanced-search-pane {
@include inactive-opacity;
-
+
flex-direction: column;
// Inline padding is further down the tree so focus rings work with overflow: auto
padding-block: 8px;
diff --git a/scss/elements/_zoteroSearch.scss b/scss/elements/_zoteroSearch.scss
index c1dd224333..661ae4d7e7 100644
--- a/scss/elements/_zoteroSearch.scss
+++ b/scss/elements/_zoteroSearch.scss
@@ -10,6 +10,9 @@ zoterosearch {
gap: 8px;
overflow: auto;
max-height: 33vh;
+ // Keep the whole builder a comfortable width on a wide window (left-aligned) rather
+ // than stretching the rows across the full width. The value field flexes within this.
+ max-width: 64em;
@include macOS-normalize-controls;
// Use the full pane width
@@ -23,6 +26,9 @@ zoterosearch {
// Zero the toolkit groupbox margin/padding
margin: 0;
padding: 0;
+ // The XUL groupbox shrink-wraps to content by default; fill its parent so the
+ // conditions box (and the value field) can use the available width
+ width: -moz-available;
.conditions {
min-width: fit-content; // Grow with content, don't overflow
@@ -118,6 +124,76 @@ zoterosearch {
}
}
+ .level-warning {
+ // Each group's own warning, in a contained box below its conditions (the root's sits at
+ // the bottom; a nested group's sits inside its box) rather than as loose text. A light
+ // red callout mirrors the binding hint's light blue one, but at full size (vs the hint's
+ // smaller text) since this is a hard error -- the search can never match -- not a hint.
+ color: var(--accent-red);
+ // 10px inline is a small visual nudge to bring the text slightly in (it doesn't line up
+ // with anything exactly); 8px block keeps the two-line message from cramping.
+ padding: 8px 10px;
+ background: var(--accent-red10);
+ border-radius: 6px;
+ // Fill the pane width but contribute no intrinsic (max-content) width of its own, so
+ // showing/hiding this notice can't widen the search pane and shrink the collections
+ // pane. (Its long text would otherwise push the pane to its max-width.)
+ box-sizing: border-box;
+ width: 0;
+ min-width: 100%;
+
+ description {
+ // Wrap to the available width instead of overflowing the pane; min-width: 0 lets
+ // it shrink within the flex row rather than contributing an intrinsic width
+ flex: 1;
+ min-width: 0;
+ margin: 0;
+ white-space: normal;
+ }
+ }
+
+ // A nested group's warning sits inside the outer box, so leave room below it rather than
+ // letting it butt against the box's bottom edge. (The root's warning is below the box.)
+ search-condition-group:not([root]) .level-warning {
+ margin-block-end: 6px;
+ }
+
+ #search-binding-hint {
+ // A distinct suggestion callout under the conditions (not plain body text): a light
+ // blue, rounded block with slightly smaller text, one "[statement] [button]" line per
+ // bindable level. Fills the pane width but contributes no intrinsic width (like the
+ // warning) so its line can't widen the search pane.
+ display: flex;
+ flex-direction: column;
+ // Each row hugs its content (so the button sits right next to the text) while the
+ // box itself still fills the pane width
+ align-items: flex-start;
+ gap: 8px;
+ padding: 8px;
+ font-size: 0.9em;
+ background: var(--accent-blue10);
+ border-radius: 6px;
+ box-sizing: border-box;
+ width: 0;
+ min-width: 100%;
+
+ hbox {
+ display: flex;
+ align-items: center;
+ gap: 5px;
+ }
+
+ button {
+ margin: 0;
+ }
+
+ // The explicit `display: flex` above overrides the `hidden` attribute's display:none,
+ // so re-assert it -- otherwise the empty box shows when there's nothing to suggest
+ &[hidden] {
+ display: none;
+ }
+ }
+
#search-option-checkboxes {
flex-direction: row;
flex-wrap: wrap;
@@ -144,6 +220,13 @@ zoterosearch {
label {
margin: 0;
}
+
+ // Native menulists sit a couple pixels high relative to the inline label text
+ // ("Find ... matching ... of the following:"); baseline/align-self alignment is
+ // locked by the platform, so nudge them down to sit on the text
+ menulist {
+ margin-block-start: 2px;
+ }
}
label:first-child:not(tooltip label), checkbox:first-child {
@@ -180,9 +263,25 @@ zoterosearch {
position: absolute;
}
- .valuefield, .valuemenu, .value-date-age {
+ .valuemenu, .value-date-age {
flex: 1;
}
+
+ // The text value field grows to use the available row width (the overall cap is on
+ // the builder, not here). It's a flex container itself so the inner stack/input
+ // fills the host.
+ .valuefield {
+ display: flex;
+ flex: 1;
+ min-width: 12em;
+
+ // The `display: flex` above overrides the `hidden` attribute's display:none, so
+ // re-assert it -- otherwise the textbox shows alongside the value menu for
+ // menu-based conditions (Collection, Item Type, Attachment File Type)
+ &[hidden] {
+ display: none;
+ }
+ }
}
.join-mode-menu {
@@ -193,9 +292,15 @@ zoterosearch {
width: 15em;
}
- #operatorsmenu, #valuemenu, #valuefield, .search-in-the-last {
+ #operatorsmenu, #valuemenu, .search-in-the-last {
width: 12em;
}
+
+ // Fill the flexed value field (host -> stack -> input)
+ #search-textbox {
+ flex: 1;
+ width: 100%;
+ }
#valuemenu::part(icon) {
max-height: 16px;
diff --git a/scss/themes/_dark.scss b/scss/themes/_dark.scss
index 6a95c28e8f..6d8336ccd1 100644
--- a/scss/themes/_dark.scss
+++ b/scss/themes/_dark.scss
@@ -8,6 +8,7 @@ $-colors: (
accent-green: #39bf68d9,
accent-orange: #ff794cd9,
accent-red: #db2c3ae5,
+ accent-red10: #db2c3a4d,
accent-teal: #59adc4e5,
accent-white: #fff,
accent-wood-dark: #996b6f,
diff --git a/scss/themes/_light.scss b/scss/themes/_light.scss
index ce64579b64..db7d5e062d 100644
--- a/scss/themes/_light.scss
+++ b/scss/themes/_light.scss
@@ -8,6 +8,7 @@ $-colors: (
accent-green: #39bf68,
accent-orange: #ff794c,
accent-red: #db2c3a,
+ accent-red10: #db2c3a1a,
accent-teal: #59adc4,
accent-white: #fff,
accent-wood-dark: #996b6f,
diff --git a/test/tests/advancedSearchTest.js b/test/tests/advancedSearchTest.js
index 2a2c17d412..8a9bdf20d2 100644
--- a/test/tests/advancedSearchTest.js
+++ b/test/tests/advancedSearchTest.js
@@ -52,6 +52,56 @@ describe("Advanced Search", function () {
await otherItem.eraseTx();
});
+
+ it("should run a cross-level search across a multi-collection selection", async function () {
+ var word = 'zmc' + Zotero.Utilities.randomString();
+ var makeMatch = async function (collection) {
+ var item = await createDataObject('item', { collections: [collection.id] });
+ var attachment = await importPDFAttachment(item);
+ await createAnnotation('highlight', attachment, { comment: 'foo ' + word + ' bar' });
+ return item;
+ };
+ var c1 = await createDataObject('collection');
+ var c2 = await createDataObject('collection');
+ var c3 = await createDataObject('collection');
+ var itemA = await makeMatch(c1);
+ var itemB = await makeMatch(c2);
+ var itemC = await makeMatch(c3); // in an unselected collection
+
+ // Select c1 and c2, leaving c3 out
+ var cv = zp.collectionsView;
+ await cv.selectByID("C" + c1.id);
+ await waitForItemsLoad(win);
+ cv.selection.toggleSelect(cv.getRowIndexByID("C" + c2.id));
+ await zp.onCollectionSelected();
+ await zp.itemsView.waitForLoad();
+
+ // Top-level items with a descendant annotation matching the comment
+ var s = new Zotero.Search();
+ s.libraryID = Zotero.Libraries.userLibraryID;
+ s.addCondition('resultLevel', 'item');
+ s.addCondition('annotationComment', 'contains', word);
+
+ var iv = zp.itemsView;
+ await iv.setFilter('advanced-search', s);
+ await iv.waitForLoad();
+
+ // The view merges getItems() across the selected rows (collectionViewItemTree),
+ // so the cross-level search runs scoped to each collection and the results union:
+ // both selected collections' matching items, but not the unselected one's
+ var rows = zp.getCollectionTreeRows();
+ var ids = new Set();
+ for (let arr of await Promise.all(rows.map(row => row.getItems()))) {
+ for (let item of arr) ids.add(item.id);
+ }
+ assert.sameMembers([...ids], [itemA.id, itemB.id]);
+
+ await iv.setFilter('advanced-search', null);
+ await selectLibrary(win);
+ await Zotero.Items.erase([itemA.id, itemB.id, itemC.id]);
+ await Zotero.Collections.erase([c1.id, c2.id, c3.id]);
+ });
+
it("shouldn't show trashed items outside the trash", async function () {
var item = await createDataObject('item', { setTitle: true });
item.deleted = true;
@@ -486,6 +536,23 @@ describe("Advanced Search", function () {
// Focus moves to the new condition's drop-down
assert.equal(win.document.activeElement, newRow.querySelector('#conditionsmenu'));
});
+ it("should hide the value textbox for a menu-based condition", async function () {
+ var s = new Zotero.Search();
+ s.libraryID = Zotero.Libraries.userLibraryID;
+ s.addCondition('title', 'is', '');
+ pane.search = s;
+
+ var row = conditions.firstChild;
+ row.onConditionSelected('fileTypeID');
+
+ // File Type uses a value menu, so the textbox must actually be hidden, not just
+ // sitting alongside the menu
+ var valuefield = row.querySelector('#valuefield');
+ assert.isTrue(valuefield.hidden);
+ assert.equal(win.getComputedStyle(valuefield).display, 'none');
+ assert.isFalse(row.querySelector('#valuemenu').hidden);
+ });
+
describe("Find-as-you-type", function () {
function typeInMenu(menu, str) {
@@ -989,6 +1056,295 @@ describe("Advanced Search", function () {
assert.equal(conditions.firstChild.querySelector('#valuefield').value, '');
});
+ it("should default a fresh search to the top-level item result level", function () {
+ // Opening a new (unseeded) advanced search defaults the result level to
+ // top-level items, so child conditions map up without grouping
+ pane.search = null;
+ assert.equal(searchBox.rootGroup.resultLevel, 'item');
+ });
+
+ it("should render and serialize the root result level", function () {
+ var s = new Zotero.Search();
+ s.libraryID = Zotero.Libraries.userLibraryID;
+ s.addCondition('resultLevel', 'annotation');
+ s.addCondition('annotationText', 'contains', 'foo');
+ pane.search = s;
+
+ assert.equal(searchBox.rootGroup.resultLevel, 'annotation');
+
+ searchBox.updateSearch();
+ var sequence = Object.values(searchBox.search.getConditions())
+ .map(c => c.condition);
+ assert.deepEqual(sequence, ['resultLevel', 'annotationText']);
+ var scopeCond = Object.values(searchBox.search.getConditions())
+ .find(c => c.condition == 'resultLevel');
+ assert.equal(scopeCond.operator, 'annotation');
+ });
+
+ it("should render and serialize a nested group result level", function () {
+ var s = new Zotero.Search();
+ s.libraryID = Zotero.Libraries.userLibraryID;
+ s.addCondition('creator', 'contains', 'Smith');
+ s.addCondition('groupStart', 'true', '');
+ s.addCondition('resultLevel', 'annotation');
+ s.addCondition('annotationText', 'contains', 'foo');
+ s.addCondition('groupEnd', 'true', '');
+ pane.search = s;
+
+ var group = conditions.querySelector('search-condition-group');
+ assert.equal(group.resultLevel, 'annotation');
+
+ searchBox.updateSearch();
+ var sequence = Object.values(searchBox.search.getConditions())
+ .map(c => c.condition);
+ assert.deepEqual(sequence,
+ ['creator', 'groupStart', 'resultLevel', 'annotationText', 'groupEnd']);
+ });
+
+ it("should show a binding menu for a group of same-level descendant conditions", function () {
+ var s = new Zotero.Search();
+ s.libraryID = Zotero.Libraries.userLibraryID;
+ s.addCondition('resultLevel', 'item'); // result level
+ s.addCondition('groupStart', 'true', '');
+ s.addCondition('annotationText', 'contains', 'foo');
+ s.addCondition('annotationComment', 'contains', 'bar');
+ s.addCondition('groupEnd', 'true', '');
+ pane.search = s;
+
+ var group = conditions.querySelector('search-condition-group');
+ assert.isFalse(group.bindingMenu.hidden);
+ var values = [...group.bindingMenu.querySelectorAll('menuitem')].map(i => i.value);
+ assert.includeMembers(values, ['any', 'annotation']);
+ // No attachment conditions, so it isn't offered
+ assert.notInclude(values, 'attachment');
+
+ // Bind to the same annotation and confirm it serializes
+ group.resultLevel = 'annotation';
+ searchBox.updateSearch();
+ var seq = Object.values(searchBox.search.getConditions()).map(c => c.condition);
+ assert.deepEqual(seq,
+ ['resultLevel', 'groupStart', 'resultLevel', 'annotationText', 'annotationComment', 'groupEnd']);
+ var groupScope = Object.values(searchBox.search.getConditions())
+ .filter(c => c.condition == 'resultLevel')[1];
+ assert.equal(groupScope.operator, 'annotation');
+ });
+
+ it("should not show a binding menu for a group of item-level conditions", function () {
+ var s = new Zotero.Search();
+ s.libraryID = Zotero.Libraries.userLibraryID;
+ s.addCondition('resultLevel', 'item');
+ s.addCondition('groupStart', 'true', '');
+ s.addCondition('title', 'contains', 'a');
+ s.addCondition('title', 'contains', 'b');
+ s.addCondition('groupEnd', 'true', '');
+ pane.search = s;
+
+ var group = conditions.querySelector('search-condition-group');
+ assert.isTrue(group.bindingMenu.hidden);
+ });
+
+ it("should render a stored group binding into the menu", function () {
+ var s = new Zotero.Search();
+ s.libraryID = Zotero.Libraries.userLibraryID;
+ s.addCondition('resultLevel', 'item');
+ s.addCondition('groupStart', 'true', '');
+ s.addCondition('resultLevel', 'annotation');
+ s.addCondition('annotationText', 'contains', 'foo');
+ s.addCondition('annotationComment', 'contains', 'bar');
+ s.addCondition('groupEnd', 'true', '');
+ pane.search = s;
+
+ var group = conditions.querySelector('search-condition-group');
+ assert.isFalse(group.bindingMenu.hidden);
+ assert.equal(group.resultLevel, 'annotation');
+ assert.equal(group.bindingMenu.value, 'annotation');
+ });
+
+ it("should offer a binding hint for ungrouped sibling descendant conditions", function () {
+ var s = new Zotero.Search();
+ s.libraryID = Zotero.Libraries.userLibraryID;
+ s.addCondition('resultLevel', 'item');
+ s.addCondition('annotationText', 'contains', 'foo');
+ s.addCondition('annotationComment', 'contains', 'bar');
+ pane.search = s;
+
+ var hint = searchBox.querySelector('#search-binding-hint');
+ assert.isFalse(hint.hidden);
+ // One bindable level (annotation), so one suggestion line with one button
+ assert.lengthOf([...hint.querySelectorAll('button')], 1);
+ });
+
+ it("should not offer a binding hint for item-level sibling conditions", function () {
+ var s = new Zotero.Search();
+ s.libraryID = Zotero.Libraries.userLibraryID;
+ s.addCondition('resultLevel', 'item');
+ s.addCondition('title', 'contains', 'a');
+ s.addCondition('title', 'contains', 'b');
+ pane.search = s;
+
+ assert.isTrue(searchBox.querySelector('#search-binding-hint').hidden);
+ });
+
+ it("should not offer a binding hint for an unpopulated condition", function () {
+ // One populated annotation condition plus an empty one: not enough to suggest
+ // binding until the second is actually filled in
+ var s = new Zotero.Search();
+ s.libraryID = Zotero.Libraries.userLibraryID;
+ s.addCondition('resultLevel', 'item');
+ s.addCondition('annotationText', 'contains', 'foo');
+ s.addCondition('annotationComment', 'contains', '');
+ pane.search = s;
+
+ assert.isTrue(searchBox.querySelector('#search-binding-hint').hidden);
+ });
+
+ it("should wrap conditions into a bound group when the hint is taken", function () {
+ var s = new Zotero.Search();
+ s.libraryID = Zotero.Libraries.userLibraryID;
+ s.addCondition('resultLevel', 'item');
+ s.addCondition('annotationText', 'contains', 'foo');
+ s.addCondition('annotationComment', 'contains', 'bar');
+ pane.search = s;
+
+ searchBox.rootGroup.bindSameEntity('annotation');
+
+ // The two conditions are now one group bound to annotation
+ var group = conditions.querySelector('search-condition-group');
+ assert.ok(group);
+ assert.equal(group.resultLevel, 'annotation');
+ assert.lengthOf(
+ [...group.conditionsContainer.children].filter(c => c.localName == 'zoterosearchcondition'),
+ 2);
+
+ searchBox.updateSearch();
+ var seq = Object.values(searchBox.search.getConditions()).map(c => c.condition);
+ assert.deepEqual(seq,
+ ['resultLevel', 'groupStart', 'resultLevel', 'annotationText', 'annotationComment', 'groupEnd']);
+
+ // Conditions are no longer ungrouped siblings, so the hint is gone
+ assert.isTrue(searchBox.querySelector('#search-binding-hint').hidden);
+ });
+
+ it("should fold a legacy noChildren into the result level", function () {
+ var s = new Zotero.Search();
+ s.libraryID = Zotero.Libraries.userLibraryID;
+ s.addCondition('noChildren', 'true');
+ s.addCondition('title', 'contains', 'foo');
+ pane.search = s;
+
+ // Shown as result level = top-level items, no separate checkbox
+ assert.equal(searchBox.rootGroup.resultLevel, 'item');
+
+ searchBox.updateSearch();
+ var seq = Object.values(searchBox.search.getConditions()).map(c => c.condition);
+ assert.notInclude(seq, 'noChildren');
+ assert.deepEqual(seq, ['resultLevel', 'title']);
+ });
+
+
+ it("should warn when ALL conditions can't match the same item at a mixed result level", function () {
+ // No result type (mixed), so an item-level and an annotation-level condition
+ // can't both be true of one row
+ var s = new Zotero.Search();
+ s.libraryID = Zotero.Libraries.userLibraryID;
+ s.addCondition('title', 'contains', 'a');
+ s.addCondition('annotationText', 'contains', 'b');
+ pane.search = s;
+
+ assert.isFalse(searchBox.querySelector('.level-warning').hidden);
+ });
+
+ it("should not warn when a result type lets the conditions combine", function () {
+ // Result type item: the annotation condition maps up, so it's satisfiable
+ var s = new Zotero.Search();
+ s.libraryID = Zotero.Libraries.userLibraryID;
+ s.addCondition('resultLevel', 'item');
+ s.addCondition('title', 'contains', 'a');
+ s.addCondition('annotationText', 'contains', 'b');
+ pane.search = s;
+
+ assert.isTrue(searchBox.querySelector('.level-warning').hidden);
+ });
+
+ it("should warn when a condition can't reach the result type", function () {
+ // A note can never be (or be under) an attachment
+ var s = new Zotero.Search();
+ s.libraryID = Zotero.Libraries.userLibraryID;
+ s.addCondition('resultLevel', 'attachment');
+ s.addCondition('note', 'contains', 'a');
+ pane.search = s;
+
+ assert.isFalse(searchBox.querySelector('.level-warning').hidden);
+ });
+
+ it("should warn on the group when a condition can't reach its binding", function () {
+ // A group bound to "same annotation" with a note condition: the conflict is the
+ // group's binding, so the warning belongs on the group, not at the root.
+ var s = new Zotero.Search();
+ s.libraryID = Zotero.Libraries.userLibraryID;
+ s.addCondition('resultLevel', 'item');
+ s.addCondition('groupStart', 'true', '');
+ s.addCondition('resultLevel', 'annotation');
+ s.addCondition('annotationComment', 'contains', 'a');
+ s.addCondition('note', 'contains', 'b');
+ s.addCondition('groupEnd', 'true', '');
+ pane.search = s;
+
+ var group = conditions.querySelector('search-condition-group');
+ assert.isFalse(group.levelWarning.hidden);
+ assert.isTrue(searchBox.rootGroup.levelWarning.hidden);
+ });
+
+ it("should not warn for a \"separately\" group whose conditions roll up", function () {
+ // "Separately" inherits the result level (item), so annotations and a note all
+ // roll up independently -- satisfiable, no warning (so "match separately" really
+ // does clear the bound-group warning)
+ var s = new Zotero.Search();
+ s.libraryID = Zotero.Libraries.userLibraryID;
+ s.addCondition('resultLevel', 'item');
+ s.addCondition('groupStart', 'true', '');
+ s.addCondition('annotationComment', 'contains', 'a');
+ s.addCondition('annotationComment', 'contains', 'b');
+ s.addCondition('note', 'contains', 'c');
+ s.addCondition('groupEnd', 'true', '');
+ pane.search = s;
+
+ assert.isTrue(conditions.querySelector('search-condition-group').levelWarning.hidden);
+ });
+
+ it("should not warn for an ordinary search", function () {
+ pane.search = null;
+ assert.isTrue(searchBox.querySelector('.level-warning').hidden);
+ });
+
+ it("should keep a legacy includeParentsAndChildren editable and round-tripping", function () {
+ var s = new Zotero.Search();
+ s.libraryID = Zotero.Libraries.userLibraryID;
+ s.addCondition('title', 'contains', 'foo');
+ s.addCondition('includeParentsAndChildren', 'true');
+ pane.search = s;
+
+ assert.isFalse(searchBox.querySelector('#search-legacy-options').hidden);
+ assert.isTrue(searchBox.querySelector('#includeParentsAndChildrenCheckbox').checked);
+
+ searchBox.updateSearch();
+ var seq = Object.values(searchBox.search.getConditions()).map(c => c.condition);
+ assert.include(seq, 'includeParentsAndChildren');
+ });
+
+ it("should drop includeParentsAndChildren when its legacy checkbox is unchecked", function () {
+ var s = new Zotero.Search();
+ s.libraryID = Zotero.Libraries.userLibraryID;
+ s.addCondition('title', 'contains', 'foo');
+ s.addCondition('includeParentsAndChildren', 'true');
+ pane.search = s;
+
+ searchBox.querySelector('#includeParentsAndChildrenCheckbox').checked = false;
+ searchBox.updateSearch();
+ var seq = Object.values(searchBox.search.getConditions()).map(c => c.condition);
+ assert.notInclude(seq, 'includeParentsAndChildren');
+ });
+
it("should add a sibling condition outside the group from the group's +", function () {
var s = new Zotero.Search();
s.libraryID = Zotero.Libraries.userLibraryID;