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()); + }); + }); })