Disable Web Notifications in service workers

RESOLVED FIXED in Firefox 42

Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: lina, Assigned: nsm)

Tracking

(Blocks 1 bug)

Trunk
mozilla43
Points:
---

Firefox Tracking Flags

(firefox42 fixed, firefox43 fixed)

Details

User Story

Acceptance Criteria: 

1. Verify that service workers cannot open a Web Notification, in other words that 1130687 is not fixed. This must be true for non-e10s windows, but should be true for e10s windows as wel.
2. Verify using test applications like people.mozilla.org/~ewong2/push-notification-test/ that a service worker cannot create a visible notification on screen.

Attachments

(1 attachment)

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.
(Reporter)

Updated

4 years ago
See Also: → 1130687
(Reporter)

Updated

4 years ago
See Also: → 1203326
Depends on: 1201571
Assigned to Doug, who can do this or ask someone else to.
Assignee: nobody → dougt
Status: NEW → ASSIGNED
Assignee: dougt → nsm.nikhil
Kit, I'd like to leave this enabled in Nightly builds and switch it off for others.
(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.
(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.
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
Attachment #8661531 - Flags: review?(wchen)
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 on attachment 8661531 [details]
MozReview Request: Bug 1203324 - disable notifications on serviceworkers. r?ehsan,wchen

https://reviewboard.mozilla.org/r/19389/#review17589
Attachment #8661531 - Flags: review?(wchen) → review+
Blocks: 1201571
No longer depends on: 1201571
https://hg.mozilla.org/mozilla-central/rev/f2cc5afecb1f
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
Nikhil, this needs to get uplifted to 42 (beta at this point), right?
Flags: needinfo?(nsm.nikhil)
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.
Flags: needinfo?(nsm.nikhil)
Attachment #8661531 - Flags: approval-mozilla-beta?
Attachment #8661531 - Flags: approval-mozilla-aurora?
Comment on attachment 8661531 [details]
MozReview Request: Bug 1203324 - disable notifications on serviceworkers. r?ehsan,wchen

aurora is now 43. Taking it for 42.
Attachment #8661531 - Flags: approval-mozilla-beta?
Attachment #8661531 - Flags: approval-mozilla-beta+
Attachment #8661531 - Flags: approval-mozilla-aurora?
Attachment #8661531 - Flags: approval-mozilla-aurora-
(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.
Flags: needinfo?(sledru)
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"
Flags: needinfo?(sledru)

Updated

4 years ago
See Also: → 1208560

Updated

4 years ago
User Story: (updated)
You need to log in before you can comment on or make changes to this bug.