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

Proposal: Remove requestPermission() #246

Closed
rsolomakhin opened this issue Jan 30, 2018 · 11 comments · Fixed by #277
Closed

Proposal: Remove requestPermission() #246

rsolomakhin opened this issue Jan 30, 2018 · 11 comments · Fixed by #277

Comments

@rsolomakhin
Copy link
Collaborator

rsolomakhin commented Jan 30, 2018

According to the permissions team (@raymeskhoury, @mgiuca) .requestPermission() style API is an anti-pattern and users generally do not understand permission prompts that may be lacking context. I would like to send a pull-request for removing .requestPermission() and recommending browsers to show a prompt when they deem necessary. The exact amount of specification will be hashed out between the implementers. In case of Chrome, we will show a permission prompt only when the website calls ServiceWorkerREgisgration.paymentManager.instruments.set(key, data) for the first time, i.e., the instruments have never been set for this site. Chrome will require a user gesture for the first-time call to instruments.set(key, data). We will also look into using site engagement, safe browsing database, and certificate validity to outright reject instruments.set(key, data) with NotAllowedError without showing any prompt at all. I would love to hear feedback from the interested parties, in particular @romandev and @marcoscaceres .

cc: @DurgaTheHutt, @gogerald, @anthonyvd

@rsolomakhin
Copy link
Collaborator Author

Relevant reading: w3c/permissions#83

@rsolomakhin
Copy link
Collaborator Author

Pull request at rsolomakhin#1.

@mgiuca
Copy link

mgiuca commented Jan 31, 2018

I'm not 100% on this being an "anti-pattern". There's a delicate balance between declarative registration (let the user agent ask when to prompt the user) and programmatic.

For what it's worth, I think we will be going this way on the other handler types:

  • registerProtocolHandler is a programmatic API in the style of requestPermission, but I think we will move to a world where RPH is treated as a declaration, silently "registering" the handler in the background (not as the default), then at a later time we can present registered handlers to the user.
  • Web Share Target will be declaratively specified in the manifest. As with RPH, when we encounter such a site, it is silently registered in the background.

On the other hand, we tried this model with web app installation (the user agent can determine when to show an install prompt) and developers really did not like it. We have been moving ever closer to a programmatic API (BeforeInstallPromptEvent.prompt) where the developer chooses when to show the prompt.

So, maybe we should not so hastily remove requestPermission. Instead, it could be taken like registerProtocolHandler as a silent background registration that does not explicitly prompt the user.

@romandev
Copy link
Member

+1 removing requestPermission()

BTW, instruments.set() is exposed to ServiceWorker as well as Window. So, if it is called in SW context during the SW running, what's expected behavior? If implementor provides an UI to grant a permission, might probably have to make the API to expose on Window only.

@rsolomakhin
Copy link
Collaborator Author

BTW, instruments.set() is exposed to ServiceWorker as well as Window. So, if it is called in SW context during the SW running, what's expected behavior?

  1. If instruments.set() has never been called before, then reject the call in Service Worker with error message NotAllowedError: The first call to instruments.set() should be from Window context with a User Gesture.
  2. If instruments.set() has been called from Window before and user gave their consent, then instruments.set() should work in the Service Worker as well.

@rsolomakhin
Copy link
Collaborator Author

So, maybe we should not so hastily remove requestPermission. Instead, it could be taken like registerProtocolHandler as a silent background registration that does not explicitly prompt the user.

@mgiuca: requestPermission() by itself does not install a payment handler. That happens during instruments.set(). I can see instruments.set() being silent without any prompts in some cases. That's why the pull request makes the prompt optional.

@mgiuca
Copy link

mgiuca commented Feb 1, 2018

OK makes sense.

@ianbjacobs
Copy link
Contributor

Hi @rsolomakhin,

I am hearing that user consent is still required (for 'something') but the timing might vary by implementation. Indeed, within a single implementation there might be different timings (e.g., "register now on this button click" v. "just-in-time registration").

Does that sound like what you intend?

If so, I think we should augment the Security and Privacy Considerations to say so.

Ian

@marcoscaceres
Copy link
Member

I have concerns around .set() having this permissions-related side-effect. That doesn't feel right to me.

Setting a permission when registration of a payment handler occurs would be appropriate tho (i.e., not one of its instruments), however. We may need to perhaps mirror what Web Share Target is doing, or reach out to a few other folks for ideas on how best to do this (I've not given it a lot of thought and I'm somewhat struggling to come up with some precedence for "register SW X to handle Y").

@rsolomakhin
Copy link
Collaborator Author

@marcoscaceres: Isn't installing a Payment Handler equivalent to an instruments.set() call?

@rsolomakhin
Copy link
Collaborator Author

@marcoscaceres: I've addressed your comments on the pull request and explcitly called out that a Payment Handler is considered installed after the first call to instruments.set().

Note that requesting user consent for an action during the action is something that permissions team considers best practice. The requestPermission() call was not tied to a user action, possibly creating confusion in users' minds as to why a Payment Handler is not installed, even though they have given their consent earlier.

aarongable pushed a commit to chromium/chromium that referenced this issue Feb 27, 2018
This patch is an initial implementation of the following spec change:
  - w3c/payment-handler#246
  - https://chromium-review.googlesource.com/c/chromium/src/+/533193

This feature is still behind runtime flag.

Bug: 665949
Change-Id: Ied225b89c7aed3a39955e49e9af2e4e3866a92c2
Reviewed-on: https://chromium-review.googlesource.com/914661
Reviewed-by: Jochen Eisinger <[email protected]>
Reviewed-by: Raymes Khoury <[email protected]>
Reviewed-by: Rouslan Solomakhin <[email protected]>
Reviewed-by: Kinuko Yasuda <[email protected]>
Commit-Queue: Jinho Bang <[email protected]>
Cr-Commit-Position: refs/heads/master@{#539499}
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.

5 participants