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

Introduce "condition to update latest reading" #280

Merged
merged 4 commits into from
Sep 26, 2017

Conversation

alexshalamov
Copy link

@alexshalamov alexshalamov commented Sep 21, 2017

Fixes: #227
Fixes: #223
Fixes: #222


Preview | Diff

index.bs Outdated
@@ -771,6 +764,23 @@ Note: For example, the UA may estimate [=optimal sampling frequency=] as a Least
(LCD) for a set of {{[[desiredSamplingFrequency]]|desired sampling frequencies}} capped by
[=sampling frequency=] bounds defined by the underlying platform.

<h3 id="model-sensor">Conditions to update latest reading</h3>

Choose a reason for hiding this comment

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

maybe we could convert it to a feature of the browsing context and move this section into "Concepts"? For example:
"The browsing context is <dfn>safe to share platform data</dfn> if the following conditions are met ..." and in the text above it could be "Any time a new [=sensor reading=] for a [=platform sensor=] is obtained and if the current browsing context is [=safe to share platform data=] the user agent invokes [=update latest reading=]..." WDYT?

Copy link
Author

@alexshalamov alexshalamov Sep 22, 2017

Choose a reason for hiding this comment

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

Sounds good. <dfn>safe to provide sensor data</dfn>

  • platform => sensor. To make it explicit, platform has lots of other data
  • share => provide. Share might sound as if UA shares data with something other than web page.

wdyt?

Choose a reason for hiding this comment

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

I'm fine with that, maybe "propagate" would be even more appropriate verb here: "safe to propagate sensor readings (to be consistent :-) )"

Copy link
Member

Choose a reason for hiding this comment

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

Alternative proposals for the definition:
<dfn>can expose sensor readings</dfn>
<dfn>check if we can expose sensor readings</dfn>

Where "expose" refers to "expose to scripts" specifically.

To be consistent with e.g.:

https://html.spec.whatwg.org/#can-share-memory-with
https://html.spec.whatwg.org/#check-if-we-can-run-script

The term "propagate" has its own meaning in https://dom.spec.whatwg.org/ so I'd rather avoid it unless we define it more specifically.

Copy link
Author

Choose a reason for hiding this comment

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

  • Added <dfn>can expose sensor readings</dfn>
  • Moved section to concepts

index.bs Outdated
@@ -771,6 +764,23 @@ Note: For example, the UA may estimate [=optimal sampling frequency=] as a Least
(LCD) for a set of {{[[desiredSamplingFrequency]]|desired sampling frequencies}} capped by
[=sampling frequency=] bounds defined by the underlying platform.

<h3 id="model-sensor">Conditions to update latest reading</h3>
Copy link
Member

Choose a reason for hiding this comment

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

Alternative proposals for the definition:
<dfn>can expose sensor readings</dfn>
<dfn>check if we can expose sensor readings</dfn>

Where "expose" refers to "expose to scripts" specifically.

To be consistent with e.g.:

https://html.spec.whatwg.org/#can-share-memory-with
https://html.spec.whatwg.org/#check-if-we-can-run-script

The term "propagate" has its own meaning in https://dom.spec.whatwg.org/ so I'd rather avoid it unless we define it more specifically.

index.bs Outdated
are not delivered in such cases.
A [=security check=] is run before [=sensor readings=] are delivered to ensure that.

To mitigate this threat, user agent can check if it [=can expose sensor readings=].

Choose a reason for hiding this comment

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

s/can/should ?

index.bs Outdated
@@ -679,6 +667,20 @@ The [=reading change treshold|treshold=] value depends on the surrounding softwa
environment constraints, e.g., software power consumption optimizations or the underlying
[=device sensor=]'s accuracy.

## Conditions to expose sensor readings ## {#concepts-can-expose-sensor-readings}

In order to check if user agent <dfn>can expose sensor readings</dfn> following criteria may be used:

Choose a reason for hiding this comment

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

maybe tell that user agent is to define the necessary criteria to meet the [=conditions=] , i.e. which [=mitigation strategies=] must be applied and it depends on the underlying device' form factor or so.. The example is below

