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

When is user activation / prompting required? #120

Closed
foolip opened this issue Nov 20, 2019 · 4 comments · Fixed by #200
Closed

When is user activation / prompting required? #120

foolip opened this issue Nov 20, 2019 · 4 comments · Fixed by #200
Milestone

Comments

@foolip
Copy link
Member

foolip commented Nov 20, 2019

User activation is mentioned in two domintro boxes now:
https://wicg.github.io/native-file-system/#api-filesystemhandle-querypermission
https://wicg.github.io/native-file-system/#api-filesystemhandle-requestpermission

There are TODOs in those sections, but the phrasing "attempting to modify the file or directory this handle represents will require user activation" makes it sound like user activation might be required for write(), truncate() or close(). Presumably it won't be required with every call to write() however, so defining this well will be important to get interoperability in practice, as any difference between implementations can easily turn into a broken site in some browser.

@mustaqahmed

@foolip foolip changed the title When is user activation required? When is user activation / prompting required? Nov 20, 2019
@foolip
Copy link
Member Author

foolip commented Nov 20, 2019

Expanding issue to also cover prompting which is closely related. From "attempting to modify the file or directory this handle represents will require user activation and will result in a confirmation prompt being shown to the user" it sounds like the prompt might be shown as a result of calling write() and friends, which might not be the intended behavior.

Overall, it would be good to slim down these sections to what web developers really need to understand as the normative algorithms are fleshed out.

@foolip
Copy link
Member Author

foolip commented Nov 20, 2019

The comes up also in the non-normative domintro of getFile():

If this handle doesn’t already have write permission, this could result in a prompt being shown to the user.

I'm having a hard time thinking of what a prompt at this point would say that would be easy to understand the consequences of. Perhaps this case should just reject, requiring the directory itself to be opened with write permission for it to work?

@foolip
Copy link
Member Author

foolip commented Nov 20, 2019

I'll throw one final thing onto this issue. https://wicg.github.io/native-file-system/#api-filesystemhandle-querypermission says 'a handle retrieved from IndexedDB is also likely to return "prompt"' and this "likely" is a bit concerning. This needs to be spelled out in normative text.

@mkruisselbrink
Copy link
Contributor

I think there are two parts to this issue, first there is the question about user activation requirements. I think that is largely well defined by now in the spec (i.e. algorithms call the "request permission steps" when needed, which checks for user activation if the current permission status is "prompt").
The algorithm for chooseFileSystemEntries still needs to be fleshed out, but that will also check for transient user activation.

The other question seems to be more about the permission model itself, the lifetime of permission grants, etc. I think in that area there is a fine line between specifying enough to be useful to developers while still making it possible for web browsers to make their own decisions in this area. I.e. like the permissions spec we're intentionally vague of "new information about the user's intent".

I agree that there is room for improvement, but at the same time I don't want to dictate any particular permission model.

the phrasing "attempting to modify the file or directory this handle represents will require user activation" makes it sound like user activation might be required for write(), truncate() or close(). Presumably it won't be required with every call to write() however

Your quote leaves out the first half of the sentence (which I'm pretty sure hasn't changed since you filed this issue), the 'If this returns "prompt",' part. Of course if the browser isn't going to show a prompt anyway there is no need for user activation. However it seems totally reasonable to me to require that you have write permission for every call to write, truncate, close etc, so then the question is what these methods should do if the permission state some changes to "prompt" between creating a writer and finishing writing to it. If you don't have a user activation, I think everybody would agree that these should fail.

