Closed Bug 1457915 Opened 2 years ago Closed 2 years ago

disable service workers and push notification on 60 ESR


(Core :: DOM: Service Workers, enhancement, P2)




Tracking Status
relnote-firefox --- 60+
firefox-esr52 --- disabled
firefox-esr60 60+ fixed
firefox59 --- unaffected
firefox60 --- unaffected
firefox61 --- unaffected


(Reporter: bkelly, Assigned: bkelly)



(Keywords: site-compat)


(1 file, 3 obsolete files)

We're not ready to ship service workers on ESR yet.  We need to disable on 60 ESR.
Attached patch disable.patch (obsolete) — Splinter Review
This is the disable patch we used for 52 ESR.  It should still mostly work.  Ryan, do you have an ESR tree you can try it out on?
Assignee: nobody → bkelly
Attachment #8972000 - Flags: feedback?(ryanvm)
Priority: -- → P2
This doesn't apply to ESR60. Note that you could rebase on top of mozilla-release or the FIREFOX_60b_RELBRANCH branch on mozilla-beta if you don't have a clone of that particular tree readily available.
Flags: needinfo?(bkelly)
I'll try to get a try build pushed this morning.
Flags: needinfo?(bkelly)
The patch failed to apply for a few reasons:

- Lots of tests were converted to async function
- dom.serviceWorkers.openWindow.enabled was removed from the tree in FF59
Comment on attachment 8972572 [details] [diff] [review]

Andrea, can you sign off on this?  Its blocking the release process that needs to move forward ASAP.  Thanks.
Attachment #8972572 - Flags: review?(amarchesini)
Attachment #8972572 - Flags: review?(amarchesini) → review+
Add a commit message.  Fix an eslint error flagged by try build.
Attachment #8972572 - Attachment is obsolete: true
Attachment #8972584 - Flags: review+
Fix more tests.  Carrying r+ forward since this is all testonly pref flipping.
Attachment #8972584 - Attachment is obsolete: true
Attachment #8972605 - Flags: review+
Ryan, there are a few orange in the try in comment 8, but they look unrelated to service workers to me.  Can you land this in esr?
Flags: needinfo?(ryanvm)
please set the approval-mozilla-esr60 flag on the patch and fill out the form :)
Flags: needinfo?(ryanvm)
Comment on attachment 8972605 [details] [diff] [review]
Disable service workers and push notifications on 60 ESR. r=baku

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration: Service workers need to be disabled on ESR.  Architectural changes will make it difficult to backport fixes to 60 esr.
User impact if declined: Less maintainable ESR.
Fix Landed on Version: This is a standalone patch for ESR.
Risk to taking this patch (and alternatives if risky): Minimal.  We have disabled service workers on every ESR.
String or UUID changes made by this patch: None
Attachment #8972605 - Flags: approval-mozilla-esr60?
Comment on attachment 8972605 [details] [diff] [review]
Disable service workers and push notifications on 60 ESR. r=baku

thanks, approved for 60.0esr
Attachment #8972605 - Flags: approval-mozilla-esr60? → approval-mozilla-esr60+
Posted the site compatibility note:

Release Note Request (optional, but appreciated)
[Why is this notable]: These features will not be available in Firefox 60 ESR. Both the 45 ESR note [1] and 52 ESR notes [2] mentioned the same disability.
[Affects Firefox for Android]: No
[Suggested wording]: Service workers and push notifications disabled
[Links (documentation, blog post, etc)]: Not necessary, but the compatibility note can be linked if needed

relnote-firefox: --- → ?
Keywords: site-compat
This is included in the ESR60 relnotes as well.
You need to log in before you can comment on or make changes to this bug.