-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
[webextension] Add support for background.scripts
and background.page
#8906
[webextension] Add support for background.scripts
and background.page
#8906
Conversation
}, | ||
); | ||
} | ||
if (hot) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are now two separate blocks of HMR handling code. This is going to cause issues with HMR: Chrome requires different handling of v2 and v3 to do HMR properly. I'm not sure which version is needed for Firefox's version of Manifest V3. Could you try to investigate?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One way to fix the double-handling of hot
would be to keep the isMV2
check but copy the background.scripts
code into the else
block as well. But you'd still need to investigate if this HMR code actually works in Firefox.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for review! I have pushed the commit with the fix. 1fbbe8b
I investigated whether HMR works in Firefox and having trouble getting HMR to work in Firefox.
In Manifest V3, the script-src directives may only have 'self', 'none' and 'wasm-unsafe-eval'.
Overwrite at the following value in the dist/manifest.json
file after the build, It will work HMR when saving the files specified in the background.scripts
.
"content_security_policy": {
"extension_pages": "script-src 'self';"
},
But, occur the following errors when saving the files specified in the content scripts
It's possible that the reason is the stricter CSP constraints in Manifest V3.
In Manifest V3, all CSP sources that refer to external or non-static content are forbidden. The only permitted values are 'none', 'self', and 'wasm-unsafe-eval'. In Manifest V2, a source for a script directive is considered secure if it meets these criteria:
https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/manifest.json/content_security_policy
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes sense: Chrome and Firefox both treat MV3 the same with regards to the Content Security Policy. Instead of splitting the HMR logic into two blocks for Chrome and Firefox when isMV2
is false, you'll need to use the Chrome version in both cases, but modify it such that if it's running in Firefox, it also checks for background.scripts
.
9d44c28
to
1fbbe8b
Compare
context: 'service-worker', | ||
sourceType: | ||
program.background.type == 'module' ? 'module' : 'script', | ||
if (program.browser_specific_settings?.gecko) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📝 Using browser_specific_settings.gecko
to switch between Firefox and Chrome. Firefox add-on need an ID in their manifest.json when submitted to AMO.
All Manifest V3 extensions need an add-on ID in their manifest.json when submitted to AMO.
https://extensionworkshop.com/documentation/develop/extensions-and-the-add-on-id/
But, it's not necessary in debugging. I want to know some useful flags.
I learned that this should be fixed in Parcel >= 2.10.0 However, when im trying to build a Firefox V3 extension with Parcel 2.10.2 im still getting the "Missing property service_worker" error as before with older versions of parcel. Do i somehow need to specify for which browser i would like to bulld in order to activate this fix? |
No The current logic says that (among others) these two versions are possible {
"background": {
"service_worker": "xyz"
}
...
} {
"background": {
"scripts": ["xyz"]
}
...
} What does your manifest look like? The corresponding JSON schema sections parcel/packages/transformers/webextension/src/schema.js Lines 462 to 478 in c72c336
parcel/packages/transformers/webextension/src/schema.js Lines 79 to 87 in c72c336
|
Here is a shortened version of the manifest i'm using: {
"manifest_version": 3,
"name": "My Extension",
"short_name": "MyE",
"description": "",
"homepage_url": "https://example.com",
"version": "10.2",
"background": {
"scripts": ["javascript/sw.js"],
"persistent": false,
"type": "module"
},
"content_scripts": [
...
],
"action": {
...
},
"icons": {
...
},
"permissions": [
...
],
"web_accessible_resources": [
...
],
"host_permissions": [
...
]
} |
The schema didn't allow |
Thanks! I can confirm the extension builds when removing the type property. I'm not sure yet what implications that might have when running the extension in Firefox. |
↪️ Pull Request
🚨 Test instructions
Add this to your Firefox Add-ons manifest:
✔️ PR Todo