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

Migrate to Manifest v3 #4

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Migrate to Manifest v3 #4

wants to merge 9 commits into from

Conversation

dmlls
Copy link
Owner

@dmlls dmlls commented Mar 27, 2023

As discussed in #3, this PR implements the necessary changes to comply with Manifest V3, following the Firefox Manifest V3 migration guide.

@dmlls dmlls added the enhancement New feature or request label Mar 27, 2023
@dmlls dmlls changed the title Migrate to ManifestA v3 Migrate to Manifest v3 Mar 27, 2023
@dmlls dmlls marked this pull request as ready for review March 27, 2023 11:08
@dmlls
Copy link
Owner Author

dmlls commented Mar 27, 2023

@NyanKiyoshi here is the PR, as promised :)

Do you think you could give it a try also on Chrome to see how things work there?

@NyanKiyoshi
Copy link

NyanKiyoshi commented Mar 28, 2023

@dmlls that seems tricky to implement proper manifest v3 support.

The first issue is background.scripts becomes background.service_worker.

The second is permissions: webRequest and webRequestBlocking are no longer valid in manifest v3 (at least for chrome and chromium browsers). Thus instead of using browser.webRequest in background.js, we need to define JSON rules for the overrides.

The manifest changes required:

Patch File
diff --git a/manifest.json b/manifest.json
index facdcb3..83d7c0d 100644
--- a/manifest.json
+++ b/manifest.json
@@ -20,8 +20,7 @@
   },

   "permissions": [
-    "webRequest",
-    "webRequestBlocking",
+    "declarativeNetRequest",
     "activeTab"
   ],

@@ -363,8 +362,6 @@
   ],

   "background": {
-    "scripts": [
-      "background.js"
-    ]
+    "service_worker": "background.js"
   }
 }

On the compatibility matrix I do not see any support for rules in Firefox thus it may not work: https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/declarativeNetRequest#browser_compatibility.

I do not personally see a way to re-implement background.js without whether having a huge amount of dynamic rules, or redirecting to the extension's URL (e.g. extension://yang-extension-id/handle-yang.html?search=foo) which sounds like bad UX due to having to redirect twice the user (doesn't work based on: https://bugs.chromium.org/p/chromium/issues/detail?id=1262147).

@dmlls
Copy link
Owner Author

dmlls commented Apr 2, 2023

Hi @NyanKiyoshi, thanks a lot for the detailed feedback! 💯

I see things are not looking all that great... I'll take a close look during the next week and see what we can do, but yeah, hope there's a solution that doesn't involve any ugly workarounds, as you mentioned.

@dmlls
Copy link
Owner Author

dmlls commented Apr 3, 2023

@NyanKiyoshi So actually the webRequestBlocking permission is not needed, so it can be simply removed.

As for the background.scripts vs background.service_worker conflict, it seems indeed there is currently no way of making them work simultaneously on both Firefox and Chrome. A possible solution would be to keep two separate manifest.json as mentioned here. Since the change is minimal, at least for now this wouldn't add much overhead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants