userscripts
userscripts copied to clipboard
Only change `@inject-into` if the `@grant` value is valid?
Take the following userscript for example:
// ==UserScript==
// @name NewScript-fg00n069
// @description This is your new file, start writing code
// @match *://*/*
// @grant foo
// @inject-into auto
// ==/UserScript==
console.log("bar")
The @grant value is invalid, but because it has an @grant, the @inject-into will be set to content. It might make sense to check *if the @grant value is validbefore changing the@inject-into`.
related #265
Also only use grant in GM.info.script object - currently the keys grant and grants both appear.
Maybe restricting to content script context only when granting supported high-level APIs, it sounds like that would work.
After all, except for the supported APIs, It's just an undefined function name, it doesn't have any effect, we don't have to deal with it.
Personally, I think a much better solution would be to change the context and display warning message that the requested @grant was not provided. From my experience, this type of logic where "A changes to B, but only if C is not D" usually leads to cryptic errors that are hard to debug.
Also semantically, there's a difference between @grant unsafeWindow (valid but not possible in Userscripts) and @grant foo (does not exist anywhere).
Another case -- what would happen if valid and invalid @grants are requested?
@scholtzm
Personally, I think a much better solution would be to change the context and display warning message that the requested @grant was not provided.
Are you suggesting we expand the current warning to include which @grants caused the context change?
Currently, the warning is a bit ambiguous, example:
Somefile.js @inject-into value changed due to @grant values
From my experience, this type of logic where "A changes to B, but only if C is not D" usually leads to cryptic errors that are hard to debug.
This is a valid point, but I'd want to reduce the number of compatibility issues, as noted in #305 - I think it could be a very simple change (more on this below).
Another case -- what would happen if valid and invalid @grants are requested?
This is somewhat related to the quote above as well (ie. reducing the complexity), but the implementation I was thinking of was quite simple.
The current implementation in the beta build is:
const grants = userscript.scriptObject.grants; // <---------
const injectInto = userscript.scriptObject["inject-into"];
// prepare the api string
let api = `const US_uid = "${uid}";\nconst US_filename = "${filename}";`;
// all scripts get access to US_info / GM./GM_info, prepare that object
const scriptData = {
"script": userscript.scriptObject,
"scriptHandler": scriptHandler,
"scriptHandlerVersion": scriptHandlerVersion,
"scriptMetaStr": userscript.scriptMetaStr
};
api += `\nconst US_info = ${JSON.stringify(scriptData)}`;
api += "\nconst GM_info = US_info;";
gmMethods.push("info: US_info");
// if @grant explicitly set to none, empty grants array
if (grants.includes("none")) {
grants.length = 0;
}
// @grant exist for page scoped userscript
if (grants.length && injectInto === "page") {
// remove grants
grants.length = 0;
// provide warning for content script
userscript.warning = `${filename} @grant values changed due to @inject-into value`;
}
// @grant exist for auto scoped userscript
if (grants.length && injectInto === "auto") {
// change scope
userscript.scriptObject["inject-into"] = "content";
// provide warning for content script
userscript.warning = `${filename} @inject-into value changed due to @grant values`;
}
// loop through each @grant for the userscript, add methods as needed
for (let j = 0; j < grants.length; j++) {
// add methods here
}
As you can see above, it's a really basic implementation. We get the @grant values and check if any are present. If so, @grant or @inject-into is manipulated.
To make the change suggested in this issue, we could simply filter out invalid grant values from const grants = userscript.scriptObject.grants; from a set/array of valid grant values, either in this background.js file or on the swift side before the grants are passed to background.js.
That way, when the check is run to determine whether or not to change @grant or @inject-into only valid grant values with be present.
This is a valid point, but I'd want to reduce the number of compatibility issues, as noted in https://github.com/quoid/userscripts/issues/305 - I think it could be a very simple change (more on this below).
Basically, what I was hinting at is a scenario like this:
- script author uses unsupported
@grantvalue or just makes a typo - scope is changed
- completely different error occurs in the script, related to switched context
- no warning about unsupported
@grantis shown in the console
Needless to say, if compatibility is the main concern, the proposed solution makes sense.
done in https://github.com/quoid/userscripts/commit/9c7066f70482cb432247b883a10a7b68d8dad3c6