-
Notifications
You must be signed in to change notification settings - Fork 394
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
Use permissions API for XR feature requests #900
Changes from 3 commits
ee370bd
c69936c
82495dc
6914665
e980903
68670dc
13babff
8a4a443
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,6 +23,9 @@ Abstract: This specification describes support for accessing virtual reality (VR | |
<pre class="link-defaults"> | ||
spec:infra; | ||
type:dfn; text:string | ||
type:dfn; text:tuple | ||
spec:permissions-1; | ||
type:dfn; text:powerful feature | ||
</pre> | ||
|
||
<pre class="anchors"> | ||
|
@@ -346,8 +349,10 @@ When this method is invoked, the user agent MUST run the following steps: | |
1. Abort these steps. | ||
1. Let |session| be a new {{XRSession}} object. | ||
1. [=Initialize the session=] with |session|, |mode|, and |device|. | ||
1. [=Resolve the requested features=] given by |options|' {{XRSessionInit/requiredFeatures}} and |options|' {{XRSessionInit/optionalFeatures}} values for |session|, and let |resolved| be the returned value. | ||
1. If |resolved| is <code>false</code>, run the following steps: | ||
1. Let |descriptor| be an {{XRPermissionDescriptor}} initialized with |session|, |options|' {{XRSessionInit/requiredFeatures}}, and |options|' {{XRSessionInit/optionalFeatures}}. | ||
1. Let |status| be an {{XRPermissionStatus}}, initially <code>null</code> | ||
1. [=Request the xr permission=] with |descriptor| and |status|. | ||
1. If |status|' {{PermissionStatus/state}} is {{PermissionState/"denied"}} run the following steps: | ||
1. [=Reject=] |promise| with a "{{NotSupportedError}}" {{DOMException}}. | ||
1. If |immersive| is <code>true</code>, set [=pending immersive session=] to <code>false</code>. | ||
1. Abort these steps. | ||
|
@@ -496,36 +501,6 @@ Note: {{XRReferenceSpaceType/"local"}} is always included in the [=requested fea | |
|
||
Note: For example, several VR devices support either configuring a safe boundary for the user to move around within or skipping boundary configuration and operating in a mode where the user is expected to stand in place. Such a device can be considered to be [=capable of supporting=] {{"bounded-floor"}} {{XRReferenceSpace}}s even if they are currently not configured with safety boundaries, because it's expected that the user could configure the device appropriately if the experience required it. This is to allow user agents to avoid fully initializing the [=XRSession/XR device=] or waiting for the user's environment to be recognized prior to [=resolve the requested features|resolving the requested features=] if desired. If, however, the user agent knows that the boundary state at the time the session is requested without additional initialization it may choose to reject the {{"bounded-floor"}} feature if the safety boundary not already configured. | ||
|
||
<div class="algorithm" data-algorithm="resolve-features"> | ||
|
||
To <dfn>resolve the requested features</dfn> given |requiredFeatures| and |optionalFeatures| for an {{XRSession}} |session|, the user agent MUST run the following steps: | ||
|
||
1. Let |consentRequired| be an empty [=/list=] of {{DOMString}}. | ||
1. Let |consentOptional| be an empty [=/list=] of {{DOMString}}. | ||
1. Add every [=feature descriptor=] in the [=default features=] table associated with |session|'s [=XRSession/mode=] to the indicated feature list if it is not already present. | ||
1. For each |feature| in |requiredFeatures| perform the following steps: | ||
1. If |feature| is not a valid [=feature descriptor=], return <code>false</code>. | ||
1. If the requesting document's origin is not allowed to use any [[#feature-policy|feature policy]] required by |feature| as indicated by the [=feature requirements=] table, return <code>false</code>. | ||
1. If |session|'s [=XRSession/XR device=] is not [=capable of supporting=] the functionality described by |feature| or the user agent has otherwise determined to reject the feature, return <code>false</code>. | ||
1. If the functionality described by |feature| requires [=explicit consent=], append it to |consentRequired|. | ||
1. Else append |feature| to |session|'s [=list of enabled features=]. | ||
1. For each |feature| in |optionalFeatures| perform the following steps: | ||
1. If |feature| is not a valid [=feature descriptor=], continue to the next entry. | ||
1. If the requesting document's origin is not allowed to use any [[#feature-policy|feature policy]] required by |feature| as indicated by the [=feature requirements=] table, continue to the next entry. | ||
1. If |session|'s [=XRSession/XR device=] is not [=capable of supporting=] the functionality described by |feature| or the user agent has otherwise determined to reject the feature, continue to the next entry. | ||
1. If the functionality described by |feature| requires [=explicit consent=], append it to |consentOptional|. | ||
1. Else append |feature| to |session|'s [=list of enabled features=]. | ||
1. If |consentRequired| or |consentOptional| are not empty, request [=explicit consent=] to use the functionality described by those features. | ||
1. For each |feature| in |consentRequired| perform the following steps: | ||
1. If a clear signal of [=user intent=] to enable |feature| has not been given, return <code>false</code>. | ||
1. Else append |feature| to |session|'s [=list of enabled features=]. | ||
1. For each |feature| in |consentOptional| perform the following steps: | ||
1. If a clear signal of [=user intent=] to enable |feature| has not been given, continue to the next entry. | ||
1. Else append |feature| to |session|'s [=list of enabled features=]. | ||
1. Return <code>true</code> | ||
|
||
</div> | ||
|
||
Session {#session} | ||
======= | ||
|
||
|
@@ -2334,6 +2309,118 @@ The [=default allowlist=] for this feature is <code>["self"]</code>. | |
|
||
Note: If the document's origin is not allowed to use the <code>"xr-spatial-tracking"</code> feature policy any [=immersive sessions=] will be blocked, because all [=immersive sessions=] require some use of spatial tracking. [=Inline sessions=] will still be allowed, but restricted to only using the {{XRReferenceSpaceType/"viewer"}} {{XRReferenceSpace}}. | ||
|
||
|
||
Permissions API Integration {#permissions} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't this section have a disclaimer that it's temporary and ought to move to the Permissions API? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure that's the case? E.g. web bluetooth doesn't have this disclaimer, and it would be really weird to upstream all of this machinery to that API I think. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. https://w3c.github.io/permissions/#permission-registry is here exactly for that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, okay. |
||
--------- | ||
|
||
The [[!permissions]] API provides a uniform way for websites to request permissions from users and query which permissions they have. | ||
NellWaliczek marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
The <dfn enum-value for="PermissionName">"xr"</dfn> [=powerful feature=]’s permission-related algorithms and types are defined as follows: | ||
|
||
|
||
<dl> | ||
<dt>[=permission descriptor type=]</dt> | ||
<dd> | ||
|
||
<pre class="idl"> | ||
dictionary XRPermissionDescriptor: PermissionDescriptor { | ||
XRSession session; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems odd to me that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's because there's no user-exposed handle for the "device" except for the session itself. The permissions integration needs to check against a device, and also needs to be able to deal with the device being different -- e.g. if you plug in a device that supports bounded-floor spaces, you want the permissions check to work. We could replace this with a mode enum, and use the "current XR device" instead, setting the list of granted features on the device instead of the session. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why does the permission need to be checked against a device? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Some devices don't support a feature, period.This is the existing behavior of XR features, I'm wary about changing how this works without broader agreement. |
||
sequence<any> requiredFeatures; | ||
sequence<any> optionalFeatures; | ||
Manishearth marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}; | ||
</pre> | ||
|
||
where {{PermissionDescriptor/name}} is {{PermissionName/"xr"}}. | ||
NellWaliczek marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
|
||
</dd> | ||
|
||
<dt>[=permission result type=]</dt> | ||
<dd> | ||
<pre class=idl> | ||
dictionary XRPermissionStatus: PermissionStatus { | ||
sequence<any> granted; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: I would call this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Personally I find There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. cc @toji opinions on naming? |
||
}; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This said, I wonder if we could delay this addition. Are there strong use cases for this? It would require implementations to be able to provide the list of enabled features without creating the session which may or may not an easy thing to do. If we don't have a strong use case, keeping the change small for now may be good? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I don't understand why? The spec already requires you to compute which features have been granted when you requestSession(), this just exposes that list. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ping about this. I think dropping the attribute entirely would be better. The only benefit of returning the list of features is to tell which optional features are granted. It doesn't seem critical and if someone really cares, they shouldn't have them as optional in the first place. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, this was part of the point of this change, see #794 . It's important for apps to know what optional features were granted. So yeah, there was a strong use case, this was one of the main reasons we wanted to make this pull request. |
||
</pre> | ||
|
||
</dd> | ||
|
||
|
||
<dt>[=permission request algorithm=]</dt> | ||
<dd> | ||
<div class=algorithm data-algorithm="xr-permission-request-algorithm"> | ||
To <dfn lt="request the xr permission">request the "xr" permission</dfn> with an {{XRPermissionDescriptor}} |descriptor| and a {{XRPermissionStatus}} |status|, the UA MUST run the following steps: | ||
|
||
1. Let |result| be the result of [=resolve the requested features|resolving the requested features=] given |descriptor|'s {{XRPermissionDescriptor/requiredFeatures}}, {{XRPermissionDescriptor/optionalFeatures}}, and {{XRPermissionDescriptor/session}}. | ||
1. If |result| is <code>null</code>, run the following steps: | ||
1. Set |status|'s {{XRPermissionStatus/granted}} to an empty {{FrozenArray}}. | ||
1. Set |status|'s {{PermissionStatus/state}} to {{PermissionState/"denied"}}. | ||
1. Abort these steps. | ||
1. Let <code>(|consentRequired|, |consentOptional|, |granted|)</code> be the fields of |result|. | ||
1. For each |feature| in |consentRequired| perform the following steps: | ||
1. If a clear signal of [=user intent=] to enable |feature| has not been given, set |status|'s {{PermissionStatus/state}} to {{PermissionState/"denied"}} and abort these steps. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This may just be me not groking the permissions flow, but it's not clear to me where in this algorithm the user intent should be prompted? [=resolve the requested features=] identifies what features need consent, and then this fails if they don't have consent, and it seems like there's not an opportunity in between to actually prompt for it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "Clear signal of user intent" is when they can be prompted, but that signal can also be UA preferences or something else. We could perhaps firm up the language around that, maybe adding a definition for "clear signal of user intent" that includes something about prompting. The permissions spec itself is rather wishy washy about prompting as well. FWIW "clear signal of user intent" was copied over from the old feature request code. I wasn't quite sure what to do for prompting so I left it as is. [=Resolve the requested features=] is about filtering which features need to actually be requested.
NellWaliczek marked this conversation as resolved.
Show resolved
Hide resolved
NellWaliczek marked this conversation as resolved.
Show resolved
Hide resolved
NellWaliczek marked this conversation as resolved.
Show resolved
Hide resolved
|
||
1. If |feature| is not in |granted|, append |feature| to |granted|. | ||
1. For each |feature| in |consentOptional| perform the following steps: | ||
1. If a clear signal of [=user intent=] to enable |feature| has not been given, continue to the next entry. | ||
1. If |feature| is not in |granted|, append |feature| to |granted|. | ||
1. Set |status|'s {{XRPermissionStatus/granted}} to |granted|. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should the session's [=list of enabled features=] be updated to |granted| here? I know that happens above, but that's only for the requestSession() method, while this could also be invoked by the Permissions.request() method. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good point |
||
1. Set |session|'s [=list of enabled features=] to |granted|. | ||
1. Set |status|'s {{PermissionStatus/state}} to {{PermissionState/"granted"}}. | ||
|
||
Note: The user agent may choose to batch up permissions prompts for all requested features when gauging if there is a clear signal of [=user intent=]. | ||
|
||
</div> | ||
|
||
|
||
<dt>[=permission query algorithm=]</dt> | ||
<dd> | ||
<div class=algorithm data-algorithm="xr-permission-query-algorithm"> | ||
To query the "xr" permission with an {{XRPermissionDescriptor}} |descriptor| and a {{XRPermissionStatus}} |status|, the UA MUST run the following steps: | ||
|
||
1. Set |status|'s {{PermissionStatus/state}} to |descriptor|'s [=permission state=]. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll admit that I don't really get what this line is doing, but I also see that other permissions do something similar so I'm going to assume it's a good thing. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, this confuses me too. Is there someone we can ask to validate this isn't an obsolete thing to be doing? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is basically the base algorithm to read the permission state. This is entirely fine to do. However, the way it's described in the spec makes it unclear to me whether this is taking into account the optional and required features. Unfortunately, I'm running out of time to review this :( |
||
1. If |status|'s {{PermissionStatus/state}} is {{PermissionState/"denied"}}, set |status|'s {{XRPermissionStatus/granted}} to an empty {{FrozenArray}} and abort these steps. | ||
1. Let |result| be the result of [=resolve the requested features|resolving the requested features=] given |descriptor|'s {{XRPermissionDescriptor/requiredFeatures}}, {{XRPermissionDescriptor/optionalFeatures}}, and {{XRPermissionDescriptor/session}}. | ||
1. If |result| is <code>null</code>, run the following steps: | ||
1. Set |status|'s {{XRPermissionStatus/granted}} to an empty {{FrozenArray}}. | ||
1. Set |status|'s {{PermissionStatus/state}} to {{PermissionState/"denied"}}. | ||
1. Abort these steps. | ||
1. Let <code>(|consentRequired|, |consentOptional|, |granted|)</code> be the fields of |result|. | ||
1. Set |status|'s {{XRPermissionStatus/granted}} to |granted|. | ||
1. If |consentRequired| [=list/is empty=] and |consentOptional| [=list/is empty=], set |status|'s {{PermissionStatus/state}} to {{PermissionState/"granted"}} and abort these steps | ||
1. Set |status|'s {{PermissionStatus/state}} to {{PermissionState/"prompt"}}. | ||
|
||
</div> | ||
|
||
</dd> | ||
|
||
</dl> | ||
|
||
<div class="algorithm" data-algorithm="resolve-features"> | ||
|
||
To <dfn>resolve the requested features</dfn> given |requiredFeatures| and |optionalFeatures| for an {{XRSession}} |session|, the user agent MUST run the following steps: | ||
|
||
1. Let |consentRequired| be an empty [=/list=] of {{DOMString}}. | ||
1. Let |consentOptional| be an empty [=/list=] of {{DOMString}}. | ||
1. Let |granted| be a [=/list=] of {{DOMString}} initialized to |session|'s [=list of enabled features=]. | ||
1. Add every [=feature descriptor=] in the [=default features=] table associated with |session|'s [=XRSession/mode=] to the indicated feature list if it is not already present. | ||
1. For each |feature| in |requiredFeatures| perform the following steps: | ||
1. If |feature| is not a valid [=feature descriptor=], return <code>null</code>. | ||
1. If |feature| is already in |granted|, continue to the next entry. | ||
1. If the requesting document's origin is not allowed to use any [[#feature-policy|feature policy]] required by |feature| as indicated by the [=feature requirements=] table, return <code>null</code>. | ||
1. If |session|'s [=XRSession/XR device=] is not [=capable of supporting=] the functionality described by |feature| or the user agent has otherwise determined to reject the feature, return <code>null</code>. | ||
1. If the functionality described by |feature| requires [=explicit consent=], append it to |consentRequired|. | ||
1. Else append |feature| to |granted|. | ||
1. For each |feature| in |optionalFeatures| perform the following steps: | ||
1. If |feature| is not a valid [=feature descriptor=], continue to the next entry. | ||
1. If |feature| is already in |granted|, continue to the next entry. | ||
1. If the requesting document's origin is not allowed to use any [[#feature-policy|feature policy]] required by |feature| as indicated by the [=feature requirements=] table, continue to the next entry. | ||
1. If |session|'s [=XRSession/XR device=] is not [=capable of supporting=] the functionality described by |feature| or the user agent has otherwise determined to reject the feature, continue to the next entry. | ||
1. If the functionality described by |feature| requires [=explicit consent=], append it to |consentOptional|. | ||
1. Else append |feature| to |granted|. | ||
1. Return the [=tuple=] <code>(|consentRequired|, |consentOptional|, |granted|)</code> | ||
|
||
</div> | ||
|
||
Acknowledgements {#ack} | ||
================ | ||
|
||
|
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.
It seems that "Request the xr permission" could return "status" instead of having it as in in/out parameter.
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.
It can't, that's how the permission request algorithm works. I agree that it is awkward, but that's a problem for upstream.
https://wicg.github.io/permissions-request/#permission-request-algorithm
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.
This is a different spec FWIW. You could re-use https://wicg.github.io/permissions-request/#boolean-permission-request-algorithm the same way https://w3c.github.io/permissions/#media-devices does.
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.
ping about this
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.
The media spec is using it incorrectly: it operates on the return value of the boolean permissions request algorithm, which does not exist, and it returns a value from its own permissions request algorithm, which according to the permissions spec should not exist. I'd be more than happy to clean this up but as written the permissions spec forces you to use an inout parameter. Perhaps it should be changed to use return values in many places so that it can be used in the way the media spec expects it to be.