1264797 - Do not persist web notifications beyond Gecko session

Status: RESOLVED INCOMPLETE

(Firefox for Android Graveyard :: General, defect, P3)

Description

[Chenxia Liu [:liuche]]

[Tracking Requested - why for this release]: Moving tracking flags from bug 1229835.

We persist Web notifications created by the Notifications API beyond Gecko death, which leaves us in a state where we are unable to retrieve a url. Web Notifications should not be persistent, so we should remove them from the notification center on Gecko death.

https://notifications.spec.whatwg.org/#persistent-notification

Comment 1

[Liz Henry (:lizzard) (relman/hg->git project)]

Cleanup from another bug, tracking. Note that it was tracking for fennec:48 not for firefox. 
Is this something we need to ship in 48 or is it ok for  it to ride with 49?

Comment 2

[James Willcox (:snorp) ([e-mail]) (he/him)]

Note that *some* notifications are persistent. Notably, those created via Service Workers. Bug 1264815 is dealing with that case. We need to be careful that we don't undo that.

Comment 3

[Michelle Funches - QA]

Meets expectation on Fennec Nightly 50.0a1
Notification not specific to "persistent" will close when Fx is closed.

Comment 4

[Michelle Funches - QA]

Meets expectation on Fennec Nightly 50.0a1
Notification is "persistent" The notification will remain when Firefox is closed. Acting on the notification accurately reopens  Firefox.

Comment 5

[SUN Haitao]

Attachment: new-notification.html

Comment 6

[SUN Haitao]

I believe this isn't actually finished yet. It can still be reproduced on Fennec 50.0a1 Build ID 20160623030210 with the following steps:

0. Open https://bug1264797.bmoattachments.org/attachment.cgi?id=8764880 in Fennec.
1. Click the button. A temporary notification appears (after confirmation).
2. Close Fennec. The notification still stays in the notification list.

Comment 7

[Gerry Chang [:gchang]]

Hi Chenxia,
Do you have any updates for the issue?

Comment 8

[Chenxia Liu [:liuche]]

Sorry about that - I haven't been working on this.

However, this should be as simple as finding the pathway where the web notification (and only web notification) is created, and setting setOngoing(false) for that notification (or passing in some kind of flag so that boolean is set).

If someone wants to pick this up, please feel free to take the bug! Otherwise, I will work on it probably this week or the next.

Comment 9

[SUN Haitao]

(In reply to Chenxia Liu [:liuche] from comment #8)
> However, this should be as simple as finding the pathway where the web
> notification (and only web notification) is created, and setting
> setOngoing(false) for that notification (or passing in some kind of flag so
> that boolean is set).

I'm afraid it is not that simple. Actually, all web notifications are already non-ongoing by default. But Android never close notifications automatically.

Maybe we should consider using snack-bars to render temporary notifications (As the specification suggested). Their semantics are closer.

Comment 10

[Michelle Funches - QA]

QA Confirmation: Latest test execution on Aurora & Persistent Notifications are a PASS
Version 	50.0a2
Build ID 	20160804004004
Gecko
Desired: Notifications are dismissed when Firefox is closed.
Fennec
Desired Persistent: Close via Recent Apps Firefox reopens from Notification
Desired Persistent: Close via Task Manager and Notification is closed when Fx closes

Comment 11

[Gerry Chang [:gchang]]

Hi Michelle,
Can we get a regression window when this issue was fixed?

Comment 12

[Michelle Funches - QA]

Hi Gerry,

Desktop applying Push Notification Test page
 7:06.86 INFO: Looks like the following bug has the changes which introduced the fix:
https://bugzilla.mozilla.org/show_bug.cgi?id=1224771

Desktop applying serviceworke.rs/push simple
 8:29.01 INFO: First good revision: f07e71078bc8991f74c2101944c8f869c77f442a
 8:29.01 INFO: Last bad revision: cdcd33fd6e39cd12feb5bb11951e1c981a04bd86
 8:29.01 INFO: Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=cdcd33fd6e39cd12feb5bb11951e1c981a04bd86&tochange=f07e71078bc8991f74c2101944c8f869c77f442a

Comment 13

[Sylvestre Ledru [:Sylvestre]]

Too late to do anything for 48

Comment 14

[Gerry Chang [:gchang]]

Hi Michelle,
In comment #6, the issue can be reproduced in 20160623030210 and in your comment #10, the issue is fixed in 20160804004004. Can you help to get a window when the fix is introduced between 20160623030210 and 20160804004004?

Comment 15

[SUN Haitao]

But temporary notifications should also be dismissed when closing Fennec via the Recent Apps list. So the behavior of 20160804004004 is still not right enough.

Comment 16

[Michelle Funches - QA]

I will mozregress for dates requested and post the updates asap.

Also can we get a meeting to discuss & clarify this. (Sun, Gerry) and whom ever else I may not have listed.
Also please invite Kit as I checked functionality in comment 12 with him, so I need his input to match what I will confirm.

1) Non-persistent notification - close when Firefox is closed on Desktop or Firefox. The notification should not remain where it can be acted upon on the desktop or on devices notification area.
2) Persistent notification Desktop - Notification on desktop will close when Firefox is closed. If notification is seen on desktop it will close.
3) Persistent notification Android - Notification will remain in the notification area upon closing via recent apps which still allows Firefox process to remain open... check the task manager - closing via recent apps does not close FX completely.
4) Persistent notification Android - Notification will close when Firefox is closed via Task Manager which does stop the process.
5) Non-persistent notification Android - always closed when Fx is closed.

