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

Give the UA permission to return anything it wants from query(). #97

Merged
merged 3 commits into from Jun 8, 2016

Conversation

@jyasskin
Copy link
Member

jyasskin commented May 25, 2016

This option is the most straightforward about giving the UA nearly complete flexibility. The main constraint is that the result should only change when the UA gets new information about the user's intent, but we don't say what constitutes new information. This version also gives some examples (media and durable storage) of constraining permission behavior further on a per-API basis.

Preview at https://api.csswg.org/bikeshed/?url=https://raw.githubusercontent.com/jyasskin/permissions/ua-can-return-anything/index.bs

jyasskin added 2 commits May 11, 2016
The main constraint is that the result can only change when the UA gets
new information about the user's intent.
index.bs Outdated
more detailed structure that requires more information to describe it. In
that case, they should define a customized <a>permission descriptor
type</a> dictionary that inherits from {{PermissionDescriptor}}.
Each powerful feature has one or more aspects that websites may request

This comment has been minimized.

Copy link
@marcoscaceres

marcoscaceres May 26, 2016

Member

nit: link to "powerful features" dfn?

This comment has been minimized.

Copy link
@marcoscaceres

marcoscaceres May 26, 2016

Member

nit: please avoid using "may" without a conforming product. Maybe we can just say "websites can request"?

This comment has been minimized.

Copy link
@jyasskin

jyasskin May 26, 2016

Author Member

I don't think there is a definition of "powerful features". It used to be in Secure Contexts, but it got taken out. I was going back and forth on adding a "Definitions" section in this patch, but I guess I'll do it for this one.

And I've switched to "can".

index.bs Outdated
<section>
<h2 id="permission-operations">Permission states</h2>
<p>
The user agent is responsible for tracking what powerful features each

This comment has been minimized.

Copy link
@marcoscaceres

marcoscaceres May 26, 2016

Member

Nit: link to "powerful features"\g ?

This comment has been minimized.

Copy link
@jyasskin

jyasskin May 26, 2016

Author Member

Done.

index.bs Outdated
<h2 id="permission-operations">Permission states</h2>
<p>
The user agent is responsible for tracking what powerful features each
<a>realm</a> has the user's permission to use. Other specifications can use

This comment has been minimized.

Copy link
@marcoscaceres

marcoscaceres May 26, 2016

Member

I might be mistaken, but I was under the impression we were going to avoid "realms" in web specs and leave it to ES. I wonder if there is something we could use from HTML regarding security contexts?

This comment has been minimized.

Copy link
@jyasskin

jyasskin May 26, 2016

Author Member

I don't see anything in HTML about security contexts. We could make up a term to mean "the thing that code runs inside which has a global object and a settings object", but why bother if Javascript already has it?


<p>
Other specifications can also add more constraints on the UA's behavior in
these algorithms.

This comment has been minimized.

Copy link
@marcoscaceres

marcoscaceres May 26, 2016

Member

This doesn't seem great to me... it reads like "another spec can modify these algorithms" (e.g., via monkey patch). I don't think that is the intent.

This comment has been minimized.

Copy link
@jyasskin

jyasskin May 26, 2016

Author Member

What kind of hook would read better to you? It really is the case that other specs need to be able to modify the values these algorithms can return, and you can see some examples under push, media, and persistent-storage.

index.bs Outdated
<h3 id="reading-current-states">Reading the current permission state</h3>
<p>
|descriptor|'s <dfn export>permission state</dfn> is one
of {{"granted"}}, {{"prompt"}}, or {{"denied"}}, indicating respectively

This comment has been minimized.

Copy link
@marcoscaceres

marcoscaceres May 26, 2016

Member

maybe link to the enum itself? Otherwise, we will need to duplicate these values if we need to update the enum. If these are the formal definitions for each of these states, then we should break them up into a dl.

This comment has been minimized.

Copy link
@jyasskin

jyasskin May 26, 2016

Author Member

I don't think we can ever update the enum once it's escaped into the web. Too many people will test for two of the states and else the third.

Similarly, updating this list will be the least of our worries if we change an enum value: we'll also have to update all of the callers in other specs.

index.bs Outdated
if the calling algorithm should succeed without prompting the user, show
the user a prompt to decide whether to succeed, or fail without prompting
the user. The UA must return whichever of these values most accurately
reflects the user's intent. Successive uses of |descriptor|'s <a>permission

This comment has been minimized.

Copy link
@marcoscaceres

marcoscaceres May 26, 2016

Member

It's unclear to me what "Successive uses" means here.

This comment has been minimized.

Copy link
@jyasskin

