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.

Sign up for GitHub

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

Jump to bottom

Add host_permissions and optional_host_permissions #221

Merged
merged 2 commits into from
Jul 7, 2022
Merged

Add host_permissions and optional_host_permissions #221

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5bfdb4b
Add host_permissions and optional_host_permissions
carlosjeurissen May 31, 2022
8c0d835
Change Manifest V3 casing.
carlosjeurissen Jun 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 21 additions & 8 deletions index.bs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,11 @@ The following keys must be considered valid:
* <a href="#key-devtools_page">`devtools_page`</a>: optional
* <a href="#key-externally_connectable">`externally_connectable`</a>: optional

The following keys must be considered valid in Manifest V3:

* <a href="#key-host_permissions">`host_permissions`</a>: optional
* <a href="#key-optional_host_permissions">`optional_host_permissions`</a>: optional

### Key `manifest_version`

This key must be present.
Expand All @@ -77,6 +82,22 @@ This key must be present. This property can be localized.

This key must be present.

### 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.

Comment on lines +85 to +100
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: We may want to consider sorting the list of keys alphabetically or by another well-defined scheme.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The list was already not sorted alphabetically. Grouped the permissions-related keys together. Logical grouping seems more useful than alphabetically sorting the list.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed the casing of the Manifest V3 mention.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

### Key `default_locale`

This key must be present if the `_locales` subdirectory is present, must be absent otherwise.
Expand Down Expand Up @@ -105,18 +126,10 @@ This key may be present.

This key may be present.

### Key `optional_permissions`

This key may be present.

### Key `options_ui`

This key may be present.

### Key `permissions`

This key may be present.

### Key `short_name`

The short name of the extension. This value should be used in contexts where <a href="#key-name">`name`</a> is too long to use in full. If `short_name` is not provided, manifest consumers should use a truncated version of `name`.
Expand Down