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

Reword the "Sensor Type" section, move permission revocation algorithm #466

Merged

Conversation

rakuco
Copy link
Member

@rakuco rakuco commented Jul 31, 2023

Instead of having several paragraphs mixing what a sensor type must or may and algorithms, the "Sensor Type" section now only has two paragraphs and two groups containing what must be defined and what may be defined to make it easier to follow.

This also has consequences for the "Extensibility" section, as its "Definition Requirements" subsection used to duplicate some of the contents in "Sensor Type". It now just refers to the latter and provides more details about how some of a sensor type's associated data must be defined in extension specifications.

Some data that used to be associated with a sensor type has been removed:

  • "Associated sensors" were never properly defined and it was not clear that they meant "associated platform sensors". They were only used in the generic sensor permission revocation algorithm, which has been rewritten.
  • "Permission request algorithm" is a term that has been moved from the Permissions spec to the Requesting Permissions permission in WICG. None of the extension specifications ever defined it.

The generic sensor permission revocation algorithm was moved to the "Abstract Operations" section. It has also been merged with the revoke sensor permission algorithm since it was its only caller. There should not be any user-visible effects though: the language was just made more formal by iterating over all Sensor instances belonging to the current realm (like the Web Bluetooth spec does) instead of hand-wavingly gathering all platform sensors (which technically are per-navigable) with the same sensor type.

Related to #463.


Preview | Diff

Instead of having several paragraphs mixing what a sensor type must or may
and algorithms, the "Sensor Type" section now only has two paragraphs and
two groups containing what must be defined and what may be defined to make
it easier to follow.

This also has consequences for the "Extensibility" section, as its
"Definition Requirements" subsection used to duplicate some of the contents
in "Sensor Type". It now just refers to the latter and provides more details
about how some of a sensor type's associated data must be defined in
extension specifications.

Some data that used to be associated with a sensor type has been removed:
- "Associated sensors" were never properly defined and it was not clear that
  they meant "associated _platform_ sensors". They were only used in the
  generic sensor permission revocation algorithm, which has been rewritten.
- "Permission request algorithm" is a term that has been moved from the
  Permissions spec to the Requesting Permissions permission in WICG. None of
  the extension specifications ever defined it.

The generic sensor permission revocation algorithm was moved to the
"Abstract Operations" section. It has also been merged with the revoke
sensor permission algorithm since it was its only caller. There should not
be any user-visible effects though: the language was just made more formal
by iterating over all Sensor instances belonging to the current realm (like
the Web Bluetooth spec does) instead of hand-wavingly gathering all
platform sensors (which technically are per-navigable) with the same sensor
type.

Related to w3c#463.
@rakuco rakuco requested a review from reillyeon July 31, 2023 09:04
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
: output
:: None

1. Let |activated_sensors| be |sensor|'s associated [=ordered set|set=] of [=activated sensor objects=].
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this algorithm can still use activated sensor objects to good effect, but it needs to be able to translate permissionName into a set of platform sensors by looking at each platform sensor's sensor type and checking if permissionName is contained in its sensor permission names. This is what I think the original text meant by "associated".

Without this logic using it we can probably remove the activated sensor objects concept, though it is somewhat useful for defining how sensor readings are reused between Sensor instances.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem's that it's not entirely clear what context the "permission revocation algorithm" runs in according to https://w3c.github.io/permissions/#reacting-to-revocation so I'm not sure what platform sensors should be iterated to start with. Should one start from the current realm and get a Window global and get the associated platform sensors? In this case, getting actual platform object instances that exist in a realm felt easier.

One thing I've done is add a check below to do nothing if a sensor is idle, so that we only act on active sensors (or sensors that are being activated).

@rakuco rakuco merged commit 2ea4fef into w3c:main Aug 1, 2023
1 check passed
@rakuco rakuco deleted the reword-sensor-type-move-revocation-algorithm branch August 1, 2023 18:32
github-actions bot added a commit that referenced this pull request Aug 1, 2023
#466)

SHA: 2ea4fef
Reason: push, by rakuco

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

2 participants