-
Notifications
You must be signed in to change notification settings - Fork 22.5k
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 #22644
Add declarativeNetRequest #22644
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.
Here is an initial review pass.
General notes:
- Drop the MV3 mention; as the API is going to be available to MV2 too. In Chrome/Safari there are no MV restrictions.
- Drop the Edge/Chromium boilerplate at the end of all articles.
Specific note: the structure of this draft seems copied from Chromium's documentation at https://developer.chrome.com/docs/extensions/reference/declarativeNetRequest/ . That structure is not great, as it is auto-generated from Chromium's implementation at https://source.chromium.org/chromium/chromium/src/+/main:extensions/common/api/declarative_net_request.idl , which includes many single-use types. Documentation would be more readable when these single-use types are merged with the method that "imports" the type definition.
files/en-us/mozilla/add-ons/webextensions/api/declarativeNetRequest/domaintype/index.md
Outdated
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/api/declarativeNetRequest/dynamic_ruleset_id/index.md
Outdated
Show resolved
Hide resolved
...mozilla/add-ons/webextensions/api/declarativeNetRequest/getavailablestaticrulecount/index.md
Outdated
Show resolved
Hide resolved
...mozilla/add-ons/webextensions/api/declarativeNetRequest/getavailablestaticrulecount/index.md
Outdated
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/api/declarativeNetRequest/getenabledrulesets/index.md
Outdated
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/api/declarativeNetRequest/index.md
Outdated
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/api/declarativeNetRequest/index.md
Outdated
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/api/declarativeNetRequest/requestmethod/index.md
Outdated
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/api/declarativeNetRequest/updateruleoptions/index.md
Outdated
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/api/declarativeNetRequest/domaintype/index.md
Outdated
Show resolved
Hide resolved
@Rob--W "Documentation would be more readable when these single-use types are merged with the method that "imports" the type definition." As explained elsewhere, this is what I believe I've already done - can you provide more specific examples? |
Co-authored-by: Rob Wu <[email protected]>
Preview URLs (34 pages)
Flaws (48)Note! 3 documents with no flaws that don't need to be listed. 🎉 URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
External URLs (7)URL:
URL:
URL:
URL: (comment last updated: 2023-02-12 17:46:20) |
files/en-us/mozilla/add-ons/webextensions/api/declarativenetrequest/resourcetype/index.md
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/api/declarativenetrequest/index.md
Outdated
Show resolved
Hide resolved
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.
(Only replied to comments and the context around the comments, haven't done a full review yet)
files/en-us/mozilla/add-ons/webextensions/api/declarativenetrequest/modifyheaderinfo/index.md
Show resolved
Hide resolved
|
||
If the request is not blocked or redirected, the matching `modifyHeaders` rules are evaluated, with the most recently installed extensions getting priority. Within each extension, all `modifyHeaders` rules with a priority lower than matching `allow` or `allowAllRequests` rules are ignored. | ||
|
||
If multiple `modifyHeaders` rules specify the same header, the resulting modification for the header is determined based on the priority of each rule and the operations specified: |
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.
files/en-us/mozilla/add-ons/webextensions/api/declarativenetrequest/resourcetype/index.md
Outdated
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/api/declarativenetrequest/resourcetype/index.md
Show resolved
Hide resolved
|
||
{{AddonSidebar()}} | ||
|
||
The resource type of a request. |
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 resource type of a request. | |
The resource type of a request. Comparable to {{WebExtAPIRef('webRequest.ResourceType')}}. |
The set of supported types is at https://searchfox.org/mozilla-central/rev/9dfda5ccb0fc42d7666a54b1caf1af6525e49694/toolkit/components/extensions/schemas/declarative_net_request.json#71-93 and https://source.chromium.org/chromium/chromium/src/+/main:extensions/common/api/declarative_net_request.idl;l=12-28;drc=b141211e0c65628cf86a68a37d92377186560560
I'll also suggest some deletions below of obsolete types. Please double-check that the content here and the BCD is accurate.
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.
@Rob--W So, comparing those lists it seems that quite a number of these items are supported in Chrome:
How can we check what is supported in Safari?
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.
I have asked an Apple engineer about the support state. Let's be concerned with what we know (Chrome and Firefox), and correct support information about Apple in a follow-up PR if needed.
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.
I've updated the BCD
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.
I got an answer back from @xeenon . Safari's list of supported ResourceTypes is:
"font"
"image"
"main_frame"
"media"
"other"
"ping"
"script"
"stylesheet"
"sub_frame"
"websocket"
"xmlhttprequest"
files/en-us/mozilla/add-ons/webextensions/api/declarativenetrequest/index.md
Outdated
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/api/declarativenetrequest/modifyheaderinfo/index.md
Outdated
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/api/declarativenetrequest/index.md
Outdated
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/api/declarativenetrequest/index.md
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/api/declarativenetrequest/modifyheaderinfo/index.md
Outdated
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/api/declarativenetrequest/modifyheaderinfo/index.md
Outdated
Show resolved
Hide resolved
|
||
{{AddonSidebar()}} | ||
|
||
The resource type of a request. |
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.
I have asked an Apple engineer about the support state. Let's be concerned with what we know (Chrome and Firefox), and correct support information about Apple in a follow-up PR if needed.
files/en-us/mozilla/add-ons/webextensions/api/declarativenetrequest/resourcetype/index.md
Show resolved
Hide resolved
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.
I did another full pass over the content and it generally looks good.
I added several suggested edits, to add content, improve the accuracy, fix broken links, minor formatting improvements. Feel free to edit my suggested edit before committing the changes as you see fit.
files/en-us/mozilla/add-ons/webextensions/api/declarativenetrequest/index.md
Outdated
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/api/declarativenetrequest/index.md
Outdated
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/api/declarativenetrequest/index.md
Outdated
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/api/declarativenetrequest/index.md
Outdated
Show resolved
Hide resolved
...s/mozilla/add-ons/webextensions/api/declarativenetrequest/max_number_of_regex_rules/index.md
Outdated
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/api/declarativenetrequest/matchedrule/index.md
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/api/declarativenetrequest/modifyheaderinfo/index.md
Outdated
Show resolved
Hide resolved
|
||
{{AddonSidebar()}} | ||
|
||
The resource type of a request. |
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.
I got an answer back from @xeenon . Safari's list of supported ResourceTypes is:
"font"
"image"
"main_frame"
"media"
"other"
"ping"
"script"
"stylesheet"
"sub_frame"
"websocket"
"xmlhttprequest"
...s/mozilla/add-ons/webextensions/api/declarativenetrequest/setextensionactionoptions/index.md
Outdated
Show resolved
Hide resolved
- `requestHeaders` {{optional_inline}} | ||
- : {{WebExtAPIRef("declarativeNetRequest.ModifyHeaderInfo")}}. The request headers to modify for the request. Only valid if `RuleActionType` is `"modifyHeaders"`. | ||
- `redirect` {{optional_inline}} | ||
- : {{WebExtAPIRef("declarativeNetRequest.ModifyHeaderInfo")}}. The response headers to modify for the request. Only valid if `RuleActionType` is `"modifyHeaders"`. |
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.
- `requestHeaders` {{optional_inline}} | |
- : {{WebExtAPIRef("declarativeNetRequest.ModifyHeaderInfo")}}. The request headers to modify for the request. Only valid if `RuleActionType` is `"modifyHeaders"`. | |
- `redirect` {{optional_inline}} | |
- : {{WebExtAPIRef("declarativeNetRequest.ModifyHeaderInfo")}}. The response headers to modify for the request. Only valid if `RuleActionType` is `"modifyHeaders"`. | |
- `requestHeaders` {{optional_inline}} | |
- : {{WebExtAPIRef("declarativeNetRequest.ModifyHeaderInfo")}}. The request headers to modify for the request. Only valid if the action `type` is `"modifyHeaders"`. | |
- `redirect` {{optional_inline}} | |
- : {{WebExtAPIRef("declarativeNetRequest.ModifyHeaderInfo")}}. The response headers to modify for the request. Only valid if the action `type` is `"modifyHeaders"`. |
Co-authored-by: Rob Wu <[email protected]>
…uleAction.responseHeaders Corrected markdown issues in ModifyHeaderInfo
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.
I did a final pass over every detail of the PR and added some final suggested edits. After these, the PR can be merged. Since it's a matter of just committing the suggested edits and there is not much to discuss about it, I'll approve the PR so it does not need another round of review.
...d-ons/webextensions/api/declarativenetrequest/max_number_of_enabled_static_rulesets/index.md
Outdated
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/api/declarativenetrequest/index.md
Outdated
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/api/declarativenetrequest/index.md
Outdated
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/api/declarativenetrequest/index.md
Outdated
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/api/declarativenetrequest/index.md
Outdated
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/api/declarativenetrequest/onrulematcheddebug/index.md
Outdated
Show resolved
Hide resolved
...s/mozilla/add-ons/webextensions/api/declarativenetrequest/setextensionactionoptions/index.md
Outdated
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/api/declarativenetrequest/testmatchoutcome/index.md
Outdated
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/api/declarativenetrequest/updatedynamicrules/index.md
Outdated
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/manifest.json/index.md
Outdated
Show resolved
Hide resolved
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.
Since it's a matter of just committing the suggested edits and there is not much to discuss about it, I'll approve the PR so it does not need another round of review.
Now actually marking as approved - but don't forget to commit the suggested edits!
Co-authored-by: Rob Wu <[email protected]>
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 all your work on this Richard!
Description
Adding the declarativeNetRequest API.