search

w3c / deviceorientation

W3C Device Orientation spec
https://www.w3.org/TR/orientation-event/
Other
49 stars 32 forks source link

Drop duplicate PermissionState definition #88

Closed foolip closed 1 year ago

foolip commented 4 years ago

Fixes https://github.com/w3c/deviceorientation/issues/82.

Preview | Diff

rakuco commented 3 years ago

Is there anything preventing this from being merged? I recall some long discussions about permissions in #57 and #68. WebKit does not define PermissionState in other IDLs, but I guess they can just continue shipping their enum in DeviceOrientationOrMotionPermissionState.idl.

foolip commented 3 years ago

@reillyeon your check mark is green, so I guess you have the power to merge?

FluorescentHallucinogen commented 2 years ago

Is there something that prevents from merging this?

reillyeon commented 2 years ago

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.

rakuco commented 1 year ago

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

reillyeon commented 1 year ago

@rakuco I took another careful look at this and I think that even though the "default" value isn't in the PermissionState enum from the Permissions API the way that it is used in this spec doesn't technically expose that to script and so it's fine to land this change as is. We can fix it up in a follow-up which adds proper Permissions API integration and so removes all the existing spec text that uses the "default" value.

rakuco commented 1 year ago

Thanks for reviewing it again! Can either you or @anssiko merge the PR? I don't have commit rights to this repository.