-
Notifications
You must be signed in to change notification settings - Fork 50
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
Proposal: declaring background scripts in a neutral way #282
Comments
One thing that makes this potentially less impactful is that lots of developers use bundlers which handle this sort of thing automatically. That said - adding an extra script is currently the path recommended in https://github.com/mozilla/webextension-polyfill and I think that speaks to the benefits of keeping code snippets that work in MV2 working in MV3. |
@oliverdunk bundlers can be very useful indeed and also using them in many of my extension projects. The point of this request was also to align declaring background scripts across browsers independent on whether or not the browsers use serviceWorkers or event pages. Adding the webExtension polyfill is one of the use cases. Another one is reusing script files across your options pages, popup pages, content scripts and background pages. As you mentioned I also very much support the idea of reducing friction when switching manifest versions. |
I do believe there should be at least a way to declare both Event Pages and Service Workers in a single manifest (the script performs browser check in case a browser implements both). This helps people downloading builds from GitHub zipball, those developing extensions on tablets or Chromebooks (yes, they do exist), or those who can't use bundlers for some reason. |
How would automatic import work for |
@tophf That is a good catch. Since the ServiceWorker with type moduleSo in the case of import "./script1.js";
import "./script2.js"; limited event pages with type moduleIn the case of limited event pages, we can actually also start supporting |
Any update on this? I understand this is a lot more complicated, but being able to use one |
Safari and WebKit now support |
@xeenon, just to make sure I understand, with this change is the following manifest.json valid in Safari? {
"name": "Demo",
"version": "0.0.1",
"manifest_version": 3,
"background": {
"scripts": ["lib.js", "background.js"],
"type": "module"
}
} |
I recently had a chance to discuss the original proposal with other Chrome folks. At the moment we have significant reservations with using a minimal set of manifest fields that work across all browsers. We expect that this approach will lead to situations where background scripts authored for page contexts or service worker contexts will be executed in a different environment from what the author expected and, as a direct result, unexpected runtime behavior. As such, we are currently opposed to the proposed introduction of a That said, we did not discuss (and I think it may be worth exploring) other approaches to defining a single manifest file that works across multiple environments. For example, rather than have a way for a developer to suggest an execution environment, perhaps we could have an explicit declaration of entry points per environment. |
Any updates? Chrome should suggest an alternative proposal that they wouldn't oppose. We at the Scratch Addons open-source extension (100,000+ users) are watching this closely. Is it reasonable that updating to MV3 will force us to use a bundler? We have been considering adding a bundler to our workflow for some time, but did not foresee that we would need to hurry because of Manifest V3. |
@xeenon @Rob--W Thanks for the updates! And very happy we got this far! As for moving forward. Would Apple and Mozilla be open to offer developers to explicitly share their preferred environment using the proposed |
@carlosjeurissen Yes, I would like to support |
@xeenon That is wonderful. Do we agree on the naming and syntax? The default order is up for browsers to decide. Currently it seems Do we agree on this? I can proceed on making Bugzilla tickets for Mozilla and WebKit? |
@carlosjeurissen That looks good to me. I would just use Also what about |
@carlosjeurissen I talked with @Rob--W about this some today, and he was suggesting this likely should take an array, for future compatibility and allow extension developers to specify their preferred fallback environments. So That then begs the question, should this be |
To clarify, this means that it would only fail if ALL the environments are unavailable, right? |
@WorldLanguages Yes |
@xeenon going for Happy with it being an array. This indeed opens up potential future additions and makes it even more explicit which environments in what order one is dealing with. |
@carlosjeurissen Makes sense. I like Ideally we can have it accept a string or array of strings, so I kind of walk back my preference for a plural term. |
@carlosjeurissen Another option I thought about is "precedence": ["document", "service_worker"] |
@carlosjeurissen Bikeshedding aside, what should we do in cases likes these? "background": {
"preferred_environment": [ "service_worker" ],
"service_worker": "background.js",
"scripts": [ "background.js" ]
} "background": {
"preferred_environment": [ "document" ],
"page": "background.html",
"scripts": [ "background.js" ]
} Preferring specific keys like I'm working on an implementation now for WebKit, so I'm trying to sort these things out. |
@xeenon as far as I know in Chrome and Firefox, specifying both "scripts" and "page" has always been invalid and rejected when attempting to load an extension. As for your cases: "background": {
"preferred_environment": [ "service_worker" ],
"service_worker": "background.js",
"scripts": [ "background.js" ]
} In the above case, the "background": {
"preferred_environment": [ "document" ],
"page": "background.html",
"scripts": [ "background.js" ]
} In the above case, browsers would not allow loading the extension because both If we want to move away from this limitation and allow both to be specified. Then it would indeed make sense to use To make sure we include each scenario. I assume we do not want to consider allowing multiple background instances to ever exist in the future right? For example running a serviceWorker side by side to a document or something similar. |
In Safari it is not an error to provide |
@xeenon About the naming.
|
https://webkit.org/b/272244 rdar://problem/125988233 Reviewed by Brian Weinstein. Add support for extensions declaring a `preferred_environment` in the `background` entry. The `preferred_environment` key can be a string or an array of strings. When an array is used, the browser will prefer the first one it supports. "background": { "preferred_environment": [ "service_worker", "document" ], "scripts": [ "script1.js", "script2.js" ] } If no environments are specified or supported, then we fallback to the previous handling of the `scripts`, `page` or `service_worker` keys (in that order) for backwards compatibility. This allows cross-browser extensions to have a single manifest that works in multiple browsers. Chrome plans to only support service workers going forward, but Safari and Firefox plan to support pages and service workers, and extension might prefer a document environment over a service worker in Safari and Firefox. This also adds support for a `_generated_service_worker.js` script that works with normal scripts or modules based on the `scripts` and `type` keys if `preferred_environment` is `service_worker`. WECG discussion here: w3c/webextensions#282 * Source/WebCore/en.lproj/Localizable.strings: Updated. * Source/WebKit/UIProcess/Extensions/Cocoa/WebExtensionCocoa.mm: (WebKit::WebExtension::resourceStringForPath): Support _generated_service_worker.js. (WebKit::WebExtension::resourceDataForPath): Ditto. (WebKit::WebExtension::backgroundContentUsesModules): Use renamed m_backgroundContentUsesModules. (WebKit::WebExtension::backgroundContentIsServiceWorker): Use new m_backgroundContentEnvironment. (WebKit::WebExtension::backgroundContentPath): Support _generated_service_worker.js. (WebKit::WebExtension::generatedBackgroundContent): Generate the service worker script. (WebKit::WebExtension::populateBackgroundPropertiesIfNeeded): Handle preferred_environment. * Source/WebKit/UIProcess/Extensions/WebExtension.h: Added m_backgroundContentEnvironment and renamed m_backgroundPageUsesModules to m_backgroundContentUsesModules. * Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebExtension.mm: (TestWebKitAPI::TEST(WKWebExtension, BackgroundPreferredEnvironmentParsing)): Added. * Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebExtensionController.mm: (TestWebKitAPI::TEST(WKWebExtensionController, BackgroundWithServiceWorkerPreferredEnvironment)): Added. (TestWebKitAPI::TEST(WKWebExtensionController, BackgroundWithPageDocumentPreferredEnvironment)): Added. (TestWebKitAPI::TEST(WKWebExtensionController, BackgroundWithScriptsDocumentPreferredEnvironment)): Added. (TestWebKitAPI::TEST(WKWebExtensionController, BackgroundWithMultipleDocumentModuleScripts)): Added. (TestWebKitAPI::TEST(WKWebExtensionController, BackgroundWithMultipleServiceWorkerScripts)): Added. (TestWebKitAPI::TEST(WKWebExtensionController, BackgroundWithMultipleServiceWorkerModuleScripts)): Added. Canonical link: https://commits.webkit.org/277228@main
Not under the impression Glad to see the browser behaviour when including multiple values is now documented on mdn: @xeenon Not sure the info here is correct. It states:
This should probably be
This should probably be
However these changes might not have shipped yet? |
@carlosjeurissen Correct, the recent changes have not shipped yet. The preference order change is in Safari Technology Preview 192, but that's all for now. We tend to update MDN once we ship things in a main Safari release. |
@oliverdunk When using this syntax on CWS, extensions currently seem to be rejected with the following message: The background property in the manifest is set as follows: "background": {
"service_worker": "/background.js",
"page": "/views/background.html",
"preferred_environment": ["document", "service_worker"]
} |
Thanks @carlosjeurissen - I've escalated this internally, I'll let you know directly once I hear back. For the benefit of others, the item was rejected during review. Chrome fully supports this and the Chrome Web Store allows the extensions to be uploaded, but it looks like this was a one-off where it was incorrectly flagged. It shouldn't be a widespread problem :) |
@oliverdunk Thanks for escalating! If the CWS review team is afraid including both |
I think it's more that your extension got tested using an older version of Chrome, so it actually failed to load in the browser. They are usually pretty good about using an updated version of Chrome, so I'm not sure what happened, but looking into it. |
Since the group currently does not agree what the future of background scripting should look like Safari and Firefox with limited event pages, Chrome with serviceWorkers
To make sure developers can define their background scripts in both situations I am proposing the following syntax:
Browsers only supporting serviceWorkers would use the script directly as serviceWorker if only one script is defined. If multiple script files are listed, it will create a generated serviceWorker named
_generated_service_worker.js
just like browsers currently generate a_generated_background_page.html
. This serviceWorker will import all scripts using importScripts():Browsers only supporting limited event pages would generate a
_generated_background_page.html
which imports the scripts using script tags:Browsers supporting both serviceWorkers and limited event pages would choose based on their own preference. We can let developers set
preferred_environment
toserviceWorker
orpage
which browser vendors can ignore as such:Why is this important? This will lead to a more optimal contract between the developer and the browser. In which the developer is able to communicate their preferred environment while allowing browsers to decide what environment extensions should be using. This also makes sure browsers can change their preferred environment over time and have their own approach towards background script handling.
This also has the added benefit of allowing browsers to support both mv2 and mv3 like Safari does right now.
"type": "module"
This syntax works perfectly fine with type module as well by using imports in the serviceWorker, or the type module attribute for the limited event page.
In the case of
"type": "module"
, the_generated_service_worker.js
would look like this:In the case of limited event pages, we can actually also start supporting
"type": "module"
by adding it to the_generated_background_page.html
as such:See #289
The text was updated successfully, but these errors were encountered: