-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Add declarativeNetRequest (DNR) + MV3 examples #526
Conversation
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.
The examples in this PR looks pretty good to me, I haven't yet tried them locally but I collected a few comments that I wanted to submit in the meantime (nothing big, mainly nits, typos and a couple of thoughts).
I'll try it out the 3 extensions briefly tomorrow and after that I expect to be able to just sign-off this PR from my side.
These examples are designed to be cross-browser compatible. In particular, these extensions do not use `background` because there is currently no single manifest that can support both Firefox and Chrome, due to the lack of event page support in Chrome, and the lack of service worker support in Firefox. Three examples demonstrating the use of the declarativeNetRequest API: - dnr-block-only: One minimal example demonstrating the use of static DNR rules to block requests. - dnr-redirect-url: One minimal example demonstrating the use of static DNR rules to redirect requests. - dnr-dynamic-with-options: A generic example demonstrating how host permissions can be requested and free forms to input DNR rules.
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.
Some suggestions for your consideration
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.
As anticipated in my review pass yesterday, I did try it out each of these 3 extensions locally, followed the instructions from the README to use the example and then adapt it to an MV2 extension that can run on Firefox and provide the exact same behaviors on the MV3 one.
I confirm that the extensions were working as described in the README and that following the steps to adapt to a Firefox MV2 extension also worked as expected on all 3 of them.
As a side note I also tried all 3 MV3 extensions examples on Chrome as well, the only differences in behavior that I've noticed were all highlighted already (e.g. on removing the granted permission in dnr-redirect-url was one, and an inline comment nearby where the errors is reported is already explaining the different behavior in chrome), and all the DNR related behavior worked as expected also on Chrome.
Feel free to merge this once you have integrated all you need from the feedback pass from Richard.
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 the language improvements Richard. I'll apply almost all of them (and add comments where the suggestion cannot be applied as-is).
Github choked at my attempt to use the UI to apply the changes, so I'll update the PR and force-push the commit.
dnr-redirect-url/README.md
Outdated
- example.com/ to `redirectTarget.html` packaged with the extension. | ||
- example.com/ew to extensionworkshop.com | ||
- https://www.example.com/[anything] to the same URL but the domain changed to example.com and `?redirected_from_www=1` appended to the URL. | ||
- URLs matching regular expression `^https?://([^?]+)$` to the same URL but with scheme changed to https and `?redirected_by_regex` appended. |
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.
Yes, the pattern matches http and https.
Co-authored-by: rebloor <[email protected]>
Co-authored-by: rebloor <[email protected]>
Co-authored-by: rebloor <[email protected]>
Co-authored-by: rebloor <[email protected]>
Congratulations on your first merged pull request. 🎉 Thank you for your contribution! Did you know we have a project board with high-impact contribution opportunities? We look forward to your next contribution. |
Description
Three examples demonstrating the use of the declarativeNetRequest API:
These examples are designed to be cross-browser compatible. In particular, these extensions do not use
background
because there is currently no single manifest that can support both Firefox and Chrome, due to the lack of event page support in Chrome, and the lack of service worker support in Firefox.Where relevant the examples also include a comparison with MV2.
Motivation
Need demos to complement the documentation at https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/declarativeNetRequest
Additional details
Related issues and pull requests
Relates to #496. While this PR adds some examples, it does not fully resolve #496 because it avoids the use of
background
to be cross-browser-compatible. More examples are needed before we can close #496.