Comment 17

[Michelle Funches - QA]

QA Update: [Notification is to be removed from view on desktop and not display in notification area on device.]
Persistent Notification on Desktop - not working as expected. A persistent notification should close when Gecko closes.
Persistent Notification on Android - now working as expected. A persistent notification should close when Recent Apps is applied to close the browser.

Comment 18

[SUN Haitao]

According to my understanding of https://notification.spec.whatwg.org/ ，the suggested behaviors are:

* Non-persistent notifications disappear after being showed a few seconds. They do not appears in the system notification center. If the browser process ends before they disappear, nothing happens while clicking them.

* Persistent notifications appears in the system notification center. They stay there no matter what happened to the browser process. While clicking/closing them, the browser process will be activated (or relaunched if necessary).

Nick Alexander suggests we "show these [Non-persistent] notifications in the system tray, for some amount of time" in https://bugzilla.mozilla.org/show_bug.cgi?id=1229835#c14 . But based on my recent experiences, the behavior in specification (Making Non-persistent notifications behave like Android Snackbars) may be more handy.

Comment 19

[Gerry Chang [:gchang]]

Hi Jim,
Since you fixed bug 1267419, do you think you can help on this and provide more information?

Comment 20

[(inactive) Jim Chen [:jchen] [:darchons]]

