[go: nahoru, domu]

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

Proposal: declaring background scripts in a neutral way #282

Open
carlosjeurissen opened this issue Sep 27, 2022 · 86 comments
Open

Proposal: declaring background scripts in a neutral way #282

carlosjeurissen opened this issue Sep 27, 2022 · 86 comments
Labels
implemented: safari Implemented in Safari proposal Proposal for a change or new feature supportive: chrome Supportive from Chrome supportive: firefox Supportive from Firefox topic: service worker Related to service worker background scripts

Comments

@carlosjeurissen
Copy link
Contributor
carlosjeurissen commented Sep 27, 2022

Edit by @Rob--W on 2023-02-24: The spirit of this feature request is to enable cross-browser development with a single manifest file. Chrome opposed the initial proposal that is described below, but after proposing an alternative solution that consists of allowing service_worker and scripts to co-exist (with the latter being ignored in Chrome), the position changed to Chrome being supportive.

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:

"background": {
  "scripts": ["script1.js", "script2.js"]
}

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():

importScripts(
  'script1.js',
  'script2.js'
);

Browsers only supporting limited event pages would generate a _generated_background_page.html which imports the scripts using script tags:

<head></head><body>
<script src="script1.js"></script>
<script src="script2.js"></script>
</body>

Browsers supporting both serviceWorkers and limited event pages would choose based on their own preference. We can let developers set preferred_environment to serviceWorker or page which browser vendors can ignore as such:

"background": {
  "scripts": ["script1.js", "script2.js"],
  "preferred_environment": "serviceWorker"
}

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:

import "./script1.js";
import "./script2.js";

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:

<head></head><body>
<script type="module" src="script1.js"></script>
<script type="module" src="script2.js"></script>
</body>

See #289

@carlosjeurissen carlosjeurissen added topic: service worker Related to service worker background scripts inconsistency Inconsistent behavior across browsers agenda Discuss in future meetings labels Sep 27, 2022
@xeenon xeenon added supportive: safari Supportive from Safari proposal Proposal for a change or new feature and removed inconsistency Inconsistent behavior across browsers labels Sep 28, 2022
@xeenon xeenon changed the title Inconsistency: declaring background scripts in a neutral way Proposal: declaring background scripts in a neutral way Sep 29, 2022
@xeenon xeenon added next manifest version Consider for the next manifest version and removed next manifest version Consider for the next manifest version labels Sep 29, 2022
@dotproto dotproto added follow-up: chrome Needs a response from a Chrome representative neutral: chrome Not opposed or supportive from Chrome labels Sep 29, 2022
@oliverdunk
Copy link
Member

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.

@Rob--W Rob--W added the supportive: firefox Supportive from Firefox label Sep 29, 2022
@carlosjeurissen
Copy link
Contributor Author

@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.

@apple502j
Copy link

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.

@tophf
Copy link
tophf commented Oct 11, 2022

How would automatic import work for "type": "module" where importScripts is not available? You can't just do import './script2.js' because its scope will be isolated not shared as it's in importScripts or in standard DOM scripts.

@carlosjeurissen
Copy link
Contributor Author
carlosjeurissen commented Oct 13, 2022

@tophf That is a good catch. Since the "type": "module" is a case in which developers already expect isolation, having this behaviour in background scripting in extensions seems expected. If needed, developers can share scope by setting / getting properties on globalThis anyway. To make this extra clear, we can add it to the documentation.

ServiceWorker with type module

So in the case of "type": "module", the _generated_service_worker.js would look like this:

import "./script1.js";
import "./script2.js";

limited event pages with type module

In the case of limited event pages, we can actually also start supporting "type": "module" by adding it to the _generated_background_page.html, see #289

@apple502j
Copy link

Any update on this? I understand this is a lot more complicated, but being able to use one manifest.json file for every browser does help a lot.

@xeenon
Copy link
Collaborator
xeenon commented Nov 3, 2022

Safari and WebKit now support "type": "module" for _generated_background_page.html as well as for service_worker (just not a generated service worker.) WebKit/WebKit@ba3a201

@dotproto
Copy link
Member
dotproto commented Nov 9, 2022

@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"
  }
}

@dotproto dotproto added opposed: chrome Opposed by Chrome and removed follow-up: chrome Needs a response from a Chrome representative labels Nov 10, 2022
@dotproto
Copy link
Member

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 "preferred_environment" key in the background object or the use of a minimal set of properties that apply to multiple different execution environments.

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.

@WorldLanguages
Copy link

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.

@carlosjeurissen
Copy link
Contributor Author

@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 preferred_environment? This way we get a non-breaking long-term solution with maximum browser backward compatibility.

@xeenon
Copy link
Collaborator
xeenon commented Mar 15, 2024

@carlosjeurissen Yes, I would like to support preferred_environment.

@carlosjeurissen
Copy link
Contributor Author

@xeenon That is wonderful. Do we agree on the naming and syntax?
The proposal so far has been to:
add a preferred_environment key under the background dictionary which can be set to either serviceWorker or page. If set to page, it would either use scripts or page depending on which one is specified. If serviceWorker, it will use the script defined in service_worker, while opening up a scenario in which we want to add some form of _generated_service_worker.js based on the scripts key.

The default order is up for browsers to decide. Currently it seems
Chrome will only use serviceWorker nomatter what
Firefox will prefer serviceWorker if defined and implemented
Safari will prefer page over serviceWorker when both are defined due to better support

Do we agree on this? I can proceed on making Bugzilla tickets for Mozilla and WebKit?

