From d62b044386fa377090dfec0a77de961aa88b5010 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Wed, 2 Sep 2026 17:42:52 -0400 Subject: [PATCH] Shim removed selection methods when a single row is selected Plugins written for earlier versions call methods like CollectionTree#getSelectedSearch(), which now throw. We did this intentionally to make old code more obviously broken, even with single selection, so developers would fix their code, but we're not yet blocking plugins that illegitimately declared compatibility with a future version, so some haven't been updated and are breaking Zotero. With a single row selected, the pre-10 functions can technically continue to work, so for now, restore them and just warn and name the plugin instead of throwing. They still need to throw if multiple rows are selected, since plugins that haven't been updated can't safely act on a selection that might span collections or libraries. https://forums.zotero.org/discussion/133565/ (cherry picked from commit 5697ee0af77ae221ea34c9f6c48137cae5c2962a) --- chrome/content/zotero/collectionTree.jsx | 49 ++++++++++++++++----- chrome/content/zotero/xpcom/plugins.js | 44 +++++++++++++++++++ chrome/content/zotero/zoteroPane.js | 54 +++++++++++++++++------- test/tests/collectionTreeTest.js | 27 ++++++++++++ 4 files changed, 148 insertions(+), 26 deletions(-) diff --git a/chrome/content/zotero/collectionTree.jsx b/chrome/content/zotero/collectionTree.jsx index 8bb8471baf..2901e08bf2 100644 --- a/chrome/content/zotero/collectionTree.jsx +++ b/chrome/content/zotero/collectionTree.jsx @@ -1407,9 +1407,28 @@ var CollectionTree = class CollectionTree extends LibraryTree { return this.getRow(index).getName(); } + /** + * Throw for a plugin calling one of the removed single-selection methods while multiple + * rows are selected, where the items list can span collections or libraries and acting + * on one row's value isn't safe + * + * Also used by the ZoteroPane methods that wrap these, which pass their own names. + * + * @param {String} name + * @param {String} replacement + */ + _requireSingleSelection(name, replacement) { + if (this.selection.count > 1) { + throw new Error(`${name} was removed -- use ${replacement}`); + } + Zotero.Plugins.warnRemovedAPICall(name, replacement); + } + getSelectedLibraryID() { - throw new Error("CollectionTree#getSelectedLibraryID() was removed " - + "-- use getSelectedLibraryIDs()"); + this._requireSingleSelection("CollectionTree#getSelectedLibraryID()", + "getSelectedLibraryIDs()"); + var libraryIDs = this.getSelectedLibraryIDs(); + return libraryIDs.length ? libraryIDs[0] : false; } /** @@ -1429,9 +1448,10 @@ var CollectionTree = class CollectionTree extends LibraryTree { return libraryIDs; } - getSelectedCollection() { - throw new Error("CollectionTree#getSelectedCollection() was removed " - + "-- use getSelectedCollections()"); + getSelectedCollection(asID) { + this._requireSingleSelection("CollectionTree#getSelectedCollection()", + "getSelectedCollections()"); + return this.getSelectedCollections(asID)[0]; } getSelectedCollections(asID) { @@ -1445,9 +1465,11 @@ var CollectionTree = class CollectionTree extends LibraryTree { return collections; } - getSelectedSearch() { - throw new Error("CollectionTree#getSelectedSearch() was removed " - + "-- use getSelectedSearches()"); + getSelectedSearch(asID) { + this._requireSingleSelection("CollectionTree#getSelectedSearch()", + "getSelectedSearches()"); + var searches = this.getSelectedSearches(asID); + return searches.length ? searches[0] : false; } getSelectedSearches(asID) { @@ -1461,9 +1483,14 @@ var CollectionTree = class CollectionTree extends LibraryTree { return searches; } - getSelectedGroup() { - throw new Error("CollectionTree#getSelectedGroup() was removed " - + "-- filter getSelectedRows() by isGroup()"); + getSelectedGroup(asID) { + this._requireSingleSelection("CollectionTree#getSelectedGroup()", + "getSelectedRows() filtered by isGroup()"); + var group = this.getSelectedRows().find(row => row.isGroup()); + if (group) { + return asID ? group.ref.id : group.ref; + } + return false; } getIconName(index) { diff --git a/chrome/content/zotero/xpcom/plugins.js b/chrome/content/zotero/xpcom/plugins.js index 1e536dd495..088d5e99af 100644 --- a/chrome/content/zotero/xpcom/plugins.js +++ b/chrome/content/zotero/xpcom/plugins.js @@ -379,6 +379,35 @@ Zotero.Plugins = new function () { }; + var warnedRemovedAPICalls = new Set(); + + /** + * Warn about a call to an API that was removed, naming the plugin that called it + * + * Resolving the plugin is async, so the warning is logged after this returns. + * + * @param {String} name - The removed API, e.g. "CollectionTree#getSelectedSearch()" + * @param {String} replacement - What to use instead, e.g. "getSelectedSearches()" + */ + this.warnRemovedAPICall = function (name, replacement) { + let stack = new Error().stack; + // Warn once per call site, since a plugin may call these repeatedly (e.g., on every + // item list load). The frames above the caller are this method and the removed + // method itself, which are the same for every caller. + let callerFrame = stack.split("\n").find(line => !_isZoteroStackFrame(line)); + let key = name + "\n" + (callerFrame || ""); + if (warnedRemovedAPICalls.has(key)) { + return; + } + warnedRemovedAPICalls.add(key); + this.getPluginFromError(stack) + .then((plugin) => { + let caller = plugin ? `${plugin.name} (${plugin.id})` : "Something"; + Zotero.warn(`${caller} called ${name}, which was removed -- use ${replacement}`); + }) + .catch(e => Zotero.logError(e)); + }; + /** * Resolve a chrome:// or resource:// URI to the file or JAR URI it points at, since @@ -425,6 +454,21 @@ Zotero.Plugins = new function () { } + /** + * @param {String} line - A line from an error stack + * @return {Boolean} - True if the frame is Zotero's own code + */ + function _isZoteroStackFrame(line) { + let uri = _stackFrameURI(line); + // Trailing slashes matter: a plugin package can start with our own name, as + // chrome://zoterostyle/ does + return !uri + || uri.startsWith("chrome://zotero/") + || uri.startsWith("resource://zotero/") + || uri.startsWith("resource://gre/"); + } + + /** * @param {String} line - A line from an error stack, "name@uri:line:column" * @return {String} - The URI, without the line and column diff --git a/chrome/content/zotero/zoteroPane.js b/chrome/content/zotero/zoteroPane.js index 4b21a3adba..7961df632f 100644 --- a/chrome/content/zotero/zoteroPane.js +++ b/chrome/content/zotero/zoteroPane.js @@ -2192,8 +2192,12 @@ var ZoteroPane = new function () { this.getCollectionTreeRow = function () { - throw new Error("ZoteroPane.getCollectionTreeRow() was removed " - + "-- use ZoteroPane.getCollectionTreeRows()"); + if (!this.collectionsView) { + return false; + } + this.collectionsView._requireSingleSelection("ZoteroPane.getCollectionTreeRow()", + "ZoteroPane.getCollectionTreeRows()"); + return this.getCollectionTreeRows()[0] || false; } @@ -3479,8 +3483,10 @@ var ZoteroPane = new function () { }; this.getSelectedLibraryID = function () { - throw new Error("ZoteroPane.getSelectedLibraryID() was removed " - + "-- use ZoteroPane.getSelectedLibraryIDs()"); + this.collectionsView._requireSingleSelection("ZoteroPane.getSelectedLibraryID()", + "ZoteroPane.getSelectedLibraryIDs()"); + var libraryIDs = this.getSelectedLibraryIDs(); + return libraryIDs.length ? libraryIDs[0] : false; } @@ -3495,9 +3501,10 @@ var ZoteroPane = new function () { } - this.getSelectedCollection = function () { - throw new Error("ZoteroPane.getSelectedCollection() was removed " - + "-- use ZoteroPane.getSelectedCollections()"); + this.getSelectedCollection = function (asID) { + this.collectionsView._requireSingleSelection("ZoteroPane.getSelectedCollection()", + "ZoteroPane.getSelectedCollections()"); + return this.getSelectedCollections(asID)[0]; } @@ -3506,9 +3513,11 @@ var ZoteroPane = new function () { } - function getSelectedSavedSearch() { - throw new Error("ZoteroPane.getSelectedSavedSearch() was removed " - + "-- use ZoteroPane.getSelectedSavedSearches()"); + function getSelectedSavedSearch(asID) { + this.collectionsView._requireSingleSelection("ZoteroPane.getSelectedSavedSearch()", + "ZoteroPane.getSelectedSavedSearches()"); + var searches = this.getSelectedSavedSearches(asID); + return searches.length ? searches[0] : false; } @@ -3517,9 +3526,14 @@ var ZoteroPane = new function () { } - this.getSelectedGroup = function () { - throw new Error("ZoteroPane.getSelectedGroup() was removed -- filter " - + "ZoteroPane.getCollectionTreeRows() by isGroup()"); + this.getSelectedGroup = function (asID) { + this.collectionsView._requireSingleSelection("ZoteroPane.getSelectedGroup()", + "ZoteroPane.getCollectionTreeRows() filtered by isGroup()"); + var group = this.getCollectionTreeRows().find(row => row.isGroup()); + if (group) { + return asID ? group.ref.id : group.ref; + } + return false; } @@ -4123,7 +4137,12 @@ var ZoteroPane = new function () { { getContext: () => ({ get collectionTreeRow() { - throw new Error("collectionTreeRow was removed -- use collectionTreeRows"); + if (collectionTreeRows.length > 1) { + throw new Error("collectionTreeRow was removed -- use collectionTreeRows"); + } + Zotero.Plugins.warnRemovedAPICall("Menu context collectionTreeRow", + "collectionTreeRows"); + return collectionTreeRows[0]; }, collectionTreeRows, tabType: "library", @@ -4669,7 +4688,12 @@ var ZoteroPane = new function () { { getContext: () => ({ get collectionTreeRow() { - throw new Error("collectionTreeRow was removed -- use collectionTreeRows"); + if (collectionTreeRows.length > 1) { + throw new Error("collectionTreeRow was removed -- use collectionTreeRows"); + } + Zotero.Plugins.warnRemovedAPICall("Menu context collectionTreeRow", + "collectionTreeRows"); + return collectionTreeRows[0]; }, collectionTreeRows, items, diff --git a/test/tests/collectionTreeTest.js b/test/tests/collectionTreeTest.js index ebad266eef..8560547660 100644 --- a/test/tests/collectionTreeTest.js +++ b/test/tests/collectionTreeTest.js @@ -2173,4 +2173,31 @@ describe("Zotero.CollectionTree", function () { invalidateSpy.restore(); }); }) + + describe("removed single-selection methods", function () { + it("should return the selected row when focus is on a different row", async function () { + var collection1 = await createDataObject('collection'); + var collection2 = await createDataObject('collection'); + var index1 = cv.getRowIndexByID(collection1.treeViewID); + var index2 = cv.getRowIndexByID(collection2.treeViewID); + + cv.selection.select(index1); + // Ctrl/Cmd-arrow moves focus without changing the selection + cv.selection.focused = index2; + + assert.equal(cv.getSelectedCollection(), collection1); + assert.equal(zp.getSelectedCollection(), collection1); + assert.equal(zp.getCollectionTreeRow().ref, collection1); + }); + + it("should throw when multiple rows are selected", async function () { + var collection1 = await createDataObject('collection'); + var collection2 = await createDataObject('collection'); + cv.selection.select(cv.getRowIndexByID(collection1.treeViewID)); + cv.selection.toggleSelect(cv.getRowIndexByID(collection2.treeViewID)); + + assert.throws(() => cv.getSelectedCollection()); + assert.throws(() => zp.getSelectedLibraryID()); + }); + }); })