Closed Bug 1203324 Opened 10 years ago Closed 10 years ago

Disable Web Notifications in service workers

Categories

(Core :: DOM: Push Subscriptions, defect)

Product:
Core
Component:
DOM: Push Subscriptions
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla43
Tracking Flags:
Tracking Status
firefox42 --- fixed
firefox43 --- fixed

People

(Reporter: lina, Assigned: nsm)

References

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 file)

MozReview Request: Bug 1203324 - disable notifications on serviceworkers. r?ehsan,wchen
40 bytes, text/x-review-board-request
wchen
: review+
Sylvestre
: approval-mozilla-aurora-
Sylvestre
: approval-mozilla-beta+
Details
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.
See Also: → 1130687
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
Closed: 10 years ago
status-firefox43: affected → fixed
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)
See Also: → 1208560
User Story: (updated)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: