Update: bump the manifest version to v3 #5339
Conversation
vrishtrix
requested review from
molant,
flynnolivia and
vidorteg
as code owners
|
|
|
Thank you for the PR @vrishtrix, this has been in our radar for some time. We are validating a couple of tricky scenarios and if everything works out as expected we can merge this PR. I'll come back soon and take a look. |
There was a problem hiding this comment.
I took a look and it seems this requires more work 😉, here are some of my findings:
manifest.jsonstill have some V2 only properties (left some comments)- service workers cannot longer access the
DOMsoextension-browser/src/shared/globals.tsneeds to account for this (e.g failures indocument,window,MutationObserverand impactspackages/extension-browser/src/content-script/connector.ts) tabs.executeScriptbecomesscripting.executeScriptthis impacts extension-browser/src/background-script.tsand alsomanifest.json
There might be more but these are the ones I found on a quick glance. Please address them and feel free to ping me again once review is required. Thanks again for your contributions to the webhint community.
| @@ -1,22 +1,22 @@ | |||
| { | |||
| "author": "webhint", | |||
| "background": { | |||
| "scripts": [ | |||
| "service_worker": [ | |||
| "background-script.js" | |||
| ], | |||
| "persistent": false | |||
There was a problem hiding this comment.
This should be taken out on V3
| @@ -1,22 +1,22 @@ | |||
| { | |||
| "author": "webhint", | |||
| "background": { | |||
| "scripts": [ | |||
| "service_worker": [ | |||
| "background-script.js" | |||
| ], | |||
There was a problem hiding this comment.
in V3 should be a string instead of an array
| "default_title": "webhint", | ||
| "default_popup": "browser-action/browser-action.html" | ||
| }, | ||
| "default_locale": "en", | ||
| "description": "Quickly test your website to see if there are any issues with accessibility, browser compatibility, security, performance and more.", | ||
| "devtools_page": "devtools/devtools.html", | ||
| "manifest_version": 2, | ||
| "manifest_version": 3, | ||
| "name": "webhint", | ||
| "permissions": [ | ||
| "<all_urls>", |
There was a problem hiding this comment.
seems that <all_urls> is not a valid value in permissions any more (maybe host_permissions)?
Pull request checklist
Make sure you:
For non-trivial changes, please make sure you also:
Short description of the change(s)
The first step to adding support for Manifest v3, addresses the concern laid out on #5308.
This PR bumps the manifest file version to v3 which is intended to be followed up with changes related to functionality to make the browser extension compatible.
It has no breaking changes.