(In reply to Gerry Chang [:gchang] from comment #19)
> Hi Jim,
> Since you fixed bug 1267419, do you think you can help on this and provide
> more information?

I think comment 18 makes sense. The current behavior for persistent notifications is correct but for temporary notifications we should switch to a timeout or a snackbar; it's a UX decision.

Comment 21

[Liz Henry (:lizzard) (relman/hg->git project)]

Sebastian or barbara, can you help here with the decision and figuring out what we want to do?
Wontfix at this point for 49 as we are heading into beta 8.

Comment 22

[Liz Henry (:lizzard) (relman/hg->git project)]

[Tracking Requested - why for this release]:

Comment 23

[Sebastian Kaspari (:sebastian; :pocmo)]

@barbara: Can you add an aha card for this? We already released this as-is (48), so I don't consider this a P1 to fix immediately.

> I think comment 18 makes sense. The current behavior for persistent
> notifications is correct but for temporary notifications we should switch to
> a timeout or a snackbar; it's a UX decision.

Yeah, a snackbar sounds like a good approach. However we can only display snackbars inside the app UI (and not when the app is not in the foreground). Has the spec any opinion on that? If we would use a snackbar and the app is in the background then no "notification" would be shown - is this okay?

Adding and removing an actual Android notification sounds a bit hacky. It sounds like this API was designed with a desktop browser in mind. However you can set a "ticker" text [1] for a notification. This text will appear in the status bar. If you immediately remove the notification then this will only appear in the status bar for a short time and no notification will be in the notification shade afterwards. Still hacky but maybe okay for this.

What does chrome do?

[1] https://developer.android.com/reference/android/app/Notification.Builder.html#setTicker(java.lang.CharSequence,%20android.widget.RemoteViews)

Comment 24

[Barbara Bermes [:barbara] - NI please!]

Adding UX to this (Anthony).

Something to think about in the future.

I'm not in favour of the hacks described (except for the last one Sebastian mentioned), but want to hear Anthony's view as well.

Comment 25

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

(In reply to Chenxia Liu [:liuche] from comment #0)
> [Tracking Requested - why for this release]: Moving tracking flags from bug
> 1229835.
> 
> We persist Web notifications created by the Notifications API beyond Gecko
> death, which leaves us in a state where we are unable to retrieve a url. Web
> Notifications should not be persistent, so we should remove them from the
> notification center on Gecko death.
> 
> https://notifications.spec.whatwg.org/#persistent-notification

taking a step back here, what's the issue with seeing a notification hang around? doesn't tapping it still reopen Firefox and the correct URL?

Comment 26

[SUN Haitao]

(In reply to Sebastian Kaspari (:sebastian) from comment #23)
> What does chrome do?

Currently, mobile Chrome does not support creating non-persistent notifications at all. So we have plenty of time to discuss this.

Comment 27

[Sebastian Kaspari (:sebastian; :pocmo)]

(In reply to Anthony Lam (:antlam) from comment #25)
> taking a step back here, what's the issue with seeing a notification hang
> around? doesn't tapping it still reopen Firefox and the correct URL?

It should. However the spec says:
* "User agents should run the close steps for a non-persistent notification a couple of seconds after they have been created."
* "User agents should not display non-persistent notification in a platform’s "notification center" (if available)."

Comment 28

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hi Barbara, Sebastian,

Comment 29

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hi guys, is this planned for Fx50? Seems unlikely. I noticed that it says tracking-fennec 49+. I am not going to track this as it's not something we have any plans for in the near future. Please correct me if I am wrong.

Comment 30

[Sebastian Kaspari (:sebastian; :pocmo)]

(In reply to Ritu Kothari (:ritu) from comment #29)
> Hi guys, is this planned for Fx50? Seems unlikely. I noticed that it says
> tracking-fennec 49+. I am not going to track this as it's not something we
> have any plans for in the near future. Please correct me if I am wrong.

Yeah, no one is currently working on this. I'll add a whiteboard tag to discuss this with the new team starting in Taipei. This could be something for them (depending on barbara's prioritization). But for now this doesn't need to track anything because that doesn't make anyone working on it. :)

Comment 31

[SUN Haitao]

(In reply to Sebastian Kaspari (:sebastian) from comment #23)
> Yeah, a snackbar sounds like a good approach. However we can only display
> snackbars inside the app UI (and not when the app is not in the foreground).
> Has the spec any opinion on that? If we would use a snackbar and the app is
> in the background then no "notification" would be shown - is this okay?
Toasts can be displayed by background apps. And their semantics are almost as same as snack-bars. Is it okay to using them?

Comment 32

[Sebastian Kaspari (:sebastian; :pocmo)]

(In reply to SUN Haitao from comment #31)
> Toasts can be displayed by background apps. And their semantics are almost
> as same as snack-bars. Is it okay to using them?

Maybe. There are some limitations and I do not know if this is spec compliant:
* Toasts are only shown a short amount of time and this is not configurable (there's only SHORT or LONG)
* Toasts are not clickable, so they can only be used to display text but not have an action assigned (We have a custom implementation of toasts that allows to add an action but we either have removed it or are going to as soon as all implementations are replaced with snackbars)
* Toasts can only display a very short text and you usually cannot see which app is showing the toast (if it is not the app that is currently in the foreground)

Comment 33

[Barbara Bermes [:barbara] - NI please!]

as per Sebastian, we can discuss further with the TDC team.

Comment 34

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

changed to + because it still takes some efforts to deep dive into the spec and come out solution

Comment 35

[BMO Automation]

We have completed our launch of our new Firefox on Android. The development of the new versions use GitHub for issue tracking. If the bug report still reproduces in a current version of [Firefox on Android nightly](https://play.google.com/store/apps/details?id=org.mozilla.fenix) an issue can be reported at the [Fenix GitHub project](https://github.com/mozilla-mobile/fenix/). If you want to discuss your report please use [Mozilla's chat](https://wiki.mozilla.org/Matrix#Connect_to_Matrix) server https://chat.mozilla.org and join the [#fenix](https://chat.mozilla.org/#/room/#fenix:mozilla.org) channel.