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

Invoke callback/promise with "default" when dialog is dismissed #70

Open
annevk opened this issue Mar 31, 2016 · 20 comments
Open

Invoke callback/promise with "default" when dialog is dismissed #70

annevk opened this issue Mar 31, 2016 · 20 comments

Comments

@annevk
Copy link
Member

annevk commented Mar 31, 2016

It seems this is what's being implemented https://bugzilla.mozilla.org/show_bug.cgi?id=1259148 and I now remember @beverloo telling me Chrome shipped that too.

@mvano
Copy link
Contributor

mvano commented Mar 31, 2016

@domenic Is there guidance somewhere about letting promises hang forever? Because that's something we've tried to avoid in multiple places.

@annevk It seems that Mozilla has some discussion about whether dismissing a permission prompt is a decision made by the user or not. In my eyes, it's a decision to postpone grant/deny and get on with using the site. Still, that seems like an implementation specific detail that would not be defined in the spec. What aspect are you proposing to spec here?

@annevk
Copy link
Member Author

annevk commented Mar 31, 2016

@mvano in particular, that the callback can be invoked with "default" is something novel that needs to be defined.

@mvano
Copy link
Contributor

mvano commented Mar 31, 2016

Ah yeah that seems reasonable to me.

@domenic
Copy link
Member

domenic commented Mar 31, 2016

@domenic Is there guidance somewhere about letting promises hang forever? Because that's something we've tried to avoid in multiple places.

Not really; it's pretty unremarkable, so there's no guidance about it. Certainly not about avoiding it.

@ghost
Copy link

ghost commented Apr 5, 2016

Thanks, @annevk. To clarify, are we talking about resolving the promise with "default", too, as Chrome does now if the permission dialog is closed? Or are we only going to call the callback with default?

@annevk
Copy link
Member Author

annevk commented Apr 6, 2016

@kitcambridge I think we should do both. We'd say that if the user agent has dismissal UI and the user dismisses, the answer is "default".

@ghost
Copy link

ghost commented Apr 6, 2016

@annevk That sounds reasonable. Thank you for the clarification!

@mnoorenberghe
Copy link

@kitcambridge I think we should do both. We'd say that if the user agent has dismissal UI and the user dismisses, the answer is "default".

That prevents UI like Firefox's where the user can re-open the prompt and make an explicit decision later. It seems like the callback should be un-deprecated and Firefox's UI should be allowed.

@annevk
Copy link
Member Author

annevk commented Apr 7, 2016

@mnoorenberghe for that UI it seems developers should migrate towards https://w3c.github.io/permissions/ which supports changes. I don't think we should try to retrofit legacy permission APIs for that use case.

@annevk
Copy link
Member Author

annevk commented Apr 7, 2016

I could point that out in the draft I suppose.

@sicking
Copy link

sicking commented Apr 7, 2016

The way that the mozilla UI works, there is no way to "dismiss"/"cancel" the dialog. The user can click outside the dialog to minimize it, but that just minimizes it and does not cancel it.

It would not work to automatically dismiss the dialog when we minimize it, and then signal through the permission API if the user bring up the dialog and make a choice. Most pages do not use the permission API to listen for permission changes and so most pages would break if we did that.

Note that I don't really have a strong opinion if other browsers want to have the ability dismiss the dialog and indicate to the website that the user didn't choose "allow" nor "block". Gecko just wouldn't take advantage of such functionality.

@annevk
Copy link
Member Author

annevk commented Apr 8, 2016

@sicking did you read the bug mentioned in OP?

@sicking
Copy link

sicking commented Apr 8, 2016

Ah, ok, I'll let @mnoorenberghe lead the conversation from mozilla's POV here then.

@beverloo
Copy link
Member

beverloo commented Apr 8, 2016

I'm not convinced that changing the callback's behaviour is Web compatible. There is no case right now where it would be invoked multiple times. Code such as the following is definitely not rare:

Notification.requestPermission(status => {
  if (status !== 'denied')
    new Notification(...);
});

Currently both behaviours are allowed per the spec, maintaining such flexibility would have my preference. It's unfortunate that we're seeing sites depending on behaviour of any one browser.