@xeenon
Copy link
Collaborator
xeenon commented Mar 31, 2024

@carlosjeurissen That looks good to me. I would just use service_worker for the value and not serviceWorker, for consistency.

Also what about document instead of page to avoid confusion since page is technically deprecated.

@carlosjeurissen
Copy link
Contributor Author

@xeenon I like the use of document instead of page. Good one! As for service_worker seems based on the discussion in #563, using snake case would not be out of place. Would work for me!

@Rob--W any thoughts?

@xeenon
Copy link
Collaborator
xeenon commented Apr 4, 2024

@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 "preferred_environments": [ "document", "service_worker" ] would prefer document, then service worker.

That then begs the question, should this be supported_environments or maybe just environments, where it is a defined list of what the developer expects (i.e. known working environments), and if the browser does not support anything in the list, it will fail to load the extension?

@WorldLanguages
Copy link

if the browser does not support anything in the list, it will fail to load the extension

To clarify, this means that it would only fail if ALL the environments are unavailable, right?

@xeenon
Copy link
Collaborator
xeenon commented Apr 4, 2024

@WorldLanguages Yes

@carlosjeurissen
Copy link
Contributor Author
carlosjeurissen commented Apr 4, 2024

@xeenon going for supported_environments or environments would be very ambiguous. Based on what keys the extension provides, it is already clear what environments the extension itself supports. The concept behind preferred is the fact browsers can decide to ignore this preference (Chromium)). Tho I agree preferred_environments may not be the best naming. We could think of alternatives like preference: ["document", "service_worker"]. Which drops the uncomfortable term environments.

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.

@xeenon
Copy link
Collaborator
xeenon commented Apr 4, 2024

@carlosjeurissen Makes sense. I like preference. It does not limit the interpretation to a specific type of content or execution context. That said, another option could be preferred_context, or just context if preference is implied and can be ignored by the browser.

Ideally we can have it accept a string or array of strings, so I kind of walk back my preference for a plural term.

@xeenon
Copy link
Collaborator
xeenon commented Apr 5, 2024

@carlosjeurissen Another option I thought about is precedence.

"precedence": ["document", "service_worker"]

@xeenon
Copy link
Collaborator
xeenon commented Apr 5, 2024

@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 page or service_worker over scripts aligns better with Chrome’s choices and keeps our approach consistent. Even if page is deprecated, it matches well with service_worker, making our logic clearer and more predictable.

I'm working on an implementation now for WebKit, so I'm trying to sort these things out.

@carlosjeurissen
Copy link
Contributor Author

@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 service_worker will always be used. Except for browsers who do not understand preferred_environment and prefer scripts over service_worker.

"background": {
    "preferred_environment": [ "document" ],
    "page": "background.html",
    "scripts": [ "background.js" ]
}

In the above case, browsers would not allow loading the extension because both page and scripts are specified. Which would make it invalid.

If we want to move away from this limitation and allow both to be specified. Then it would indeed make sense to use page and scripts as is. However, this would prevent a future implementation of _generated_service_worker.js based on the scripts specified in scripts, if we would want something like that.

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.

@xeenon
Copy link
Collaborator
xeenon commented Apr 5, 2024

In Safari it is not an error to provide page and scripts. So we need to prefer one over the other.

@carlosjeurissen
Copy link
Contributor Author

@xeenon About the naming.

precedence works for me. The downside of preferred_environment is the fact it implies singular, yet takes an array. While both precedence and preference lack this kind of singular / plural implication.

webkit-commit-queue pushed a commit to xeenon/WebKit that referenced this issue Apr 9, 2024
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
@carlosjeurissen
Copy link
Contributor Author

@xeenon Even if page is deprecated

Not under the impression page was ever deprecated. (except by chrome in mv3).

Glad to see the browser behaviour when including multiple values is now documented on mdn:
https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/manifest.json/background#browser_support

@xeenon Not sure the info here is correct. It states:

mdn: supports background.scripts (or background.page) if service_worker is not specified.

This should probably be

Supports specifying scripts, page and service_worker simultaneously (since Safari x?).

in Safari, the service_worker property is used, and a service worker starts that opens the tab because Safari gives priority to using service workers for background scripts.

This should probably be

Safari prefers `scripts`, then `page`, then `service_worker` since Safari x. Until Safari x, a `service_worker` is preferred over `page` and `scripts.

However these changes might not have shipped yet?

@xeenon
Copy link
Collaborator
xeenon commented Apr 13, 2024

@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.

@carlosjeurissen
Copy link
Contributor Author

@oliverdunk When using this syntax on CWS, extensions currently seem to be rejected with the following message:
The "background.page" key cannot be used with manifest_version 3. Use the "background.service_worker" key instead."

The background property in the manifest is set as follows:

"background": {
  "service_worker": "/background.js",
  "page": "/views/background.html",
  "preferred_environment": ["document", "service_worker"]
}

@oliverdunk
Copy link
Member

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 :)

@carlosjeurissen
Copy link
Contributor Author

@oliverdunk Thanks for escalating! If the CWS review team is afraid including both service_worker and page / scripts is not intentional, it could require the preferred_environment?

@oliverdunk
Copy link
Member

@oliverdunk Thanks for escalating! If the CWS review team is afraid including both service_worker and page / scripts is not intentional, it could require the preferred_environment?

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
implemented: safari Implemented in Safari proposal Proposal for a change or new feature supportive: chrome Supportive from Chrome supportive: firefox Supportive from Firefox topic: service worker Related to service worker background scripts
Projects
None yet
Development

No branches or pull requests