index.bs Outdated
@@ -679,6 +667,20 @@ The [=reading change treshold|treshold=] value depends on the surrounding softwa
environment constraints, e.g., software power consumption optimizations or the underlying
[=device sensor=]'s accuracy.

## Conditions to expose sensor readings ## {#concepts-can-expose-sensor-readings}

In order to check if user agent <dfn>can expose sensor readings</dfn> following criteria may be used:

Choose a reason for hiding this comment

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

maybe tell that user agent is to define the necessary criteria to meet the [=conditions=] , i.e. which [=mitigation strategies=] must be applied and it depends on the underlying device' form factor or so.. The example is below

index.bs Outdated
@@ -730,6 +732,10 @@ Any time the user agent obtains a new [=sensor reading=] for a [=platform sensor
platform, it invokes [=update latest reading=] with the [=platform sensor=] and the [=sensor reading=]
as arguments.

Any time a new [=sensor reading=] for a [=platform sensor=] is obtained, user agent must check if

Choose a reason for hiding this comment

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

Any time a new [=sensor reading=] for a [=platform sensor=] is obtained and if the user agent [=can expose sensor readings=] ..

index.bs Outdated
Using a more complex {{PermissionDescriptor}}.
(e.g. with a boolean `allowBackgroundUsage = false`; [=dictionary member=]),
might be the solution to relax this restriction.

Copy link
Contributor

Choose a reason for hiding this comment

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

How is this issue being addressed?

Copy link
Author

Choose a reason for hiding this comment

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

@rwaldron If you want, I can remove it in separate PR. We already have several issues related to exposing sensors to service workers https://github.com/w3c/sensors/projects/5. Removed inline issue is redundant.

Copy link
Author

Choose a reason for hiding this comment

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

Added issue back, will remove it in separate PR.

Alexander Shalamov added 3 commits September 26, 2017 09:55
- Fixes for Mikhail's comments
- Added inline issue back, would remove in separate PR
index.bs Outdated
## Conditions to expose sensor readings ## {#concepts-can-expose-sensor-readings}

The user agent can define the list of [=conditions=] to be used to check if
<dfn lt="can expose sensor readings">sensor readings can be exposed</dfn>.
Copy link
Member

Choose a reason for hiding this comment

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

I would say:

to check if it <dfn>can expose sensor readings</dfn>

That is, use the "can expose sensor readings" operation's canonical name.

index.bs Outdated
is "visible".
- [=Currently focused area=] belongs to an [=active document=] of [=platform sensor=]'s associated
[=browsing context=] or to an [=active document=] of a [=nested browsing context=] whose
[=active document=]'s [=origin=] is the [=same origin-domain=] as the [=top-level browsing context=]'s
Copy link
Member

Choose a reason for hiding this comment

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

nit: remove whitespace after "is the "

index.bs Outdated
[=active document=]'s [=origin=] is the [=same origin-domain=] as the [=top-level browsing context=]'s
[=active document=]

Note: In order to release hardware resources, user agent can request underlying [=platform sensor=]
Copy link
Member

Choose a reason for hiding this comment

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

the user agent

index.bs Outdated
@@ -658,6 +652,23 @@ The [=reading change threshold|threshold=] value depends on the surrounding soft
environment constraints, e.g., software power consumption optimizations or the underlying
[=device sensor=]'s accuracy.

## Conditions to expose sensor readings ## {#concepts-can-expose-sensor-readings}

The user agent can define the list of [=conditions=] to be used to check if

Choose a reason for hiding this comment

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

should we mention that these conditions are based on [=mitigation strategies=] list? Also there is a list of mandatory requirements https://w3c.github.io/sensors/#construct-sensor-object (top level browsing context, secure context) think it is also worth mentioning here.

Copy link
Author

Choose a reason for hiding this comment

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

I would keep it as it is 😄

Copy link

@pozdnyakov pozdnyakov left a comment

Choose a reason for hiding this comment

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

lgtm

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.

None yet

4 participants