jyasskin May 26, 2016

Author Member

"Subsequent"?

index.bs Outdated
the user a prompt to decide whether to succeed, or fail without prompting
the user. The UA must return whichever of these values most accurately
reflects the user's intent. Successive uses of |descriptor|'s <a>permission
state</a> with the same <a>current settings object</a> should return the

This comment has been minimized.

Copy link
@marcoscaceres

marcoscaceres May 26, 2016

Member

don't you mean "MUST return the same value"... it would be strange if it suddenly changed for whatever reason. The next statement should clarify about changing the value.

This comment has been minimized.

Copy link
@jyasskin

jyasskin May 26, 2016

Author Member

Sure.

index.bs Outdated

<p>
Some powerful features have more information associated with them than
just a {{PermissionState}}. For example, {{MediaDevices/getUserMedia()}}

This comment has been minimized.

Copy link
@marcoscaceres

marcoscaceres May 26, 2016

Member

The example here seems to mirror the information that was already given in the example above, no?

This comment has been minimized.

Copy link
@jyasskin

jyasskin May 26, 2016

Author Member

I actually expect that the "camera" permission descriptor will become more like MediaStreamConstraints, rather than just the device ID that's there now. The descriptor is about describing a desired permission, while the extra permission data is about describing a granted permission, and they can be totally different kinds of data.

I've updated the example above to make that more clear, by using "bluetooth" instead of "camera".

index.bs Outdated
<dfn export>extra permission data</dfn> is the instance of that type
matching the UA's impression of the user's intent. Successive uses of
|name|'s <a>extra permission data</a> should return the same value, unless
the UA receives new information about the user's intent.

This comment has been minimized.

Copy link
@marcoscaceres

marcoscaceres May 26, 2016

Member

the text " unless the UA receives new information about the user's intent" seems to be repeated throughout. Can we capture this as a concept to avoid having to repeat it?

This comment has been minimized.

Copy link
@jyasskin

jyasskin May 26, 2016

Author Member

I can define the term, but I don't immediately see a shorter phrase that works well. Do you have ideas?

index.bs Outdated
<div algorithm="request-permission-to-use">
<p>
To <dfn export>request permission to use</dfn> a |descriptor|, the UA
must perform the following steps. Note that these steps may wait for

This comment has been minimized.

Copy link
@marcoscaceres

marcoscaceres May 26, 2016

Member

the "note" text should be moved into its own "note" section.

This comment has been minimized.

Copy link
@jyasskin

jyasskin May 26, 2016

Author Member

Done.

index.bs Outdated
that value and abort these steps.
</li>
<li>
Show the user a prompt asking their permission for the calling

This comment has been minimized.

Copy link
@marcoscaceres

marcoscaceres May 26, 2016

Member

Note: "show" implies visual prompt - some UAs may prompt through different modalities, so maybe use prompt as a verb:

"Prompt the user for permission x..."

This comment has been minimized.

Copy link
@jyasskin

jyasskin May 26, 2016

Author Member

Done.

index.bs Outdated
If the user grants permission, return {{"granted"}}; otherwise return
{{"denied"}}. Depending on the details of the user's interaction, the
UA may also treat this as new information about the user's intent for
other realms with the same origin.

This comment has been minimized.

Copy link
@marcoscaceres

marcoscaceres May 26, 2016

Member

nit: link to realms and origin.

This comment has been minimized.

Copy link
@jyasskin

jyasskin May 26, 2016

Author Member

Done.

<p class="note">
This is intentionally vague about the details of the permission UI
and how the UA infers user intent. UAs should be able to explore
lots of UI within this framework.

This comment has been minimized.

Copy link
@marcoscaceres
index.bs Outdated
<p>
To <dfn export>prompt the user to choose</dfn> one of several options
associated with a |descriptor|, the UA must perform the following steps.
Note that these steps may wait for user input, so they should not be

This comment has been minimized.

Copy link
@marcoscaceres

marcoscaceres May 26, 2016

Member

Nit: if possible, we should avoid duplicating this text. Maybe we can move this up as a general note about prompting.

This comment has been minimized.

Copy link
@jyasskin

jyasskin May 26, 2016

Author Member

Done.

index.bs Outdated

<div algorithm="prompt-user-to-choose">
<p>
To <dfn export>prompt the user to choose</dfn> one of several options

This comment has been minimized.

Copy link
@marcoscaceres

marcoscaceres May 26, 2016

Member

Link to options? Particularly as they go in as input to the algorithm.

This comment has been minimized.

Copy link
@jyasskin

jyasskin May 26, 2016

