Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix compatibility with Violentmonkey #64

Closed
wants to merge 1 commit into from

Conversation

artillect
Copy link
Contributor

I removed the (apparently nonfunctional) await on line 133, and replaced all of the GM. functions with GM_ functions, as violentmonkey doesn't have those built in

Fixes #63

@aclist
Copy link
Owner

aclist commented Jul 12, 2023

Violentmonkey seems to be strict about that; I was picking through it and able to resolve that part by wrapping it in an async function:

async function getJson(){
   let json = await GM_getValue("json");
  return json
}
const j = getJson();
j.then((result) => {
  //process result here
})

But applying your fix above seems to clear the first issue, but I'm getting issues with getModSettings() is not defined. Are you seeing that?

@aclist
Copy link
Owner

aclist commented Jul 12, 2023

To clarify my comment, this allows the JSON to be processed, but I don't think any of the options function because getModSettings() is failing

@aclist
Copy link
Owner

aclist commented Jul 12, 2023

Seems to occur when any of the mods calls getModSettings() from its scope, as if they aren't operating in the same scope as the main script. Could be a violent monkey setting on my side?

@artillect
Copy link
Contributor Author

Hmm, I'm not seeing any settings that'd affect that unless I'm missing something

@aclist
Copy link
Owner

aclist commented Jul 12, 2023

Have a look at this:

violentmonkey/violentmonkey#173
violentmonkey/violentmonkey#408
https://violentmonkey.github.io/posts/inject-into-context/

I tried violentmonkey's various injection modes, but none of them worked unless explicitly doing this:

OLD:

function getModSettings()
//...

NEW:

unsafeWindow.getModSettings = function(namespace){
//...

Second method works, all settings load. However, I don't remember all of the implications of this, so don't think it's a good idea to include this in the normal script. Nor did I test every interaction of elements. I think getModSettings() is the only function from the main context that the mods call. We don't have any other shared functions.

To better identify the problem surface: does this only affect VM + Firefox? Does the same script in VM + e.g. Chrome work when using VM's "auto" page injection context? (Can you test that?)

It seems related to Firefox's stricter CSP policy.

@artillect
Copy link
Contributor Author

I get the same issue in Chrome with VM.

Is there a way to pass the function to the mods so they have access to it in their namespace? This is a bit beyond my JS knowledge

@aclist
Copy link
Owner

aclist commented Jul 12, 2023

I think this should fix it and is a fairly vanilla approach:

window.getModSettings = function(namespace){
  let settings = localStorage.getItem(namespace);
    if (!settings) {
        settings = {};
    } else {
        settings = JSON.parse(settings);
    }
    return settings;
}

Should bind it to the global window context and allow this one function to be seen by the scope of the others.

I think the "unsafewindow" is a bit of a misnomer in that it's widely supported and modern browser should already be isolating injected JS from self JS. I think it used to be the case that pages could "see" a third party script this way, but that should be ancient history by now sandboxing is already taking place.

Some additional discussion here: https://stackoverflow.com/questions/65552070/why-do-userscript-managers-still-suport-the-use-of-unsafewindow

@aclist
Copy link
Owner

aclist commented Jul 12, 2023

I have pushed the fix, please have a look.

Also, looks like your collapsed comments mod is calling GM.addStyle instead of GM_addStyle, and this is causing VM to exhibit problems. Would you mind taking a crack at that and pushing a new PR?

@artillect
Copy link
Contributor Author

I'll fix that real quick, we should add a note in the documentation to only use GM_ functions for better compatibility across the *monkey family

@artillect
Copy link
Contributor Author

Looks like your fix is working

@aclist
Copy link
Owner

aclist commented Jul 12, 2023

I'll fix that real quick, we should add a note in the documentation to only use GM_ functions for better compatibility across the *monkey family

That makes sense, probably suggest TM over VM and GM just because it's the most feature complete and mature at this point, but that's a great idea, will look into adding it to a testing suite that will prevent these kinds of things from inadvertently making their way in.

@aclist aclist closed this Jul 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants