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
Use programmatic content scripts? #2663
Comments
Assuming I understand it correctly, could be useful in that GM doesn't have to figure out what scripts match on navigation, only when installing / modifying and then have Firefox do the rest. Though, for the topic of |
Even if we can detect the execution time, we have to (async) beam the script content down, which arrives at some non-start time. In theory if we register the script with Firefox, it will "just work"? (I don't want to get in the business of pre-caching things in content scopes in the hopes that it will already be there at start time.) |
https://bugzilla.mozilla.org/show_bug.cgi?id=1332273 was fixed in latest FF59 some hours ago:
So we need an additional API to use this? |
No. I don't think so. They where just modifying the description / summary in order to more clearly label what the bug was about. Documentation is needed though. But looking at the test cases can give an idea on how this is implemented in a WebExtension. As for Greasemonkey, I think this should allow for the removal of the |
I took a quick look at this and it'll be a bit tricky in regard to matches and includes. Greasemonkey takes the 'OR' approach while Mozilla content scripts take an 'AND' approach for matches + includes. This is doable but not very cleanly. Also regex not supported. |
Sxderp, you're awesome ;) Can't really review code, but I've seen you've linked to a manifest.json page. Regarding the match / regex problem I just found the related site. Sounds to me like programmatically can be used for all tabs. Maybe this could include then an additional Regex check for the URL?
|
The method described on that page is what GM currently uses, I'm not sure if regex is considered a major dealbreak or not. I don't use it, sticking with globs, but others might use it. |
@Sxderp You can always just bail out of your script after testing the URL against a regex yourself. Also, can we confirm that this new method won't prevent scripts from running when the page has a CSP not including Also, #2715 is really bothersome. Is it blocked on this or can we just quickly apply the fix described by @arantius in that issue? |
Do you have a testcase? BTW: there's a bug in FF's test to register scripts ( https://bugzilla.mozilla.org/show_bug.cgi?id=1419576 ) |
@kekkc should be one of my branches. Clone, checkout, load temporary add-on. |
@Sxderp pretty cool, it worked with FF59 nightly, no errors in console. I tested a userscript that just alerts sth. and used a website that also just alerts sth. Only one small drawback:
|
It would be nice to keep regex support for existing @include usage. It's the main feature that makes @include useful rather than just having @match, IMO. The most obvious implementation, to me, is to just have such script match all URLs (unless @match is also present) and then do a quick check to see if it matches the Regex, returning early if it doesn't. With the script being available at document-start, I don't think this would be a significant performance degradation. |
Ouch, thought I fixed all the issues regarding unregistered. Though, there are some changes I want to make, so this will probably get wrapped up in those changes.
Correct. Which is also why it isn't a pull request (aside from some changes I want to make). |
While this is possible, just have some wrapper code, it'll also defeat part of the benefit that is gained from using the programmatic content script registration. All page matching is handled by Firefox. If there are no other @match or @include then it's reasonable to do a check for the regex. However, once we start adding additional flags then you'd have to re implement the entire matching logic in the userscript. Otherwise this becomes fuzzy with figuring out what to match against. I suppose you could do separate registrations with different wrapper code for the regex, but that feels bad to me. It's not a very nice situation. |
I'm in favor of accepting no regex support and letting scripts add their own oneliner for a regex test and return if they want to. |
@Sxderp Cool! The user scripts appear in the debugger's sources panel. Small fix: diff --git a/src/user-script-obj.js b/src/user-script-obj.js
index 0aa7375..ef4c3bf 100644
--- a/src/user-script-obj.js
+++ b/src/user-script-obj.js
@@ -285,7 +285,7 @@ window.EditableUserScript = class EditableUserScript
userScript();
})();
} catch (e) { console.error("Script error: ", e); }
- //# sourceURL=user-script:${escape(this.id)}`;
+ `;
this._evalContentVersion = EVAL_CONTENT_VERSION;
}
|
Oh! Now I see the problem. I was assuming that the included regex code was supposed to run after the matches and other includes had completed. So it would be In that case, I think what would be useful would be to see if we can find some data on how many scripts from the userscript sites (e.g. openuserjs.org, greasyfork.org, and possibly an archive of userscripts.org) actually use the regex feature. Maybe we could change it to work as Excludes, of course, are easy. They actually are My ideal is that we break as few scripts as reasonably possible. I know the promise-based system broke GM_ functions, but many scripts don't use those. |
@NoneGiven: That's fine for new code, but what about preexisting code? I think we'd do better to find out how often regex is used with It's better to check how much you will break before choosing the most breaking solution. Question is, does anyone know how to try to find such data, without crawling through the UserScript websites ourselves? |
@trlkly It's already broken for anyone using GM4 right now. If it absolutely must be implemented, then it should be done so I don't take any perf hits, allegedly negligible or otherwise, unless I actually use |
Should work now for script enable / disable, script uninstall, and global enable / disable.
Included @trlkly, even allowing regex for Honestly, I'd like what @NoneGiven suggested. Let authors implement the Regex. There's not going to be any performance improvements if those details are pushed onto GM. However, there's probably going to be backlash, a la #2724, and the many times it has come up. It's also important to note, FF59 is still a few months off. |
So, I looked at all my scripts (most of them I've made), and only one has any regex used. On an excludes. |
It's working perfectly for me, tested with several registers & unregisters via MonkeyMenu. No error in Console & all scripts were correctly working. Fantastic ;)
Wow, 2018-03-13. Maybe @arantius can implement this commit in Beta (probably after GM4.1) and set the min version to 59 (BTW: it was confirmed in the Mozilla bug that we definitely won't get it before FF59) |
Looking back at some Bugzilla entries; https://bugzilla.mozilla.org/show_bug.cgi?id=1391669 was marked as duplicate of https://bugzilla.mozilla.org/show_bug.cgi?id=1332273 but there isn't any privilege restriction options in that API. Might be worth a new bug with the explicit goal of adding restrictions to the |
Sounds like Cu.Sandbox https://bugzilla.mozilla.org/show_bug.cgi?id=1353468 . Wondering if when we can debug our scripts again (and click on a link in a debug message to highlight the affected source code line). |
Similar, but I don't think that is going to happen. To me, directly creating a Cu.Sandbox seems fairly broad and not in line with the goals Mozilla has for WebExtensions. Adding new methods to an existing API (although new) that modify the functionality of the API seems much more likely of being implemented. While both provide roughly the same outcome I feel this way has better semantics which seem pretty important in the WebExtension ecosystem.
Posted somewhere in this thread, using the new |
While this should be done, I think holding off for iteration two is the better bet. Mostly due to 1509339. A few things that I can list off.
|
Given that this is your bug, @arantius, and that you are the one who pushed Mozilla to add APIs for Userscripts and a way to get a proper sandbox, I assumed you thought it important. (The latter suggests a security reason.) I was also under the impression this would greatly simplify the GreaseMonkey code since more would be handled in native code. @kekkc has been arguing that fixing this would fix a large number of scripts that no longer work. I am not so convinced, because he suggests they work on TamperMonkey, and TamperMonkey does not have programmatic content scripts. I suspect the problem @kekkc is having is the opposite: that GreaseMonkey injects earlier than it used to, and much earlier than TamperMonkey. That is a problem I've had to deal with, requiring me to use MutationObservers to wait on the documentElement to even exist. It's a problem I never had to deal with before GM4. Fortunately, I only tend to use document-start to inject stylesheets, so it was not that difficult to write a function to work around this. So, if the security, code simplification, and speedup are not significant, then I do suspect there would not be a huge change. |
The issue is a little more nuanced. Greasemonkey will sometimes catch script injection in time for "document-start" but it's not guaranteed due to asynchronous message passing. The problem you're hitting about being earlier mostly has to do with the script being injected close to document-start. From Mozilla's documentation document-start indicates "DOM is loading." And by that they mean "you probably won't even have I don't know if the DOM issue can be resolved, but I'd consider adding MutationObservers much less breaking than completely missing your target. |
Exactly that's my biggest issue, even with MutationObservers there's currently no reliable method to inject a script before e.g. the title tag (so that the title of a page can be changed). Even worse, it is not executed later, it's simply not executed at all :( Explained in the lenghty original bug since 2013: And now we've a solid way by Firefox (even if it's ripe only in FF65). The effort was made by @Sxderp already who provided a proof of concept for the initial content script (which is similar and already ready to use). Is this just about the adjustment of this concept and adaption to the current master? Even if I don't know the code really, I think I could do this. Or would this be waste of time, after dozens of Mozilla & community people provided the grounds here already as a prio 1 mainly for GM, that are currently being discussed if they will be used ? |
I was doing some testing with the I'm still testing but looking at the code xrays are still enabled with no way to disable them. And, somewhat problematically the export helpers are not available in the userscript sandbox which does make interacting with the page somewhat difficult. I created a bug to hopefully address this issue. |
Awesome ;) BTW: Luca was fast & had some suggestions in https://bugzilla.mozilla.org/show_bug.cgi?id=1516356 |
I have a new proof of concept using the cool APIs available in 66+. Unfortunately the backwards compatibility regex (mentioned some post above in this thread) doesn't quite work. There's supposed to be a way to cancel the user script execution in the |
Another added benefit is that cross origin requests are no longer leaky ( #2549 ). |
This is amazing, thank you so much @Sxderp . Tested and this works flawlessly: Tested in FF developer 65 & 66 Nightly, xpinstall.signatures.required;false & extensions.webextensions.userScripts.enabled;true, https://github.com/Sxderp/greasemonkey/tree/userscript-api downloaded / zipped / renamed as XPI and loaded in FF, scripts imported via GUI:
|
That’s probably because of bug 1503681 getting fixed in Firefox 65. |
Yeah, that was me forgetting to remove a debugging line.
I'm thinking about opening a bug to see if this can be further improved. Right now the 'file' is just a string of random alpha numerics. Would be great if we could set fake file names so you know which script is causing the error without having to click on it. Might be a lot of effort for too little gain.
Interesting, it should work. Granted I didn't test it. I only tested
Could be, but they should work (untested) if your excludes are globs only and the user script in question is only using includes (as globs) or matches (not both). |
Oh, there is an issue regarding this. Let us assume that the conditions are right such that the backwards compatibility code is NOT in use. If you add a new global excludes the script objects in the background will not refresh the cached runtime includes/excludes/matches nor will they reregister themselves. However, if you look at the monkey menu you should see all your scripts move to the "other user scripts" section, as the monkey menu creates new user script objects every time it is opened. The fix is fairly simple, just need to make sure a global refresh is done when global settings are changed (similar to when the plugin is toggled on / off but also refresh the cache). This should only take a few minutes and I'll push a commit. |
First, huge thanks for all your effort.
Maybe we can make it work, but I'm extremely wary of trying to cache things and keep them in sync as they change, that's a Hard Problem. |
Could just refresh the expressions every time the script is reregistered (which has to be done regardless). Monkey menu does this every time it opens since it recreates the objects. The change would be simple. Remove most of the changes from my latest commit and make the register command unconditionally call the refresh. If this is the case then the refresh called at the end of an update can also be removed. |
@ExE-Boss thx, I'll keep an eye on that.
would be nice to have, but if you say that after submitting your monster patch here, I guess it's not worth.
was using sth. like "window.wrappedJSObject.alert = cloneInto(function (){}, window.wrappedJSObject, {cloneFunctions: true});", but I think I've read in some moz bug that they still need to implement functions for cloneInto. exportFunction should be the cleaner way and this works already.
Many thanks, global excludes from monkey menu work again ;) Might just need a refresh for deregister, too. |
I've been thinking about other ways this can benefit. And I'm sure it can lead to simplifying some of the GM APIs. For example I think the XHR API can be rewritten to not send any messages to the background and removed the need for opening the port. I think the same can be done for the open in tab API. Might be able to 'fix' the clipboard API, but I'm not sure about that one. Anyway, I really think that switching to this WebExtension API has so much potential for improvement. With the only downside is trying to stay performant by using native matching when possible (since that's done in C code) and maintaining backward compatibility with historic matching mechanisms (which I think should be dropped, but there's probably a large split on that subject). |
Finally the userscript API has landed in FF68 and is switched on by default (https://bugzilla.mozilla.org/show_bug.cgi?id=1514809), FF68 being available 9th July. Is GM prepared as well? |
Update:
Apart from the advantages mentioned here, this should cover all current features of GM in current FF releases, or are there still showstoppers? |
Seems this is a mess. GM and UserScripts seem to be dead. TM & VM don't offer access to DOM (exportfunction / cloneinto) & Mozilla doesn't want to enable it by default: erosman/support#103 (comment) "To protect the user" the user should simply display an alert box with his userscripts (cause there's not much else left without access to the site). Anyone who has an idea how to use a proper userscript API with FF? @Sxderp |
Of course it is. Untill someone started coding instead of complaining in bug trackers. |
@kekkc Good question. I have a few things to say about this but here isn't the place. Send me an email if you want more information. But in short, perhaps, if there's interest. |
Would love to get some background, unfortunately there's no longer PM feature in github or mail (and I guess noone wants to include their real mail in the public profile for spammers). You can contact me directly via kekkc@spambog.de |
I'm so sick of it. This POC is based from the initial one from one year ago & extended by Luca's suggestion for exportFunction / cloneInto support. This saves currently only one script, but runs at document-start, correct error messages and all the fancy stuff that we want here. If anyone wants to support, let's include support for multiple scripts / permanent storage and bring it to AMO for easy installation and forget this crap here ongoing since 1 year (all the while with a working POC in the background, all credits to https://bugzilla.mozilla.org/user_profile?user_id=609792 (unfortunately this great POC didn't even make it to the official webextension examples https://github.com/mdn/webextensions-examples . Otherwise I would submit this 10x to AMO in order to use 10 userscripts with it. sry for the rant, I'm quiet now, once and for all https://bugzilla.mozilla.org/attachment.cgi?id=9033244 goto about:debugging -> load temporary addon: |
It's really hard to respond to negativity positively. It would be more constructive to calm down and reword, than to tack on an apology.
Besides not knowing what "all the fancy stuff" is, that sounds pretty good. |
https://bugzilla.mozilla.org/show_bug.cgi?id=1332273
Mostly setting this up to make sure the bugzilla bug is tracked. This could/should be useful for us, especially for
@document-start
support? Look into this after the above bug is resolved. If it's useful, gate on stable release of the feature and implement.The text was updated successfully, but these errors were encountered: