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 Policy integration #121

Merged
merged 2 commits into from
Dec 5, 2023
Merged

Conversation

rakuco
Copy link
Member

@rakuco rakuco commented Nov 10, 2023

This substantive change makes the firing of the events defined in this
specification dependent on a few different policy-controlled features:
"accelerometer", "gyroscope", and "magnetometer.

Both Blink and WebKit already integrate their implementations with the
Permissions Policy spec and use the same tokens, although a bit differently:

  • Both require "accelerometer" and "gyroscope" for the devicemotion event.
  • Blink requires "accelerometer" and "gyroscope" to provide relative
    orientation data for the deviceorientation event, and additionally the
    "magnetometer" feature to fall back to absolute orientation data. WebKit
    always requires the three tokens for the deviceorientation event.
  • Blink requires "accelerometer", "gyroscope", and "magnetometer" for the
    deviceorientationabsolute event, whereas WebKit does not implement this
    event.

We have opted to codify Blink's behavior in the specification, as since
relative orientation data does not require a magnetometer sensor, the event
should not require the corresponding token either.

Fixes #64.


Preview | Diff

@rakuco rakuco changed the base branch from main to editorial/section-division-more-dfns November 10, 2023 17:43
@rakuco
Copy link
Member Author

rakuco commented Nov 10, 2023

@reillyeon the Accelerometer, Gyroscope and Magnetometer specifications already define and export the same policy-controlled feature tokens with the same allowlists. I considered referencing them here, but opted for redefining them here given the concerns about depending on definitions from specs less far along the standards track. Please let me know what you think.

@rakuco rakuco changed the title permissions policy integration Add Permissions Policy integration Nov 10, 2023
index.bs Outdated Show resolved Hide resolved
index.bs Show resolved Hide resolved
@rakuco rakuco force-pushed the editorial/section-division-more-dfns branch from e188fd8 to f0ff533 Compare November 13, 2023 17:56
Base automatically changed from editorial/section-division-more-dfns to main November 13, 2023 17:58
@rakuco rakuco force-pushed the permissions-policy-integration branch from 4f1f7dc to 2c32680 Compare November 13, 2023 17:59
@reillyeon
Copy link
Member

@rakuco, when updating a PR please don't use a force push as it breaks the relationship between the original changes and the updated changes.

@rakuco
Copy link
Member Author

rakuco commented Nov 14, 2023

@rakuco, when updating a PR please don't use a force push as it breaks the relationship between the original changes and the updated changes.

Sorry. I think this was a problem specific to this PR, as I'd sent it targeting the branch I'd created for #120 since I needed those changes, and when it was merged with some suggested changes GitHub got confused.

@rakuco
Copy link
Member Author

rakuco commented Nov 14, 2023

Before merging, I'd just like to double check

  • That it's better to duplicate the "accelerometer", "gyroscope" and "magnetometer" tokens here instead of using those defined and exported by the respective specs.
  • Whether there needs to be any involvement from the WebApps WG since this is a substantive change.

rakuco added a commit that referenced this pull request Nov 14, 2023
…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.
@reillyeon
Copy link
Member

Before merging, I'd just like to double check

  • That it's better to duplicate the "accelerometer", "gyroscope" and "magnetometer" tokens here instead of using those defined and exported by the respective specs.

Given that these are part of the normative requirements what do you think about moving the definitions here and referencing from the Generic Sensors specs?

  • Whether there needs to be any involvement from the WebApps WG since this is a substantive change.

The relationship hasn't been formalized yet but ideally we could hold a joint meeting to discuss these Permissions / Permissions Policy integrations.

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.
@rakuco
Copy link
Member Author

rakuco commented Nov 30, 2023

Given that these are part of the normative requirements what do you think about moving the definitions here and referencing from the Generic Sensors specs?

Ack. As discussed here and in #123, I've pushed a new commit here exporting the permissions policy tokens in this specification.

Like with #123, I'm deferring merging to you and @anssiko in case the WebApps WG needs to be involved.

@anssiko
Copy link
Member

anssiko commented Dec 4, 2023

@rakuco can you fix the merge conflicts.

This substantive change makes the firing of the events defined in this
specification dependent on a few different policy-controlled features:
"accelerometer", "gyroscope", and "magnetometer.

Both Blink and WebKit already integrate their implementations with the
Permissions Policy spec and use the same tokens, although a bit differently:

- Both require "accelerometer" and "gyroscope" for the devicemotion event.
- Blink requires "accelerometer" and "gyroscope" to provide relative
  orientation data for the deviceorientation event, and additionally the
  "magnetometer" feature to fall back to absolute orientation data. WebKit
  always requires the three tokens for the deviceorientation event.
- Blink requires "accelerometer", "gyroscope", and "magnetometer" for the
  deviceorientationabsolute event, whereas WebKit does not implement this
  event.

We have opted to codify Blink's behavior in the specification, as since
relative orientation data does not require a magnetometer sensor, the event
should not require the corresponding token either.

Fixes #64.
@rakuco rakuco force-pushed the permissions-policy-integration branch from b059808 to f87717b Compare December 5, 2023 12:16
@rakuco
Copy link
Member Author

rakuco commented Dec 5, 2023

@rakuco can you fix the merge conflicts.

Done. I'll proceed and merge. Thanks everyone!

@rakuco rakuco merged commit e81d70f into main Dec 5, 2023
2 checks passed
@rakuco rakuco deleted the permissions-policy-integration branch December 5, 2023 12:18
github-actions bot added a commit that referenced this pull request Dec 5, 2023
SHA: e81d70f
Reason: push, by rakuco

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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 31, 2024
…olicy one

This addresses a conflict that was introduced in #121:

- The presence of the Permissions Policy integration means usage of the
  Device Orientation API can be allowed in third-party iframes provided that
  the right tokens are in place.
- The "Security and privacy considerations" section contains a requirement
  that events are fired only on child navigables that are same-origin with
  the top-level traversable.

The latter was introduced in #25 and served as a stop-gap measure before
Permissions Policy integration was added.

The current implementation status is:
- Blink never implemented the same-origin requirement, but added Permissions
  Policy integration in 2018.
- WebKit has always implemented Permissions Policy integration.
- Gecko implements the same-origin requirement (see Mozilla bug 1197901).

This means we can safely replace the same-origin requirement with a
requirement to support the Permissions Policy integration, as switching from
one to the other is transparent in the sense that the exact same set of
websites that worked before will continue to work with the change, as the
features we define have a default allowlist of "self".

Fixes #133
rakuco added a commit that referenced this pull request Jan 31, 2024
…olicy one (#136)

This addresses a conflict that was introduced in #121:

- The presence of the Permissions Policy integration means usage of the
  Device Orientation API can be allowed in third-party iframes provided that
  the right tokens are in place.
- The "Security and privacy considerations" section contains a requirement
  that events are fired only on child navigables that are same-origin with
  the top-level traversable.

The latter was introduced in #25 and served as a stop-gap measure before
Permissions Policy integration was added.

The current implementation status is:
- Blink never implemented the same-origin requirement, but added Permissions
  Policy integration in 2018.
- WebKit has always implemented Permissions Policy integration.
- Gecko implements the same-origin requirement (see Mozilla bug 1197901).

This means we can safely replace the same-origin requirement with a
requirement to support the Permissions Policy integration, as switching from
one to the other is transparent in the sense that the exact same set of
websites that worked before will continue to work with the change, as the
features we define have a default allowlist of "self".

Fixes #133
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 Permissions Policy
3 participants