[DNR] Add DNR API bindings and permissions
Categories
(WebExtensions :: Request Handling, task, P1)
Tracking
(firefox105 fixed)
Tracking | Status | |
---|---|---|
firefox105 | --- | fixed |
People
(Reporter: robwu, Assigned: robwu)
References
(Blocks 2 open bugs)
Details
(Whiteboard: [addons-jira])
Attachments
(2 files)
The registration of the declarativeNetRequest API and its permissions is not trivial, because it has three permissions, and one of them has a permission warning which requires coordination with an external repository (Android-Components, referenced in bug 1671453).
This bug is a placeholder to attach the patch that introduces the API bindings, permissions and unit tests for it (behind new default-off preferences).
I'm linking bug 1490797 to this one, because the introduction of a new permission-with-warning triggers the conditions described in that bug. I'm not marking it as a blocker, since the new API is disabled by default.
To improve the visibility of the work needed to introduce a new permission with a permission warning for future reference, I'm linking this to bug 1671453.
Assignee | ||
Updated•2 years ago
|
Updated•2 years ago
|
Assignee | ||
Comment 1•2 years ago
|
||
This patch adds the minimum necessary to register the
declarativeNetRequest API and its permissions, behind prefs.
Tests have been added/updated to verify that the permissions and API
access are enforced correctly (effectiveness of preferences, API
visibility, permission warnings).
Before landing this, we need to register the permission warning in
Android-Components too, as mentioned in the bug (i.e. bug 1671453).
Assignee | ||
Updated•2 years ago
|
Comment 2•2 years ago
|
||
(In reply to Rob Wu [:robwu] from comment #1)
Before landing this, we need to register the permission warning in
Android-Components too, as mentioned in the bug (i.e. bug 1671453).
while I was reading this last part of the patch description, I was wondering:
- does the addition of the permission warning in the android components really need to block this while the about:config pref is still disabled by default?
- have we already considered to also keep the new API disabled (independently from the pref value) in non-nightly builds?
(until we have finalized the permission strings to be shown and got enough of the API to make it useful and something that the extension developers may start to try it out)
Updated•2 years ago
|
Assignee | ||
Comment 3•2 years ago
|
||
(In reply to Luca Greco [:rpl] [:luca] [:lgreco] from comment #2)
(In reply to Rob Wu [:robwu] from comment #1)
Before landing this, we need to register the permission warning in
Android-Components too, as mentioned in the bug (i.e. bug 1671453).while I was reading this last part of the patch description, I was wondering:
- does the addition of the permission warning in the android components really need to block this while the about:config pref is still disabled by default?
In this specific case, it isn't strictly necessary to be blocked on Android-Components, because the permission won't be seen on Android by default (with the implementation in the patch that filters out the permission). In general, it is recommended to not forget about the permission, or else the Android application would crash.
- have we already considered to also keep the new API disabled (independently from the pref value) in non-nightly builds?
(until we have finalized the permission strings to be shown and got enough of the API to make it useful and something that the extension developers may start to try it out)
The pref being off-by-default should be sufficient. I am however open to disabling the feature by default on Nightly if we really want to prevent pre-release usage of this feature. In that case we would have to skip some tests too.
Assignee | ||
Comment 4•2 years ago
|
||
Restrict the DNR permissions to MV3 to allow the add-ons linter to
easily flag the use of the not-yet-supported DNR permission, until
we do actually enable the feature.
Since the full DNR namespace is gated on this permission, this
effectively means that in order to use the API, not only the
extensions.dnr.enabled pref needs to be set, but also the
extensions.manifestV3.enabled pref + using manifest_version: 3..
Assignee | ||
Comment 5•2 years ago
|
||
Created https://github.com/mozilla-mobile/android-components/issues/12616 as a follow-up for the A-C part.
Comment 7•2 years ago
|
||
Backed out for causing xpcshell failures on test_ext_permission_warnings.js
- Backout link
- Push with failures
- Failure Log
- Failure line: TEST-UNEXPECTED-FAIL | xpcshell.ini:toolkit/components/extensions/test/xpcshell/test_ext_permission_warnings.js | declarativeNetRequest_permission_with_warning - [declarativeNetRequest_permission_with_warning : 498] Expected origins and permissions - {"permissions":[],"origins":[]} deepEqual {"origins":[],"permissions":["declarativeNetRequest"]}
Assignee | ||
Comment 9•2 years ago
|
||
(In reply to Cristian Tuns from comment #7)
Backed out for causing xpcshell failures on test_ext_permission_warnings.js
This was only in condprof tests, which is an issue on its own - handled in bug 1783828 (and follow-up in bug 1769184).
Comment 10•2 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/8149daf3084a
https://hg.mozilla.org/mozilla-central/rev/c9335a3ca699
Comment 11•2 years ago
|
||
Assignee | ||
Updated•2 years ago
|
Description
•