601477 - Desktop Notifications in Fennec require user action before any notification

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Paul Rouget [:paul]]

Calling notification.show() brings the "foo.com wants to use notifications. Allow - Don't allow" prompt every time.

A "Always allow" option should be added and the prompt should be shown on createNotification(), not on show().

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

We currently use the same mechanism that geo-location uses. After answering "allow" 5 times, we never ask again for that domain.

Comment 2

[Doug Turner (:dougt)]

i was mostly concerned about the "the prompt should be shown on createNotification(), not on show()."

not sure if I agree, but i will discuss with other implementers.  paul, any reason why you do not like the permission prompt on show()?

Comment 3

[Paul Rouget [:paul]]

The notification mechanism exists to attract the attention of the user, even if the browser is in the background.

With current behavior, the notification is only shown if the user is in front of the browser.

I think it would be better if:
* the user load a web page
* web page asks for the permission to show notification (on createNotifaction or on via another dedicated request)
* notification happens later in the future

Comment 4

[Doug Turner (:dougt)]

yeah, i think you are right.

Comment 5

[Doug Turner (:dougt)]

oh, i should point out that we have been discussing a permission api such that you can ask the user at any point for a set of permissions.  For example, something like:

navigator.requestPermissions("geolocation, desktop-notifications", callback())

This would also address the problem, but i will think you are right.

Comment 6

[Doug Turner (:dougt)]

s/will/still

Comment 7

[Doug Turner (:dougt)]

Attachment: patch v.1

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Could you explain the behavior the patch gives.

Comment 9

[Doug Turner (:dougt)]

yes.  the patch will change the behavior of prompting when .show() is called to when createNotification() is called.  This will allow sites to request permission prior to the show().

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

What happens if one does createNotification().show() ?

Comment 11

[Doug Turner (:dougt)]

we remember that show() was called, and when the notification is approved, we post a desktop notification.  Is that what you are asking?

Comment 12

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 502571 [details] [diff] [review]
patch v.1


> 
> void
>+nsDOMDesktopNotification::SetAllow(PRBool aAllow)
>+{
>+  mAllow = aAllow;
>+
>+  // if we have called Show() already, lets go ahead and post a notification
>+  if (mShowHasBeenCalled)
>+    PostDesktopNotification();
>+}
>+

Why do you call PostDesktopNotification() even is aAllow is false?

Comment 13

[Doug Turner (:dougt)]

as an exercise for the reviewer! :D

Comment 14

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 502571 [details] [diff] [review]
patch v.1

Ok, fix that :)

I wish there were some tests...

Comment 15

[Doug Turner (:dougt)]

http://hg.mozilla.org/mozilla-central/rev/f8b39d7c6fff

Comment 16

[Anna (Waverley)]

Can you provide some steps to reproduce so that i could verify this bug ?

Comment 17

[Cristian Nicolae (:xti)]

Is it http://vingtetun.org/todo/ a proper website to verify this bug? If no, could you suggest me one please?