w3c / deviceorientation

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

Add Permissions API integration, start requiring requestPermission() usage #123

Closed rakuco closed 7 months ago

rakuco commented 8 months ago

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:

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

rakuco commented 8 months ago

@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 commented 8 months ago

@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 commented 8 months ago

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

Done, PTAL.

rakuco commented 7 months ago

@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 commented 7 months ago

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.

rakuco commented 7 months ago

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 commented 7 months ago

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 https://github.com/w3c/deviceorientation/labels/privacy-tracker 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