Then the question is if you happen to have a user activation, should we show a prompt? (keep in mind that how the permission state might have changed is totally dependent on what a user agent considers "new information about the user's intent). As currently written the spec says no, although I'm not 100% convinced that is the right behavior (I also believe it's not what we implemented; this is one of the areas where we'll need to figure out automated testing and make sure the spec matches the implementation).

To summarize, as currently spec'ed directory.getFile({create: true}), directory.getDirectory({create: true}), directory.removeEntry() and filehandle.createWritable() are all the methods that could potentially prompt for permission (but will only prompt if there is a transient user activation). Other methods (like write, truncate and close) will still fail if permission state isn't granted, but won't prompt (although I don't see any strong reason why they shouldn't prompt).

On the other hand, none of the read-only operations will ever prompt. If the read permission state is "prompt" you need to explicitly call requestPermission() (or requestPermission({writable: true}) on the handle before you can use it.

The comes up also in the non-normative domintro of getFile():

If this handle doesn’t already have write permission, this could result in a prompt being shown to the user.

I'm having a hard time thinking of what a prompt at this point would say that would be easy to understand the consequences of. Perhaps this case should just reject, requiring the directory itself to be opened with write permission for it to work?

I'm not sure I understand your question/suggestion. Are you saying we should always require explicit "requestPermission" calls, and never implicitly treat the attempt to do a operation as a request for the permission for that operation? That seems like it kind of goes against how most of the other APIs on the web function. The vast majority of APIs don't have explicit permission request APIs, and instead request permission is implicit in doing the operation. So pretty much any existing permission gated API shares the behavior here where if you don't already have permission you might need a user activation to call the API, but if you already were granted permission you don't need user activation.

(that aside, giving a way to open a directory with write permission already granted is something we do want to support, which isn't currently possible. But I don't see how that seems orthogonal to the question of whether doing an operation should possibly prompt for permission or if all permission requests should always be explicit separate method calls).

I'll throw one final thing onto this issue. https://wicg.github.io/native-file-system/#api-filesystemhandle-querypermission says 'a handle retrieved from IndexedDB is also likely to return "prompt"' and this "likely" is a bit concerning. This needs to be spelled out in normative text.

What the likely is trying to capture is that browser might (and chrome currently does) treat closing all windows for an origin as "new information about the user's intent", in particular the user's intent to revoke all current permissions for that origin. So when retrieving a handle from IndexedDB it is quite likely that this is a new visit to the website, it is similarly likely that the permission state is now "prompt". But by no means is this something we want to force browsers to do. I.e. we're actively exploring how we want to permission model to behave exactly, and what signals we use to infer the user's intent, and how we decide when we might change a permission state from "granted" back to "prompt". I'm really not sure what more I could specify in normative text without making it impossible for browsers to figure out whatever permission model works best for themselves.

Anyway having said all that, I do agree that I need to go back and rewrite the non-normative text around permission handling. Especially the dom-intro for queryPermission and requestPermission could be a lot clearer.

For the normative text I hope we'll be able to come up with some way to move most of the heavy lifting to the permissions spec (although that wouldn't change when/how we actually possibly request permission, which I think is your main question here).

@mkruisselbrink mkruisselbrink added this to the V1 milestone Apr 10, 2020
mkruisselbrink added a commit that referenced this issue Jul 14, 2020
This is partially editorial (just changing how algorithms check
permissions), but also has a couple of normative changes:

- A change from `boolean writable` to a enum, with currently "read"
  and "readwrite" options. This is to keep open the possibility for
  write-only handles as discussed in #119.
- Changes from NotAllowedError to SecurityError in a couple of places,
  to better align with how other APIs behave.
- And of course the integration with navigator.permissions.query,
  although that is unlikely to be implemented in chrome any time soon,
  as a lot of the infra for that is missing in chrome.

This fixes #120.
mkruisselbrink added a commit that referenced this issue Jul 20, 2020
…200)

* Rewrite the permissions logic to integrate with the permissions API.

This is partially editorial (just changing how algorithms check
permissions), but also has a couple of normative changes:

- A change from `boolean writable` to a enum, with currently "read"
  and "readwrite" options. This is to keep open the possibility for
  write-only handles as discussed in #119.
- Changes from NotAllowedError to SecurityError in a couple of places,
  to better align with how other APIs behave.
- And of course the integration with navigator.permissions.query,
  although that is unlikely to be implemented in chrome any time soon,
  as a lot of the infra for that is missing in chrome.

This fixes #120.
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 a pull request may close this issue.

2 participants