Author Member

You mean <var> them? Done.

index.bs Outdated
return one of the options and abort these steps. If the UA returns
without prompting, then successive <a lt="prompt the user to
choose">prompts for the user to choose</a> from the same set of
options with the same |descriptor| should return the same option,

This comment has been minimized.

Copy link
@marcoscaceres

marcoscaceres May 26, 2016

Member

As above. I think this "should" needs to be a MUST, whereby the permission state could be modified out-of-band (e.g., the user changes the permission via browser UI)... in which case, it MUST change to reflect the new information asynchronously. However, this reads like the permission could be synchronously changed before the turn of the event loop, which I don't think is the intent (maybe it doesn't matter, but I don't think we would want to implement it that way because there will be IPC involved).

This comment has been minimized.

Copy link
@jyasskin

jyasskin May 26, 2016

Author Member

Changed to 'must'. I don't think constraining changes to turn boundaries is particularly useful to authors, since this algorithm's running in parallel anyway and so you can't arrange to have two calls to it happen within the same turn. I tried to guarantee this in #96 if you want to take a look.

index.bs Outdated
If the user chose an option, return it; otherwise return {{"denied"}}.
Depending on the details of the user's interaction, the UA may also
treat this as new information about the user's intent for other realms
with the same origin.

This comment has been minimized.

Copy link
@marcoscaceres

marcoscaceres May 26, 2016

Member

So, given what is written here, it would cause a propagation across realms. It sounds like "queue a task" might be needed for propagating the change, no?

This comment has been minimized.

Copy link
@jyasskin

jyasskin May 26, 2016

Author Member

What would the task's steps be? Since information about the user's intent always comes in asynchronously, and it only affects the return values from other mostly-unconstrained algorithms, I don't think we generally need to talk about the tasks involved.

index.bs Outdated
treat this as new information about the user's intent for other realms
with the same origin.

<p class="note">

This comment has been minimized.

Copy link
@marcoscaceres

marcoscaceres May 26, 2016

Member

Again, we should avoid duplicating text. Let's centralize this kind of text somewhere.

<h3 id="reacting-to-revocation">Reacting to users revoking permission</h3>

<p>
When the UA learns that the user no longer intends to grant permission for

This comment has been minimized.

Copy link
@marcoscaceres

marcoscaceres May 26, 2016

Member

❤️"Well hello queue a task" ❤️ ... The algorithms above need to link to here.

This comment has been minimized.

Copy link
@jyasskin

jyasskin May 26, 2016

Author Member

This is different from the other "may treat this as new information" steps and wouldn't be appropriate as their behavior.

index.bs Outdated
If unspecified, this defaults to {{PermissionDescriptor}}.
</p>
<p>
The feature can define a partial order on instances. If |descriptorA| is

This comment has been minimized.

Copy link
@marcoscaceres

marcoscaceres May 26, 2016

Member

Not sure what " partial order " means here? Maybe example is needed?

This comment has been minimized.

Copy link
@jyasskin
index.bs Outdated
The feature can define a partial order on instances. If |descriptorA| is
<dfn for="PermissionDescriptor">stronger</dfn> than |descriptorB|, then
if |descriptorA|'s <a>permission state</a> is {{"granted"}},
|descriptorB|'s <a>permission state</a> must also be {{"granted"}}, and

This comment has been minimized.

Copy link
@marcoscaceres

marcoscaceres May 26, 2016

Member

Nit: These are statements of fact, so please use "is" instead of "must". for instance, <a>permission state</a> is {{"granted"}}.

This comment has been minimized.

Copy link
@jyasskin

jyasskin May 26, 2016

Author Member

These are actually constraints on what the UA returns from the permission state algorithm.

index.bs Outdated
newly-created instance of the <a>permission result type</a>. Uses the
algorithms in <a href="#requesting-more-permission"></a> to show the user
any necessary prompt to try to increase permissions, and updates the
instance <a>permission result type</a> to match. May return a {{Promise}}

This comment has been minimized.

Copy link
@marcoscaceres

marcoscaceres May 26, 2016

Member

Wait! it would be kinda sad if some callers returned a Promise an others don't. This appears to be what is implied here, or am I misreading this?

This comment has been minimized.

Copy link
@jyasskin

jyasskin May 26, 2016

Author Member

This algorithm is entirely internal, so what it returns isn't exposed to javascript. That said, I can probably have it possibly-throw exceptions instead of possibly-returning Promises.

index.bs Outdated
</dd>
</dl>
<p>
A <dfn export>boolean permission</dfn> is a <a>permission</a> with all types
A <dfn export>boolean feature</dfn> is a powerful feature with all types
and algorithms defaulted.

