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.

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

Rename PermissionName enum to PowerfulFeatureName #286

Closed
wants to merge 5 commits into from

Conversation

marcoscaceres
Copy link
Member

@marcoscaceres marcoscaceres commented Aug 31, 2021

The PermissionName is a misnomer - it actually identifies "powerful features" for which a permission status can be queries.

To bring some sanity, I'm renaming PermissionName enum to PowerfulFeatureName.

The following tasks have been completed:

  • Modified Web platform tests - change is not observable.

Implementation commitment:


Preview | Diff

Copy link
Member

@miketaylr miketaylr left a comment

Choose a reason for hiding this comment

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

I think the analog from Permissions Policy would be "policy controlled features", so this change brings us closer in alignment, I guess.

Personal opinion: It would be nicer to just call it FeatureName, and have an implied "Powerful", but I guess that's awkward with "powerful feature" everywhere else.

@marcoscaceres
Copy link
Member Author

Can go with FeatureName - it’s less 💪. Will update.

@marcoscaceres
Copy link
Member Author

I might notify various browser vendors of this change before merging, and send patches where appropriate.

@jyasskin
Copy link
Member

This change will make it more important to unify the list of features between this spec and https://github.com/w3c/webappsec-permissions-policy/blob/main/features.md. It was easier to get away with separate lists when this spec's enum was named something specific to permissions, but if it's named very generally, it needs to actually cover all the "features".

@marcoscaceres
Copy link
Member Author

This change will make it more important to unify the list of features between this spec and https://github.com/w3c/webappsec-permissions-policy/blob/main/features.md. It was easier to get away with separate lists when this spec's enum was named something specific to permissions, but if it's named very generally, it needs to actually cover all the "features".

Right, which is why I originally renamed it to "PowerfulFeatureName". Our spec only deals with APIs that "prompt" (i.e., "powerful features"), not with all the Permission Policy "features". There is only overlap in a small number of cases (e.g., geolocation, notification, push).

I might change it back to the longer name, to make it more clear.

@marcoscaceres
Copy link
Member Author

Filed gecko bug https://bugzilla.mozilla.org/show_bug.cgi?id=1737750 and sent patch.

@equalsJeffH
Copy link

equalsJeffH commented Dec 2, 2021

Please see also my belated comment on PR #264 (relevant portion copied here for convenience):

WRT the text this PR #264 added to the Permissions spec, it seems to be only implied that where a given feature is both a powerful feature, and is also subject to permissions policy, that the feature's "policy-controlled feature token" (as defined in the feature's specification), and in the Permissions spec's PermissionName enum (aka "Powerful features registry"), can be the same, but (I am guessing) do not have to be the same. Though, I am guessing that the intent is for them to be the same (I cannot quickly find an example where they are not the same). In any case, whatever the requirement is, it ought to be stated explicitly.

E.g., the strings "camera" and "microphone" are both defined as powerful feature names and are also declared as policy-controlled feature tokens (aka "strings").

Additionally, I find this statement in the text this PR added confusing:

The APIs and features in scope for the Permissions Policy specification go beyond those identified in this specification's PermissionName enum (e.g., "sync-xhr" and "gamepad").

...because: neither "sync-xhr" nor "gamepad" are permission names (i.e., enum values within PermissionName) -- so it is not obviously clear what using those policy-controlled feature tokens (aka "strings") as examples is intended to mean. Clarification would be good.

@marcoscaceres
Copy link
Member Author

Superseded by #353

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants