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 Permissions API integration, start requiring requestPermission() usage #123

Merged
merged 3 commits into from
Dec 4, 2023

Conversation

rakuco
Copy link
Member

@rakuco rakuco commented Nov 14, 2023

This substantive and breaking change integrates the existing
requestPermission() calls with the Permissions API, so that we do not need
to essentially redefine the "request permission to use" algorithm here.

Additionally, calling requestPermission() is now a requirement, as
devicemotion, deviceorientation and deviceorientationabsolute events are
fired only when the permission state is "granted". This matches WebKit's
current behavior. Blink's plan is to follow suit in the near future.

The powerful feature names are identical to those proposed in #121:
"accelerometer", "gyroscope", and "magnetometer", which match the low-level
sensors that provide the data in the events fired by this specification.
These names also match those in the Accelerometer, Gyroscope and
Magnetometer specifications, which allows developers who want to transition
between these APIs to avoid having to request access to different powerful
feature names with the same goal.

Also similarly to #121, the idea is to:

  • Require "accelerometer" and "gyroscope" for the devicemotion event.
  • Require "accelerometer" and "gyroscope" to provide relative
    orientation data for the deviceorientation event, and additionally
    "magnetometer" to fall back to absolute orientation data.
  • Requires "accelerometer", "gyroscope", and "magnetometer" for the
    deviceorientationabsolute event.

DeviceOrientationEvent.requestPermission() now takes an optional absolute
argument (defaulting to false) to specify whether it will also request the
"magnetometer" permission.

IMPORTANT: As far as I can see, the WebKit implementation does not integrate
with the Permissions API. It therefore does not use the powerful feature
names described above, nor does support the new absolute argument or the
requesting of different permissions depending on whether developers want to
access to absolute orientation data.

Fixes #70.


Preview | Diff

…usage

This substantive and breaking change integrates the existing
requestPermission() calls with the Permissions API, so that we do not need
to essentially redefine the "request permission to use" algorithm here.

Additionally, calling requestPermission() is now a requirement, as
devicemotion, deviceorientation and deviceorientationabsolute events are
fired only when the permission state is "granted". This matches WebKit's
current behavior. Blink's plan is to follow suit in the near future.

The powerful feature names are identical to those proposed in #121:
"accelerometer", "gyroscope", and "magnetometer", which match the low-level
sensors that provide the data in the events fired by this specification.
These names also match those in the Accelerometer, Gyroscope and
Magnetometer specifications, which allows developers who want to transition
between these APIs to avoid having to request access to different powerful
feature names with the same goal.

Also similarly to #121, the idea is to:
- Require "accelerometer" and "gyroscope" for the devicemotion event.
- Require "accelerometer" and "gyroscope" to provide relative
  orientation data for the deviceorientation event, and additionally
  "magnetometer" to fall back to absolute orientation data.
- Requires "accelerometer", "gyroscope", and "magnetometer" for the
  deviceorientationabsolute event.

DeviceOrientationEvent.requestPermission() now takes an optional `absolute`
argument (defaulting to false) to specify whether it will also request the
"magnetometer" permission.

IMPORTANT: As far as I can see, the WebKit implementation does not integrate
with the Permissions API. It therefore does not use the powerful feature
names described above, nor does support the new `absolute` argument or the
requesting of different permissions depending on whether developers want to
access to absolute orientation data.

Fixes #70.
@rakuco
Copy link
Member Author

rakuco commented Nov 14, 2023

@reillyeon I've set this one as a draft because, like with #121, it's not clear to me whether these powerful feature names should be duplicated here or if we should use the definitions from the Accelerometer, Gyroscope and Magnetometer specs (which are already exported).

This also conflicts with #121 since https://w3c.github.io/permissions/#dfn-permission-state already performs Permissions Policy checks as well (TIL) so if this PR lands first the other one can be simplified a bit.

I've had to do some in parallel-queue a global task trickery that does not look great, but felt necessary.

The way the permission state checks were added to the parts that specify when/how to fire a device motion/orientation event are not pretty either, but I couldn't find a better way to also let event listeners added before permission is granted to start firing events once that happens (as discussed in #74).

@reillyeon
Copy link
Member

@reillyeon I've set this one as a draft because, like with #121, it's not clear to me whether these powerful feature names should be duplicated here or if we should use the definitions from the Accelerometer, Gyroscope and Magnetometer specs (which are already exported).

As mentioned on the other issue I'm leaning towards moving the definitions of the powerful features into this specification.

rakuco added a commit that referenced this pull request Nov 22, 2023
… types here

As suggested in #124 and similarly to what has been discussed in PRs #121
and #123, these definitions are shared by this specification and the
Accelerometer and Gyroscope specs.

Since this specification is further along the Rec track and is implemented
by more engines, it makes sense to have the exported definitions here and
reference them from the other specs instead.
Instead of doing that in the Accelerometer, Gyroscope and Magnetometer
specifications, export them here and reference the definitions from said
specs.

Doing so allows us not to depend on specifications that are less advanced in
the Rec track and have fewer implementations.
@rakuco rakuco marked this pull request as ready for review November 22, 2023 17:48
@rakuco
Copy link
Member Author

rakuco commented Nov 22, 2023

As mentioned on the other issue I'm leaning towards moving the definitions of the powerful features into this specification.

Done, PTAL.

index.bs Show resolved Hide resolved
@rakuco
Copy link
Member Author

rakuco commented Nov 30, 2023

@reillyeon thanks for the review. I also have the accelerometer/gyroscope/magnetometer updates ready, but I'm deferring the merging to you or @anssiko in case you'd like to discuss this (and #121) with the WebApps WG first.

@anssiko
Copy link
Member

anssiko commented Dec 4, 2023

Thank you @rakuco @reillyeon for championing this specification.

Based on my assessment, with these changes and changes in #121 we are ready to seek horizontal review from the wider W3C community ahead of our expected CR publication.

@anssiko anssiko merged commit b03d5fb into main Dec 4, 2023
2 checks passed
github-actions bot added a commit that referenced this pull request Dec 4, 2023
SHA: b03d5fb
Reason: push, by anssiko

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@rakuco rakuco deleted the permissions-api-integration branch December 4, 2023 14:49
@rakuco
Copy link
Member Author

rakuco commented Dec 5, 2023

Based on my assessment, with these changes and changes in #121 we are ready to seek horizontal review from the wider W3C community ahead of our expected CR publication.

FWIW, I'd like to fix #91 before attempting to move to CR, but I'm not sure if this should block any wide review requests.

@anssiko
Copy link
Member

anssiko commented Dec 5, 2023

Correct, #91 is not a blocker for the initiation of the wide review, but should be addressed by CR.

Given the substantive part of the changes since the last CR have been to strengthen privacy protections, it would be beneficial to update the latest status to the two privacy-tracker Group bringing to attention of Privacy, or tracked by the Privacy Group but not needing response. issues. I'm expecting PING to ask about the status of these two in their review.

In preparation for the expected CR publication, I pushed a PR with a human-readable summary of changes since the last CR to help guide wide review: #125

rakuco added a commit that referenced this pull request Dec 5, 2023
This PR integrates with the automation concepts defined in
https://w3c.github.io/sensors/#automation to allow providing motion or
orientation readings via virtual sensors through the WebDriver extension
commands defined there.

IMPORTANT: This does not mean that this specification requires
implementations to support the Generic Sensor API specification and its
derived specifications. Only the Automation section is being referenced, and
when necessary some algorithms and definitions are being duplicated here,
especially for Device Motion automation.

The definitions of the "accelerometer", "linear-acceleration", "gyroscope",
"absolute-orientation", and "relative-orientation" virtual sensor types have
been moved here from their original specifications. As suggested in #124 and
similarly to what has been discussed in PRs #121 and #123, since this
specification is further along the Rec track and is implemented by more
engines, it makes sense to have the exported definitions here and reference
them from the other specs instead.

Device Motion:
- Readings are controlled via the "accelerometer", "linear-acceleration" and
  "gyroscope" virtual sensor types.

Device Orientation:
- The "absolute-orientation" and "relative-orientation" virtual sensor types
  are defined in this specification, along with a parsing algorithm that
  reads alpha, beta and gamma doubles.
  Compared to device motion, however, we have an additional problem in that
  device orientation data is provided in Euler angles and Orientation Sensor
  uses quaternions.
  The idea is to parse readings by reading alpha/beta/gamma keys from the
  WebDriver extension command and use them in the "fire orientation event"
  algorithm and skip using quaternions altogether. The Orientation Sensor
  spec can then augment the "parse orientation data reading" algorithm with
  the required steps to both 1) accept a "quaternion" key in the WebDriver
  extension command (and convert it to Euler angles as well) and 2) derive a
  quaternion from the alpha/beta/gamma Euler angles.

Fixes #122.
rakuco added a commit that referenced this pull request Jan 4, 2024
This PR integrates with the automation concepts defined in
https://w3c.github.io/sensors/#automation to allow providing motion or
orientation readings via virtual sensors through the WebDriver extension
commands defined there.

IMPORTANT: This does not mean that this specification requires
implementations to support the Generic Sensor API specification and its
derived specifications. Only the Automation section is being referenced, and
when necessary some algorithms and definitions are being duplicated here,
especially for Device Motion automation.

The definitions of the "accelerometer", "linear-acceleration", "gyroscope",
"absolute-orientation", and "relative-orientation" virtual sensor types have
been moved here from their original specifications. As suggested in #124 and
similarly to what has been discussed in PRs #121 and #123, since this
specification is further along the Rec track and is implemented by more
engines, it makes sense to have the exported definitions here and reference
them from the other specs instead.

Device Motion:
- Readings are controlled via the "accelerometer", "linear-acceleration" and
  "gyroscope" virtual sensor types.

Device Orientation:
- The "absolute-orientation" and "relative-orientation" virtual sensor types
  are defined in this specification, along with a parsing algorithm that
  reads alpha, beta and gamma doubles.
  Compared to device motion, however, we have an additional problem in that
  device orientation data is provided in Euler angles and Orientation Sensor
  uses quaternions.
  The idea is to parse readings by reading alpha/beta/gamma keys from the
  WebDriver extension command and use them in the "fire orientation event"
  algorithm and skip using quaternions altogether. The Orientation Sensor
  spec can then augment the "parse orientation data reading" algorithm with
  the required steps to both 1) accept a "quaternion" key in the WebDriver
  extension command (and convert it to Euler angles as well) and 2) derive a
  quaternion from the alpha/beta/gamma Euler angles.

Fixes #122.
rakuco added a commit to rakuco/accelerometer that referenced this pull request Jan 4, 2024
…RIENTATION

Adapt to w3c/deviceorientation#121 and w3c/deviceorientation#123.

Those two PRs have moved the definition of the "acceleromter"
permission name and permissions policy token to the Device Orientation
spec, which is more widely implemented than this specification and can
be referenced from this one.
rakuco added a commit to rakuco/accelerometer that referenced this pull request Jan 4, 2024
…RIENTATION

Adapt to w3c/deviceorientation#121 and w3c/deviceorientation#123.

Those two PRs have moved the definition of the "accelerometer"
permission name and permissions policy token to the Device Orientation
spec, which is more widely implemented than this specification and can
be referenced from this one.
rakuco added a commit to rakuco/accelerometer that referenced this pull request Jan 4, 2024
…RIENTATION

Adapt to w3c/deviceorientation#121 and w3c/deviceorientation#123.

Those two PRs have moved the definition of the "accelerometer"
permission name and permissions policy token to the Device Orientation
spec, which is more widely implemented than this specification and can
be referenced from this one.
rakuco added a commit that referenced this pull request Jan 4, 2024
This PR integrates with the automation concepts defined in
https://w3c.github.io/sensors/#automation to allow providing motion or
orientation readings via virtual sensors through the WebDriver extension
commands defined there.

IMPORTANT: This does not mean that this specification requires
implementations to support the Generic Sensor API specification and its
derived specifications. Only the Automation section is being referenced, and
when necessary some algorithms and definitions are being duplicated here,
especially for Device Motion automation.

The definitions of the "accelerometer", "linear-acceleration", "gyroscope",
"absolute-orientation", and "relative-orientation" virtual sensor types have
been moved here from their original specifications. As suggested in #124 and
similarly to what has been discussed in PRs #121 and #123, since this
specification is further along the Rec track and is implemented by more
engines, it makes sense to have the exported definitions here and reference
them from the other specs instead.

Device Motion:
- Readings are controlled via the "accelerometer", "linear-acceleration" and
  "gyroscope" virtual sensor types.

Device Orientation:
- The "absolute-orientation" and "relative-orientation" virtual sensor types
  are defined in this specification, along with a parsing algorithm that
  reads alpha, beta and gamma doubles.
  Compared to device motion, however, we have an additional problem in that
  device orientation data is provided in Euler angles and Orientation Sensor
  uses quaternions.
  The idea is to parse readings by reading alpha/beta/gamma keys from the
  WebDriver extension command and use them in the "fire orientation event"
  algorithm and skip using quaternions altogether. The Orientation Sensor
  spec can then augment the "parse orientation data reading" algorithm with
  the required steps to both 1) accept a "quaternion" key in the WebDriver
  extension command (and convert it to Euler angles as well) and 2) derive a
  quaternion from the alpha/beta/gamma Euler angles.

