1203324 - Disable Web Notifications in service workers

Status: RESOLVED FIXED

(Core :: DOM: Push Subscriptions, defect)

Description

[Lina Butler [:lina]]

Since `Clients.openWindow` isn't implemented yet, it's not possible for a service worker to open a window in response to a notification click. This makes for a poor user experience—particularly on Windows, where there's no way to return to a notification once it's been dismissed.

This ticket is a proposal to remove web notifications from service workers. This way, we can still ship Push, but restrict it to background updates only...no user-visible notifications.

Comment 1

[Bill Maggs (bmaggs)]

Assigned to Doug, who can do this or ask someone else to.

Comment 2

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Kit, I'd like to leave this enabled in Nightly builds and switch it off for others.

Comment 3

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6147523214f4

Comment 4

[Bill Maggs (bmaggs)]

(In reply to Nikhil Marathe [:nsm] (please needinfo?) from comment #2)
> Kit, I'd like to leave this enabled in Nightly builds and switch it off for
> others.

Works for me.

Comment 5

[Bill Walker [:bwalker] [@wfwalker]]

(In reply to Nikhil Marathe [:nsm] (please needinfo?) from comment #2)
> Kit, I'd like to leave this enabled in Nightly builds and switch it off for
> others.

+1, for the sake of early adopters and developers I would really like to leave as much of this enabled as possible in Nightly.

Comment 6

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=27f802f7141b

Comment 7

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Attachment: MozReview Request: Bug 1203324 - disable notifications on serviceworkers. r?ehsan,wchen

Bug 1203324 - disable notifications on serviceworkers. r?ehsan,wchen

Per the product discussion, the Notification API should be disabled in
ServiceWorker in release builds for 42 since the UX isn't great [1].

The aim is to release in 44.

[1]: https://mana.mozilla.org/wiki/x/TgAJAw

Comment 8

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=365b22aa4586

Comment 9

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Comment on attachment 8661531 [details]
MozReview Request: Bug 1203324 - disable notifications on serviceworkers. r?ehsan,wchen

Bug 1203324 - disable notifications on serviceworkers. r?ehsan,wchen

Per the product discussion, the Notification API should be disabled in
ServiceWorker in release builds for 42 since the UX isn't great [1].

The aim is to release in 44.

Apologies for the code duplication for pref checking in Notification and
ServiceWorkerRegistration. There isn't a easy way to get
ServiceWorkerRegistration's generated binding to include Notification.h without
having an attribute/method that uses Notification.

[1]: https://mana.mozilla.org/wiki/x/TgAJAw

Comment 10

[William Chen [:wchen]]

Comment on attachment 8661531 [details]
MozReview Request: Bug 1203324 - disable notifications on serviceworkers. r?ehsan,wchen

https://reviewboard.mozilla.org/r/19389/#review17589

Comment 11

[(no longer active)]

https://reviewboard.mozilla.org/r/19387/#review17727

r+

Comment 12

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f2cc5afecb1f3553d4f400b88bdfe2d3991ef8c0
Bug 1203324 - disable notifications on serviceworkers. r=ehsan,wchen

Comment 13

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/f2cc5afecb1f

Comment 14

[Andrew Overholt [:overholt]]

Nikhil, this needs to get uplifted to 42 (beta at this point), right?

Comment 15

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Comment on attachment 8661531 [details]
MozReview Request: Bug 1203324 - disable notifications on serviceworkers. r?ehsan,wchen

Approval Request Comment
[Feature/regressing bug #]: Bug 1114554
[User impact if declined]: We will expose a feature that is not as polished as we would like. Please see comment 7 for link to notification and push release plan
[Describe test coverage new/current, TreeHerder]:
[Risks and why]: Low, adds a pref off by default to disable the feature.
[String/UUID change made/needed]: None.

Comment 16

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8661531 [details]
MozReview Request: Bug 1203324 - disable notifications on serviceworkers. r?ehsan,wchen

aurora is now 43. Taking it for 42.

Comment 17

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/0766c6eeddb1

Comment 18

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

(In reply to Sylvestre Ledru [:sylvestre] from comment #16)
> Comment on attachment 8661531 [details]
> MozReview Request: Bug 1203324 - disable notifications on serviceworkers.
> r?ehsan,wchen
> 
> aurora is now 43. Taking it for 42.

We also need this in Aurora since the timeline for release of this feature is 44.

Comment 19

[Sylvestre Ledru [:Sylvestre]]

yes Nikhil and it is already the case. See comment #13, the patch lands in m-c before the merge (and when m-c was still 43 and not 44)
"status-firefox43: affected → fixed"