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

Add support for permissions that have an associated Feature Policy #163

Merged
merged 2 commits into from Sep 7, 2018

Conversation

raymeskhoury
Copy link
Collaborator

@raymeskhoury raymeskhoury commented Oct 10, 2017

This adds support for denying access to permissions which are not
allowed in the current context based on an associated Feature Policy.
The Feature Name and Default Allowlist of a policy-controlled feature
must still be declared in the owning spec. This also removes specific
support for the media feature policy which is covered by the more
general support.


Preview | Diff

This adds support for denying access to permissions which are not
allowed in the current context based on an associated Feature Policy.
The Feature Name and Default Allowlist of a policy-controlled feature
must still be declared in the owning spec. This also removes specific
support for the media feature policy which is covered by the more
general support.
@raymeskhoury
Copy link
Collaborator Author

@jyasskin ptal :)

@raymeskhoury
Copy link
Collaborator Author

raymeskhoury commented Oct 10, 2017

Btw, I'm still getting a bikeshed error with top-of-tree:
FATAL ERROR: IDL SYNTAX ERROR LINE: 1 - skipped: "PermissionName"
✘ Did not generate, due to fatal errors
make: *** [index.html] Error 1

Removing highlight='idl' from <pre highlight='idl' link-for="PermissionName"> seems to fix it but I don't know if I'm doing something wrong.

@jan-ivar
Copy link
Member

jan-ivar commented Mar 8, 2018

@raymeskhoury Any progress here?

@raymeskhoury
Copy link
Collaborator Author

@jyasskin are you ok to merge this?

Copy link
Member

@jyasskin jyasskin left a comment

Choose a reason for hiding this comment

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

The change looks good to me, with an update to match Feature Policy changes since you wrote it. I'm sorry for taking so long to look at this.

index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
@jan-ivar
Copy link
Member

jan-ivar commented Aug 7, 2018

@raymeskhoury Can this be merged?

@raymeskhoury
Copy link
Collaborator Author

@jan-ivar sorry for the long delay while I was distracted with other projects.

@jyasskin please see the updated PR.

@raymeskhoury
Copy link
Collaborator Author

raymeskhoury commented Sep 7, 2018

@marcoscaceres: jyasskin is on leave. He's approved this and I've addressed his minor comments and rebased. Are you able to merge it? Thanks!

@marcoscaceres marcoscaceres merged commit 79f9c4e into w3c:master Sep 7, 2018
@jan-ivar
Copy link
Member

Thanks @marcoscaceres! Oddly, https://w3c.github.io/permissions/ still isn't reflecting github tip.

Does someone need to pull a crank or something?

@marcoscaceres
Copy link
Member

Hmm... Travis was supposed to run the deploy script. Might need to run it manually.

@marcoscaceres
Copy link
Member

manually pulled the crank https://w3c.github.io/permissions/

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.

None yet

4 participants