This comment has been minimized.

Copy link
@marcoscaceres

marcoscaceres May 26, 2016

Member

I know this is not being changed by this PR, but this is quite vague. Do "powerful features" define defaults?

This comment has been minimized.

Copy link
@jyasskin

jyasskin May 26, 2016

Author Member

The types and algorithms above define defaults.

index.bs Outdated
to ever be {{"granted"}}: if the site calls
{{MediaDevices/getUserMedia()}} with constraints that none of the
user's devices satisfy, the UA cannot return a device without
prompting, but saying {{"granted"}} promised to do so.

This comment has been minimized.

Copy link
@marcoscaceres

marcoscaceres May 26, 2016

Member

I'm having a hard time parsing this paragraph. I don't understand "but saying {{"granted"}} promised to do so." Maybe change it to active voice? promised by who?

This comment has been minimized.

Copy link
@jyasskin

jyasskin May 26, 2016

Author Member

How's this?

index.bs Outdated
</p>
<p>
If a realm with <a>origin</a> |O| <a lt="request permission to
use">requests permission to use</a> `{name: "persistent-storage"}` and

This comment has been minimized.

Copy link
@marcoscaceres

marcoscaceres May 26, 2016

Member

nit: you don't need the lt here, as it's the same as the text content.

This comment has been minimized.

Copy link
@jyasskin

jyasskin May 26, 2016

Author Member

It's "request" vs "requests", and Bikeshed's automatic pluralization only applies to the end of a term. (@tabatkins)

This comment has been minimized.

Copy link
@tabatkins

tabatkins May 27, 2016

Member

Correct, term fixup anywhere in the phrase would be cool but complicated. (If this form shows up a lot, you can add it manually as a linking text to the dfn.)

index.bs Outdated
{{PermissionStatus}} instances are created with the following
internal slots:
{{PermissionStatus}} instances are created with a
<dfn for="PermissionStatus" attribute>\[[query]]</dfn> internal slot, which is an instance of a feature's <a>permission descriptor type</a>.

This comment has been minimized.

Copy link
@marcoscaceres

marcoscaceres May 26, 2016

Member

nit, line length.

This comment has been minimized.

Copy link
@jyasskin

jyasskin May 26, 2016

Author Member

Fixed.

@marcoscaceres

This comment has been minimized.

Copy link
Member

marcoscaceres commented May 26, 2016

Few comments, nits, etc. Thanks for putting this PR together @jyasskin!

@marcoscaceres

This comment has been minimized.

Copy link

marcoscaceres commented on index.bs in 7c3530f May 27, 2016

Maybe we should get input from @mikewest here... "powerful features" got a few eyebrows raised when it was in: https://www.w3.org/TR/secure-contexts/

That spec now only mentions the concept once, but speaks more generally about "secure contexts".

@marcoscaceres

This comment has been minimized.

Copy link

marcoscaceres commented on 7c3530f May 27, 2016

This is great @jyasskin! Thanks for addressing all my comments!

@martinthomson

This comment has been minimized.

Copy link
Member

martinthomson commented Jun 8, 2016

Yes, this is good. You should merge this and we can iterate on the outcome.

@jyasskin jyasskin merged commit 988352b into w3c:master Jun 8, 2016
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@jyasskin jyasskin deleted the jyasskin:ua-can-return-anything branch Jul 13, 2016
jyasskin added a commit to jyasskin/web-bluetooth-1 that referenced this pull request Jul 15, 2016
Lots of terminology changed in w3c/permissions#97.
jyasskin added a commit to jyasskin/web-bluetooth-1 that referenced this pull request Jul 15, 2016
Lots of terminology changed in w3c/permissions#97.
jyasskin added a commit to jyasskin/web-bluetooth-1 that referenced this pull request Jul 15, 2016
Lots of terminology changed in w3c/permissions#97.
jyasskin added a commit to jyasskin/web-bluetooth-1 that referenced this pull request Jul 18, 2016
Lots of terminology changed in w3c/permissions#97.
jyasskin added a commit to jyasskin/web-bluetooth-1 that referenced this pull request Jul 18, 2016
Lots of terminology changed in w3c/permissions#97.
jyasskin added a commit to jyasskin/web-bluetooth-1 that referenced this pull request Jul 18, 2016
Lots of terminology changed in w3c/permissions#97.
jyasskin added a commit to jyasskin/web-bluetooth-1 that referenced this pull request Jul 20, 2016
Lots of terminology changed in w3c/permissions#97.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.