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

Align requestStorageAccess rejection w.r.t lack of user gesture #36

Closed
Brandr0id opened this issue May 16, 2020 · 7 comments
Closed

Align requestStorageAccess rejection w.r.t lack of user gesture #36

Brandr0id opened this issue May 16, 2020 · 7 comments
Assignees
Labels
interop Tracking differences in existing implementations

Comments

@Brandr0id
Copy link
Member

Currently, at least in Firefox, if document.requestStorageAccess is called without a user gesture and storage is not otherwise blocked the API appears to reject. This differs slightly from the wording in the spec draft and I wanted to make sure we pick/align current implementations and the spec to one behaviour here.

Currently the draft of the spec will let the promise resolve/reject immediately if either explicit allow/deny flags/state are set for the document. I also noticed the current draft text doesn't look to take into account the current state of storage access (e.g. let result be running determine if a site has storage access with key and doc and if result is true resolve p).

I can see both POV on this where if we have a prior signal that access is granted (either access is allowed or the allow/deny flags are set) then we can immediately resolve/reject. I can also see how if we don't have a user gesture, why would we potentially resolve or reject since the API shouldn't be usable without a gesture.

Thoughts on if it matters for the ordering here and depending on the result can we either update the spec language or open bugs on implementers for the behaviour change?

A side note, at least with the current impl logic of rejecting on gesture first we can get some test coverage in WPT to validate the API rejects while w3c/webdriver#1437 is active.

@hober hober self-assigned this Jun 2, 2020
@hober
Copy link
Member

hober commented Jun 8, 2020

cc @privacycg/storage

@johannhof
Copy link
Member

Speaking for Firefox we agree that it would be good to align browsers on the same behavior. I'm also not 100% sure about what the right behavior would be, though I'm leaning towards always requiring a user gesture.

In my opinion, it's weird for developers to use an API that will sometimes require interaction and sometimes not, based on criteria that are opaque to them at runtime. When the call to rSA succeeds without user gesture you shouldn't have called it. So maybe we should make that explicit and avoid confusion by always requiring user gesture.

With that said, we've historically added such APIs (that sometimes require interaction and sometimes not) before, for example recently when requiring user interaction for Notification.requestPermission. We did that to avoid breaking too many existing sites mostly, which doesn't seem like a concern here.

If we don't require a user gesture, we would effectively turn rSA into an alternative to hasStorageAccess, when you handle the error from lack of interaction it can be used the same way. There's a small risk for compat issues when browsers don't align on that (as is the case with Safari and Firefox right now), so it does seem important to have this discussion.

@hober hober added agenda+ Request to add this issue to the agenda of our next telcon or F2F interop Tracking differences in existing implementations and removed agenda+ Request to add this issue to the agenda of our next telcon or F2F labels Jul 6, 2020
@johannhof
Copy link
Member

Looking into this again we (@englehardt and I) noticed that the behavior described in #36 (comment) seems outdated, here's what we got in our testing:

Firefox: User interaction is only required when storage access has not been granted (consecutive rSA calls on the same page, after reload, and in frames that don’t have cookies blocked)
Safari: Like Firefox
Safari Tech preview: Will reject without user interaction, independent of prior storage access state.
Edge: Didn't seem to require user interaction at all, is this a bug or a mistake on our end @Brandr0id ?

FWIW we still think that #36 (comment) applies and that Firefox should probably switch to always requiring user interaction, assuming that Webkit folks are looking to ship these changes that we're seeing in Safari Tech Preview. Then (assuming there's no objection from Edge) we can clarify this in the specification.

@johannhof
Copy link
Member

An update that I didn't test Edge correctly and that their behavior actually seems to be that the browser will reject without user interaction, independent of prior storage access state. Also, Safari (now?) seems to behave like Safari TP, which makes Firefox the only browser without the "strict" user gesture enforcement that we actually advocate for.

Seems to me like we can resolve this issue by aligning on rejecting immediately when no user gesture is present (Firefox will update our implementation). Does that sound good to you, @Brandr0id? If yes I can make a pull request to the spec.

@Brandr0id
Copy link
Member Author

Thanks @johannhof SGTM.

@jackfrankland
Copy link

jackfrankland commented Jan 7, 2021

For future behaviour, the result of #60 could be relevant I think. It could perhaps be confusing for the promise to sometimes reject, or never reject under different circumstances.

In my opinion, it's weird for developers to use an API that will sometimes require interaction and sometimes not, based on criteria that are opaque to them at runtime. When the call to rSA succeeds without user gesture you shouldn't have called it. So maybe we should make that explicit and avoid confusion by always requiring user gesture.

I'm not sure if it exactly relates to the argument, but sticky activation-gated APIs already exist. Also, a user gesture being active could under certain circumstances be opaque to the developer too.

Personally, while I agree that the recommendation should be to first check if storage access has been given, I think it's potentially more confusing that the API can seemingly reject subsequent requests for storage access after it has already been granted within the page's lifecycle.

@johannhof
Copy link
Member

Yeah, I agree that "confusing" is a very subjective thing here, depending on the individual expectations of the developer.

In my experience (working on permissions) #60 is a very hard problem to solve, especially trying to balance usability of the API with user protection. I was out when that discussion happened, thanks for putting it on my radar :)

I filed the bug for aligning Firefox here.

johannhof added a commit to johannhof/storage-access that referenced this issue Jan 26, 2021
This also removes outdated references to the WebKit and Gecko code,
both of which now also need to change to adhere to the new spec.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
interop Tracking differences in existing implementations
Projects
None yet
Development

No branches or pull requests

4 participants