mirror of
https://github.com/zotero/zotero.git
synced 2026-09-14 23:21:03 +00:00
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 5697ee0af7)
This commit is contained in:
parent
1347e2642b
commit
d62b044386
4 changed files with 148 additions and 26 deletions
|
|
@ -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) {
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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());
|
||||
});
|
||||
});
|
||||
})
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue