Add host_permissions and optional_host_permissions#221
Conversation
dotproto
left a comment
There was a problem hiding this comment.
I suggested a small change, but beyond that this LGTM.
| ### Key `permissions` | ||
|
|
||
| This key may be present. | ||
|
|
||
| ### Key `optional_permissions` | ||
|
|
||
| This key may be present. | ||
|
|
||
| ### Key `host_permissions` | ||
|
|
||
| This key may be present. | ||
|
|
||
| ### Key `optional_host_permissions` | ||
|
|
||
| This key may be present. | ||
|
|
There was a problem hiding this comment.
Nit: We may want to consider sorting the list of keys alphabetically or by another well-defined scheme.
There was a problem hiding this comment.
The list was already not sorted alphabetically. Grouped the permissions-related keys together. Logical grouping seems more useful than alphabetically sorting the list.
There was a problem hiding this comment.
Changed the casing of the Manifest V3 mention.
There was a problem hiding this comment.
I prefaced this with "nit" because it was a small, relatively unimportant observation. I was mostly just thinking out loud. I mostly agree with the intent behind grouping related keys.
That said, but for readers that aren't clear on the association between proximity-based groups I worry that it can end up feeling like a random collection rather than an intentionally curated list. One way we could address this is by making groups part of the heading structure of the document, but I don't feel strongly enough about it to tackle that concern in this PR or likely even this year.
Co-authored-by: Simeon Vincent <simeonv@google.com>
dotproto
left a comment
There was a problem hiding this comment.
LGTM. Thanks for taking this on, @carlosjeurissen.
| ### Key `permissions` | ||
|
|
||
| This key may be present. | ||
|
|
||
| ### Key `optional_permissions` | ||
|
|
||
| This key may be present. | ||
|
|
||
| ### Key `host_permissions` | ||
|
|
||
| This key may be present. | ||
|
|
||
| ### Key `optional_host_permissions` | ||
|
|
||
| This key may be present. | ||
|
|
There was a problem hiding this comment.
I prefaced this with "nit" because it was a small, relatively unimportant observation. I was mostly just thinking out loud. I mostly agree with the intent behind grouping related keys.
That said, but for readers that aren't clear on the association between proximity-based groups I worry that it can end up feeling like a random collection rather than an intentionally curated list. One way we could address this is by making groups part of the heading structure of the document, but I don't feel strongly enough about it to tackle that concern in this PR or likely even this year.
As each vendor agreed to add
optional_host_permissionsand thus alsohost_permissionsin manifest v3, it's time to add it to this repo.Background issue about supporting
optional_host_permissions: #119Preview | Diff