Fixes #122.
rakuco added a commit to rakuco/gyroscope that referenced this pull request Jan 4, 2024
…RIENTATION

Adapt to w3c/deviceorientation#121 and w3c/deviceorientation#123.

Those two PRs have moved the definition of the "gyroscope"
permission name and permissions policy token to the Device Orientation
spec, which is more widely implemented than this specification and can
be referenced from this one.
anssiko pushed a commit to w3c/gyroscope that referenced this pull request Jan 4, 2024
…RIENTATION (#59)

Adapt to w3c/deviceorientation#121 and w3c/deviceorientation#123.

Those two PRs have moved the definition of the "gyroscope"
permission name and permissions policy token to the Device Orientation
spec, which is more widely implemented than this specification and can
be referenced from this one.
anssiko pushed a commit to w3c/accelerometer that referenced this pull request Jan 4, 2024
…RIENTATION (#75)

Adapt to w3c/deviceorientation#121 and w3c/deviceorientation#123.

Those two PRs have moved the definition of the "accelerometer"
permission name and permissions policy token to the Device Orientation
spec, which is more widely implemented than this specification and can
be referenced from this one.
rakuco added a commit that referenced this pull request Jan 10, 2024
The changes added in #123 have introduced some behavior changes to the two
requestPermission() operations that were not originally intended.

Although the original steps were heavily modeled after the WebKit
implementation and do not follow what other specifications normally do, some
behavior changes from #123 have not been discussed and have been made more
backwards-compatible:

- Calls to "request permission to use" may return "prompt", while the
  previous version implicitly converted anything that was not "granted" to
  "denied" (possibly because the current Safari implementation shows a modal
  dialog asking for access, so users can only allow or deny the permission
  request, so remaining at the "prompt" state is not possible).
  From a specification perspective, if a user has not explicitly granted the
  permission request, "prompt" is essentially the same as "denied" anyway,
  so we now explicitly set anything that is not "granted" to "denied" again.
- The previous steps only checked for transient activation if users had not
  made an explicit permission choice yet, which basically means that the
  permission state is set to "prompt".
  This change restores this behavior by first checking the current
  permission states for all required permission names and only throwing a
  NotAllowed exception if one of them is "prompt" and there is no transient
  activation. In other words, if a permission is either "granted" or
  "denied" then requestPermission() will resolve to one of the values
  without requiring transient activation.

While here, refer to the right definitions of "granted" and "denied": use
the PermissionState enum definitions, which is what the Permissions API
algorithms we invoke return, rather than the non-Web IDL definitions that
these enum values correspond to.
rakuco added a commit that referenced this pull request Jan 22, 2024
The changes added in #123 have introduced some behavior changes to the two
requestPermission() operations that were not originally intended.

Although the original steps were heavily modeled after the WebKit
implementation and do not follow what other specifications normally do, some
behavior changes from #123 have not been discussed and have been made more
backwards-compatible:

- Calls to "request permission to use" may return "prompt", while the
  previous version implicitly converted anything that was not "granted" to
  "denied" (possibly because the current Safari implementation shows a modal
  dialog asking for access, so users can only allow or deny the permission
  request, so remaining at the "prompt" state is not possible).
  From a specification perspective, if a user has not explicitly granted the
  permission request, "prompt" is essentially the same as "denied" anyway,
  so we now explicitly set anything that is not "granted" to "denied" again.
- The previous steps only checked for transient activation if users had not
  made an explicit permission choice yet, which basically means that the
  permission state is set to "prompt".
  This change restores this behavior by first checking the current
  permission states for all required permission names and only throwing a
  NotAllowed exception if one of them is "prompt" and there is no transient
  activation. In other words, if a permission is either "granted" or
  "denied" then requestPermission() will resolve to one of the values
  without requiring transient activation.

While here, refer to the right definitions of "granted" and "denied": use
the PermissionState enum definitions, which is what the Permissions API
algorithms we invoke return, rather than the non-Web IDL definitions that
these enum values correspond to.
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.

Add integration with the Permissions API
3 participants