-
Notifications
You must be signed in to change notification settings - Fork 385
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
Should we Remove XRPermissionStatus? #1305
Comments
Discussed this in the meeting today; it sounds like my assertion that no one was using this was wrong and that Meta may be doing so. However, they answer for a subset of permissions that may or may not be defined in the spec, and may call for a re-write/proposal to be explicit about the permissions that we support? (To my recollection it sounded like they may integrate "xr", "hand-tracking" and "room-information" permissions into this, while only "xr" is mentioned in the spec at present). @cabanier will follow up to determine if it is this integration that they've implemented, and we will then discuss either re-writing or removing this section next week depending on how they've integrated with it. |
For full clarity, this is the portion of our spec that I'm discussing: https://immersive-web.github.io/webxr/#permissions (Currently 14.2) |
Regardless of Rik's findings; I believe we need to move the "permission request algorithm" portion of the spec; since that is no longer supported by the Permissions API, though it is directly referenced from our "request a session" algorithm. |
I agree we can remove this if this API is not going to be implemented. |
Yes, we integrated this with the permission manager. Specifically, we added the "xr", "xr-hands" and "xr-planes" permission strings. We did this so we could use the regular permission workflow that is used for other sensitive data such as microphone. |
As it stands today then it also sounds like you ignore our current spec of parsing through and replying with the state of requested/optional features? You only take a generic PermissionDescriptor with those three names? (Two of which are currently unspecced)?
NIT: s/remove/move - The algorithm is currently referenced by the "Request Session" algorithm and thus cannot be fully removed from the spec. |
I think that part is unchanged. Features like layers or anchors don't require extra permissions so they didn't get one. |
The spec as written currently requires parsing through and listing if all required/optional features are enabled or not, the "permission query algorithm" (Can't link since it's a link to the permissions spec, but it's step 3 there), says to "Resolve the requested features", which uses our existing notion of requested features (of which xr-hands and xr-planes are not ones, but "planes" and "hand-tracking" and e.g. "layers" are). If I understand correctly, it sounds like you've defined three "powerful features" ("xr", "xr-hands", "xr-planes"), and follow the more general default query algorithm, and not the more specific "query" algorithm defined in our spec. I think the notion of if we add additional "powerful features" other than "xr" is something that we should discuss separate from this issue, as it ties some UA flexibility down with regards to how we handle permissions for specific features. |
XRPermissionStatus (and the integration with the Permissions API) is something that has been in the core spec for a little while; however, to my knowledge no one has currently implemented it. Indeed, the way that we actually do include it seems to be in direct contradiction to the Permissions spec we purport to integrate with:
After speaking with @toji, he mentioned that one of the original motivations for this integration was for the “Permissions.request()” API to be able to prompt for features mid session; this API method is no longer available.
The API also doesn’t require a user gesture to the best of my knowledge, and the algorithm as written seems to allow a site to enumerate all features and determine exactly which features the current device supports vs. which features a user has granted, and so may allow for a fairly high degree of fingerprinting.
Given that no one has currently implemented this integration, and to my knowledge do not have plans to, I’m wondering if we should remove this portion of the Spec?
/agenda because I have a feeling this will be best discussed in person 🙂
The text was updated successfully, but these errors were encountered: