-
Notifications
You must be signed in to change notification settings - Fork 32
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
Drop duplicate PermissionState definition #88
Conversation
@reillyeon your check mark is green, so I guess you have the power to merge? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, looking at this again I realized there was more to fix to make this change.
Is there something that prevents from merging this? |
There's an unresolved comment regarding some additional specification text that should be updated to keep things consistent. |
@reillyeon following https://www.w3.org/2022/09/15-dap-minutes.html I wonder if this can be merged as-is? This would allow us to get rid of the duplicate PermissionState enum (WebKit even has a PermissionState.idl now), which is the biggest issue. We can replace "default" with "prompt" in another PR (the term is not exposed to script anyway), and file an issue to discuss further integration with the Permissions spec, as I imagine that will be a lot more work from a spec perspective. |
@rakuco I took another careful look at this and I think that even though the |
Thanks for reviewing it again! Can either you or @anssiko merge the PR? I don't have commit rights to this repository. |
SHA: 7e1e1d9 Reason: push, by anssiko Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Fixes #82.
Preview | Diff