Implement userScripts.execute
Categories
(WebExtensions :: General, enhancement, P3)
Tracking
(firefox153 fixed)
| Tracking | Status | |
|---|---|---|
| firefox153 | --- | fixed |
People
(Reporter: robwu, Assigned: robbendebiene)
References
(Blocks 1 open bug)
Details
(Keywords: dev-doc-complete, Whiteboard: [wecg][addons-jira])
Attachments
(2 files, 1 obsolete file)
This bug tracks our implementation of userScripts.execute as specified in https://github.com/w3c/webextensions/blob/main/proposals/user-scripts-execute-api.md (added in https://github.com/w3c/webextensions/pull/540).
This was mentioned before as the end of the list at https://bugzilla.mozilla.org/show_bug.cgi?id=1875475#c3 (and is not part of the MVP).
Chrome has not implemented this yet. I cannot even find an issue on bugs.chromium.org that tracks an implementation.
| Reporter | ||
Updated•1 year ago
|
Related Chromium bug seems to be private:
https://issues.chromium.org/issues/382190916
https://issues.chromium.org/issues/326657581
Status: Duplicate of 326657581
This is already planned, hopefully can be done in Q1 2025.
| Reporter | ||
Comment 2•1 year ago
|
||
The current proposal specifies userScripts.execute to take one script only, and I wonder whether it should be an array instead: https://github.com/w3c/webextensions/issues/477#issuecomment-2561976716
Furthermore, userScripts.execute also accepts documentId to make sure that the script is executed in the desired document. This has not been implemented in Firefox yet, bug 1891478 tracks documentId work.
| Reporter | ||
Comment 3•1 year ago
|
||
There is another pending query on the expected behavior: https://github.com/w3c/webextensions/pull/745#pullrequestreview-2556289504
Updated•1 year ago
|
| Reporter | ||
Comment 4•1 year ago
|
||
Note, for BCD when userScripts.execute is documented as part of dev-doc-needed:
Chrome has enabled userScripts.execute in Chrome version 135.
Commit: https://chromium.googlesource.com/chromium/src/+/47209ae4c98108dae21d9a52031e504547bf964e
Version: https://chromium.googlesource.com/chromium/src/+/47209ae4c98108dae21d9a52031e504547bf964e/chrome/VERSION
| Assignee | ||
Comment 6•3 months ago
|
||
I'm still interested in this bug/feature. Would you consider a contribution or are there still other things blocking it?
I read about the unspecified behaviour on runtime errors, but this doesn't seem like a major blocker.
If nothing speaks against a contribution, can someone please give me some short guidance or a starting point?
| Reporter | ||
Comment 7•3 months ago
|
||
Patches are welcome indeed. Here are some pointers feel free to ask clarification if needed.
General contribution guidelines are at https://wiki.mozilla.org/WebExtensions/Contribution_Onramp
The entry point, the first place in the implementation that handles API calls in the parent process is at https://searchfox.org/firefox-main/rev/a249a43995d7634e6e4a56bfd6496039031648bc/toolkit/components/extensions/parent/ext-userScripts.js#88-187
This is the relevant place showing where how a user script is represented internally: https://searchfox.org/firefox-main/rev/8352bcb6d75d53f3e2190221b71190e47afa0bfc/toolkit/components/extensions/ExtensionUserScripts.sys.mjs#481
For a one-off user script, look at the tabs.executeScript and scripting.executeScript implementations for inspiration:
- scripting.executeScript goes through https://searchfox.org/firefox-main/rev/8352bcb6d75d53f3e2190221b71190e47afa0bfc/toolkit/components/extensions/parent/ext-scripting.js#40
- tabs.executeScript goes through: https://searchfox.org/firefox-main/rev/8352bcb6d75d53f3e2190221b71190e47afa0bfc/toolkit/components/extensions/parent/ext-tabs-base.js#814
The object constructed there is handled here: https://searchfox.org/firefox-main/rev/8352bcb6d75d53f3e2190221b71190e47afa0bfc/toolkit/components/extensions/ExtensionContent.sys.mjs#1453
Most unit tests for MV3 userScripts are currently written as xpcshell tests. These are preferred where possible because they are self contained and can run in parallel with other tests. The current tests are found here: https://searchfox.org/firefox-main/search?q=test_ext_userScripts_mv3&path=&case=false®exp=false
Unfortunately there is one issue with xpcshell tests: they don't have browser windows and tabs, so the tabIds do not exist. As an alternative you can write the test as a browser chrome test, or a mochitest. Browser tests only run with desktop Firefox, mochitests are also run on Android. Since userScripts is a cross-platform API, a mochitest is therefore preferred. Here is a minimal example: https://searchfox.org/firefox-main/rev/8352bcb6d75d53f3e2190221b71190e47afa0bfc/toolkit/components/extensions/test/mochitest/test_ext_executeScript_mozextension.html#329-378
I am currently working on documentId support (bug 1891478); if that were to be implemented, it would be possible to inject in any context even if it is not in a tab, including in xpcshell tests.
| Assignee | ||
Comment 8•3 months ago
|
||
Thanks for the detailed description. I get to it and report back when I have something.
| Assignee | ||
Comment 9•3 months ago
|
||
handleActorExecute currently only accepts a single js code block but the js property of the userScripts.execute() function allows an array of userScripts.ScriptSource wherefore multiple code blocks can be provided with a single call.
Any suggestion on how I should deal with this? E.g. through multiple calls to tab.queryContent("Execute", options); or should I alter the handleActorExecute function for which in turn I have to change more code down the line like https://searchfox.org/firefox-main/source/toolkit/components/extensions/ExtensionContent.sys.mjs#422
| Assignee | ||
Comment 10•3 months ago
|
||
Updated•3 months ago
|
| Assignee | ||
Comment 11•3 months ago
|
||
| Assignee | ||
Comment 12•3 months ago
|
||
I pushed my first iteration for a first review to get a better feeling whether I'm on the right track.
I'm working on the test cases now.
| Assignee | ||
Comment 13•3 months ago
|
||
| Assignee | ||
Comment 14•3 months ago
|
||
I'm unsure whether I should extend the test_ext_scripting_executeScript_XXX.html tests to also test the userScripts.execute() API or create completely separate test files. The APIs share a lot of code so integrating them makes sense, but on the other hand it will bloat/complicate the testcases further and prevent potential separation in the future. I attached an example where I integrated the userScripts.execute() test into test_ext_scripting_executeScript_activeTab.html. Are there any guidelines for this?
| Reporter | ||
Comment 15•3 months ago
|
||
(In reply to robbendebiene from comment #9)
handleActorExecutecurrently only accepts a single js code block but thejsproperty of the userScripts.execute() function allows an array ofuserScripts.ScriptSourcewherefore multiple code blocks can be provided with a single call.Any suggestion on how I should deal with this? E.g. through multiple calls to
tab.queryContent("Execute", options);or should I alter thehandleActorExecutefunction for which in turn I have to change more code down the line like https://searchfox.org/firefox-main/source/toolkit/components/extensions/ExtensionContent.sys.mjs#422
The current script injection logic executes script files first, then optionally a code string (see getCompiledScripts). In practice, these are mutually exclusive:
codeandfileare mutually exclusive intabs.executeScriptfuncandfileare mutually exclusive inscripting.executeScript(funcandargsare serialized and ultimately assigned tojsCode)
In the userScripts.execute API, they can both be specified, an arbitrary number of times. Your patch tries to implement support by populating the jsPaths and (new) jsCode arrays, but the bug there is that the code would not execute in the specified order, which I'd consider to be a bug.
Since ordering is important, I recommend exploring a way to get js to contain the scripts that need to execute. Right now, there are two distinct code paths:
- for MAIN world injection, a code string is turned into a data:-URL, compiled asynchronously and executed. This is mainly to have a work-around to bypass CSP.
- In ISOLATED worlds (and there is a TODO comment to do something similar for USER_SCRIPT worlds), code is evaluated with
Cu.evalInSandbox. The latter is synchronous. - For
userScripts.register, we compile code toblob:-URLs (at#makeInternalUserScript) and pass it tojsPaths.
For simplicity, it is feasible for userScript.execute to unconditionally go through the "generate URL and compile" approach.
There are a few things to account for:
- We don't want to store huge URLs as the key in the script cache. E.g. if you were to populate
jsPathswith a data:-URL and rely on the existing implementation, thengetCompiledScriptswould populate the script cache with a long URL. - In the same theme, if we use one-off blob:-URLs, we don't want to keep them in the cache either, because nothing would ever hit the cache.
handleActorExecute's error handling logic assumes that if an error occurs while cloning the result of a script execution, that it is in the last file or else a code snippet (source. Depending on your implementation I can see you ending up with adata:-URLthere; make sure that it doesn't happen.
There is more, but I expect that to come up naturally during code review if needed.
(In reply to robbendebiene from comment #14)
I'm unsure whether I should extend the
test_ext_scripting_executeScript_XXX.htmltests to also test theuserScripts.execute()API or create completely separate test files. The APIs share a lot of code so integrating them makes sense, but on the other hand it will bloat/complicate the testcases further and prevent potential separation in the future. I attached an example where I integrated theuserScripts.execute()test into test_ext_scripting_executeScript_activeTab.html. Are there any guidelines for this?
I suggest creating a separate test file, as that would be easier to run, and intermittent test failures are easier to classify/attribute and separate.
Updated•3 months ago
|
Comment 16•2 months ago
|
||
Comment 17•2 months ago
|
||
| bugherder | ||
| Reporter | ||
Updated•2 months ago
|
Comment 18•2 months ago
|
||
Documentation updates in:
- (BCD) Bug 1930776 Implement userScripts.execute #29811
- (content ) Bug 1930776 updates for the implementation of userScripts.execute #44369
Description
•