That said, the site where the problem is seen provides a UX where the screen is blacked out until the user interacts with the prompt, even if that's minimize. I don't know how that UX can be provided without resolving with default.

They saw a very similar issue when users used Chrome's incognito mode, which left the screen blacked out indefinitely without any permission prompt at all (we don't enable notifications in incognito). We ended up automatically denying permission after a brief amount of time.

@mnoorenberghe
Copy link

@mnoorenberghe for that UI it seems developers should migrate towards https://w3c.github.io/permissions/ which supports changes. I don't think we should try to retrofit legacy permission APIs for that use case.

I filed w3c/permissions/issues/76 as it's not clear to me that the new API actually supports the current Fx UI while being able to notify developers about "minimized" requests.

@mnoorenberghe
Copy link

I'm not convinced that changing the callback's behaviour is Web compatible. There is no case right now where it would be invoked multiple times. Code such as the following is definitely not rare:

Notification.requestPermission(status => {
if (status !== 'denied')
new Notification(...);
});

Sure, but in that case there is no user-facing issue. Sure it may skew some analytics but that doesn't seem like a big deal. I would guess that most issues related to calling the callback multiple times will have little to no harmful user impact like above snippet. I'm not saying that being able to minimize permission prompts should be required so Chrome wouldn't be affected by any web compat. impact with its current UI anyways.

Currently both behaviours are allowed per the spec, maintaining such flexibility would have my preference. It's unfortunate that we're seeing sites depending on behaviour of any one browser.

Yeah, I'm not suggesting any change that would impact Chrome (or other browsers that don't have minimizable permission prompts). I'm suggesting that the spec make it more clear (e.g. with a MAY) that browsers may call the callback with a value of "default" when a prompt is minimized. This is what was mentioned at #70 (comment)

That said, the site where the problem is seen provides a UX where the screen is blacked out until the user interacts with the prompt, even if that's minimize. I don't know how that UX can be provided without resolving with default.

To be clear, the callback with a value of "default" would solve it too and makes more sense over a Promise since the "minimize" isn't a final response to the request.

@annevk
Copy link
Member Author

annevk commented Apr 12, 2016

@mnoorenberghe the callback just exists for backwards compatibility. I don't think we should diverge it from the promise-based API we've been encouraging folks to switch to. That'd just make a mess of things and does not match what is already shipping in Chrome for this setup.

It does seem somewhat sad if the permission API was solely influenced by Google UX. Hope that's not the case.

@mnoorenberghe
Copy link

@mnoorenberghe the callback just exists for backwards compatibility.

Switching from a callback to a Promise for an API which doesn't have a single answer isn't a backwards compatible change and I'd argue that it was a mistake though see below.

I don't think we should diverge it from the promise-based API we've been encouraging folks to switch to.

I think the callback and the promise can serve two slightly different purposes:

  • the promise is resolved with the final answer for the request (i.e. when there is no more UI for the request in which the user can make a decision)
  • the callback is called for each state change of the request. e.g. "default" then "denied"

That'd just make a mess of things and does not match what is already shipping in Chrome for this setup.

It does match if you use the definitions I just wrote above since all answers in Chrome are final answers IIUC.

It does seem somewhat sad if the permission API was solely influenced by Google UX. Hope that's not the case.

Any permission API that doesn't support minimizable prompts with a permission change after isn't compatible with Firefox's UX.

@beverloo
Copy link
Member

Clicking on the page info bubble in Chrome (i.e. the lock/icon next to the origin in the address bar) allows users to change previously set permissions for that site as well. Arguably that's as easily accessible as the minimized permission requests in Firefox.

@mnoorenberghe
Copy link

Clicking on the page info bubble in Chrome (i.e. the lock/icon next to the origin in the address bar) allows users to change previously set permissions for that site as well. Arguably that's as easily accessible as the minimized permission requests in Firefox.

Right, same with in Firefox but I'd argue that's not part of the request (from .requestPermission) as it can be changed even the page didn't call requestPermission. Letting sites know about those changes is addressed by the Permissions API change handler. In summary, I don't see this as a conflict with what I propose.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

6 participants