From 31d9e89165757c4739a9024a4f8cba9077659721 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Mon, 8 Jun 2026 12:55:13 -0400 Subject: [PATCH] Fix item tree focus checks broken by view-specific tree IDs The item tree's DOM id carries a view-specific suffix (e.g. "item-tree-main-default", "item-tree-main-recentlyRead"), but three call sites compared against a single hardcoded "item-tree-main": - Collection highlighting on Ctrl/Option (zoteroPane.js) -- match on the "item-tree-main" prefix to cover all views. This restores highlighting in Recently Read, where it silently failed. - Focusing the items list after Add Item by Identifier (lookup.js) -- use the current view's tree id instead of a literal that resolved to null and threw. - Shift-Tab from the item tree to the toolbar (zoteroPane.js) -- key the actionsMap on the current view's tree id. Add a test confirming focus lands on the items list after a lookup. https://forums.zotero.org/discussion/130968/collection-of-selected-papers-is-not-highlighted-in-recently-read-panel --- chrome/content/zotero/lookup.js | 3 ++- chrome/content/zotero/zoteroPane.js | 7 +++++-- test/tests/lookupTest.js | 16 ++++++++++++++++ 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/chrome/content/zotero/lookup.js b/chrome/content/zotero/lookup.js index 6abc11f2ba..cd7cbbcb72 100644 --- a/chrome/content/zotero/lookup.js +++ b/chrome/content/zotero/lookup.js @@ -147,7 +147,8 @@ var Zotero_Lookup = new function () { // Send the focus to the item tree after the popup closes ZoteroPane.lastFocusedElement = null; document.getElementById("zotero-lookup-panel").hidePopup(); - document.getElementById("item-tree-main-default").focus(); + // The item tree's DOM id has a view-specific suffix, so use the current view's id + document.getElementById(ZoteroPane.itemsView.id).focus(); } return false; }; diff --git a/chrome/content/zotero/zoteroPane.js b/chrome/content/zotero/zoteroPane.js index 5a48549b52..7ec522dac0 100644 --- a/chrome/content/zotero/zoteroPane.js +++ b/chrome/content/zotero/zoteroPane.js @@ -438,7 +438,8 @@ var ZoteroPane = new function () { itemTree.addEventListener("keydown", (event) => { let actionsMap = { - 'item-tree-main-default': { + // The item tree's DOM id has a view-specific suffix, so key on the current view's id + [ZoteroPane.itemsView?.id]: { ShiftTab: () => document.getElementById('zotero-tb-toggle-item-pane-stacked') } }; @@ -1066,7 +1067,9 @@ var ZoteroPane = new function () { else { enableHighlight = !event.shiftKey && !event.metaKey && event.key == "Control" && !event.altKey; } - let isItemTreeFocused = document.activeElement.id == "item-tree-main-default"; + // The item tree's DOM id has a view-specific suffix (e.g. "item-tree-main-default", + // "item-tree-main-recentlyRead"), so match on the prefix to cover all views + let isItemTreeFocused = document.activeElement.id.startsWith("item-tree-main"); // Only highlight collections when itemTree is focused to try to avoid // conflicts with other shortcuts if (enableHighlight && isItemTreeFocused) { diff --git a/test/tests/lookupTest.js b/test/tests/lookupTest.js index 245c0cfe53..da594dfa23 100644 --- a/test/tests/lookupTest.js +++ b/test/tests/lookupTest.js @@ -49,6 +49,22 @@ describe("Add Item by Identifier", function () { }); }); + it("should focus the items list after adding items", async function () { + // Make sure the items list is non-empty (and therefore focusable) + await createDataObject('item'); + await waitForItemsLoad(win); + // Stub the lookup itself so the test doesn't hit external services + var stub = sinon.stub(win.Zotero_Lookup, "addItemsFromIdentifier").resolves([{}]); + try { + var textbox = win.document.getElementById("zotero-lookup-textbox"); + await win.Zotero_Lookup.accept(textbox); + assert.equal(win.document.activeElement.id, win.ZoteroPane.itemsView.id); + } + finally { + stub.restore(); + } + }); + it.skip("should add a DOI with an open-access PDF"); // e.g., arXiv