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

Permission API has terminology issues one could drive a truck through. #85

Closed
jan-ivar opened this issue Apr 21, 2016 · 7 comments
Closed

Comments

@jan-ivar
Copy link
Member

Rather than rehash it here, I ask that people read what I just posted in w3c/mediacapture-main#334 (comment), as context is important.

Ask:

  1. Rename this spec to "Persistent Permissions API".
  2. Define and use the term "persistent permission" instead of "permission" throughout.
  3. Clarify how "access" is distinct from "persistent permission".
  4. Avoid use of the ambiguous term "permission".
  5. Avoid conflating a persistent permission API with an access API.
  6. Remove attempts to co-opt existing APIs for request Drop .request() #83 or revoke Consider removing Permissions.revoke(). #46 of _access_.
@jyasskin
Copy link
Member

This API has context for calls to functions like query(), request(), and revoke(): the current environment settings object. Saying that query() only refers to the persistent part of a permission like geolocation would break its use in libraries that could take advantage of the current tab having permission, even if there's no persistent permission. (There is, of course, the privacy issue #52, but we shouldn't split discussion of that.)

I believe the original intent of this spec was to deal with both temporary and persistent permissions. It doesn't do a great job of that yet, because of the description of a single permission store, but I think that's fixable.

I do think we might want to rename this spec to just "Permissions": it defines the platform permission architecture in addition to an API, and other specs, like Media Capture, are likely to reference this one primarily to interact with that architecture, not because they want to use the API. It's a little like https://storage.spec.whatwg.org/ in that regard.

I'll leave this open for a day or two to let other people chime in, but I don't expect to take these suggestions.

@jyasskin jyasskin changed the title Permission absent context == persistent permission Replace "Permission" with "Persistent Permission" globally. Apr 21, 2016
@jan-ivar
Copy link
Member Author

Please see beyond my recommendation to the issues I point out. This is after all an issue not a PR, and I believe it points out real terminology issues with the spec:

  • There is no definition of permission.
  • The implicit definition is that of a persistent permission, which is fine, but should be made clear.
  • Statements from participants about this covering access ("temporary permission") don't hold.
  • Algorithms based on this poor definition, such as request, make no sense (see below).

We should leave it open until these are addressed. Again, my main explanation about the poor terminology is laid out in w3c/mediacapture-main#334 (comment), not here, so please go read that first.

Let me walk through, for instance, the request algorithm to illustrate how little sense it makes:

Takes the previously-stored instance of the permission storage type,

I assume permission storage type must mean PermissionStorage since there are no other sub-classes anywhere in the spec. That in turn is "just an explanatory device", and really just a state, either "prompt", "granted", or "denied". Lets say this state starts out as "prompt" in this example walk-through (I'm being kind, because this algorithm appears to do nothing in any other state, yet makes no mention of checking said state at the point of entry).

an instance of the permission descriptor type,

a name essentially.

and a newly-created instance of the permission result type.

This is just another state as far as data is concerned.

Shows the user any necessary prompt

This is hand-waving.

to try to increase permissions,

The term increase permissions is not defined or explained anywhere. My best guess is that "to increase permissions" means change the state from "prompt" to "granted"? I don't know what else it can mean.

and updates the instances of the permission storage type and permission result type to match.

And we can stop here, as this seems to be all this algorithm really does: ask the user if it can change 'prompt' to 'granted', in other words: ask the user whether this site can have persistent permission please. That is: the ability to access the named ability any time it wants without having to prompt for each access. Interestingly, this API offers no mechanism through which such "prompts for access" can be made, because we just walked through the only algorithm that it could be, and we unearthed no mechanism for access, and that would be recursive anyways.

Now @jyasskin please explain to me what a "temporary permission" is in this spec.

May return a Promise if the request can fail exceptionally.

If we keep going, another red flag here is that this seems to describe an API that sometimes returns a promise and sometimes not. A terrible API for error handling. But it suggests something more ominous, which is that this appears to be an attempt to describe a pattern rather than any specific API. I've seen this before and the results are always disastrous: a spec that doesn't do anything, a bunch of air, and worse, a spec that tries to enforce generalities and shape things without doing any lifting, and without accomplishing anything other than the behavioral symmetry it seems to proclaim is a worthy goal in itself. Specs should be specific, and not dabble in generalities. Coaxing things that are inherently different into lockstep for no apparent reason other than ergonomics is close to cargo-cult science in my book.

(Merely being denied permission is not exceptional.) Used by Permissions' request() method, which handles reading and writing the permission store. If unspecified, this defaults to the boolean permission request algorithm.

This says TODO.

@jan-ivar jan-ivar changed the title Replace "Permission" with "Persistent Permission" globally. Permission API has terminology issues one could drive a truck through. Apr 22, 2016
@jyasskin
Copy link
Member

Way better title, thanks.

I'll reply to w3c/mediacapture-main#334 (comment) in this thread to keep the permissions discussion over here.

By "access" I mean the site can access the user's camera or mic. By "persistent permission" I mean the UA policy setting (prompt, grant, or deny) for dealing with new requests for access. If you are using any other terms, please define them.

This spec tries to use "permission" to mean the ability to access the capability, whether that's the camera, mic, location, notifications, etc. "persistent permission" is such an ability that will survive a page reload. "temporary permission" is such an ability that won't survive a reload, but you've pointed out that there are possibly two levels of temporary-ness: one where a new call to, e.g. navigator.watchLocation, within the same realm causes a new prompt, vs one where it wouldn't.

As I've said, the spec does a bad job of accommodating temporary permissions, but we're working on fixing that. It's always tried to accommodate them, with, for example "User agents may use a form of storage to keep track of web site permissions.", but it's always needed more to carry that through.

Let me walk through, for instance, the request algorithm to illustrate how little sense it makes:

What you're seeing here is that the algorithms aren't finished. (That's what "TODO" means.) The most finished one is the "bluetooth" permission, but even that one suffers from 1) "granted"/"prompt"/"denied" being an imperfect match for chooser permissions, 2) "revoke a permission" not passing the details of what piece of the permission to revoke, and 3) there being only one permission store, which doesn't cover the nuances of temporary permissions.

@jan-ivar
Copy link
Member Author

jan-ivar commented Apr 22, 2016

@jyasskin I'm sure you mean well, but you seem to hold a certain view of what the spec "is" or "tries to do", that I find no support for in what's actually written down. To me that means there's no consensus behind your claims.

I'm used to a process where such claims must be in the form of a proposal for people to vote on.

@jan-ivar
Copy link
Member Author

@jyasskin sorry got a little heated there. I admire your conviction, it's just that I happen to disagree with the direction you have in mind.

@jyasskin
Copy link
Member

That's why this is still an Editor's Draft, and not a Working Draft. When it's finished enough to Call for Consensus, we'll do that.

Until then, I don't claim that the current text represents any sort of consensus. I do think the direction we're going will get us to a place that has consensus, but if you don't think so, we can keep having that discussion. You're also totally welcome to fork the spec and try to produce something that reflects the community's opinions better.

@jyasskin
Copy link
Member

I think #97 fixed most of this. It changed things from talking about "having a permission" to "having permission to access/use a feature". If you find any particular places I missed, please reopen this issue or a new more focused one.

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

No branches or pull requests

2 participants