Closed Bug 1215230 Opened 9 years ago Closed 9 years ago

Disable service workers and push notifications on release builds for now

Categories

(Core :: DOM: Service Workers, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla44
Tracking Status
firefox43 + fixed
firefox44 --- fixed

People

(Reporter: ehsan.akhgari, Assigned: ehsan.akhgari)

References

Details

(Keywords: dev-doc-complete)

Attachments

(1 file)

The decision has not been made yet, but filing this bug so that release manageent has a place to look.
I've done this at least once before in bug 1208560.  Let's disable both on trunk and Aurora for non-release builds, and once we're ready to ship we can turn the pref on once and for all!
Summary: Disable service workers on 43 → Disable service workers and push notifications on non-release builds for now
Comment on attachment 8674515 [details] [diff] [review]
Disable service workers and push notifications on non-release builds until we're ready to ship them

Review of attachment 8674515 [details] [diff] [review]:
-----------------------------------------------------------------

Can we use RELEASE_BUILD in mobile.js so its enabled on aurora fennec as well?  Or does no one actually use aurora fennec?

Otherwise, r=me.

::: browser/app/profile/firefox.js
@@ +1951,3 @@
>  
>  // Enable Push API.
>  pref("dom.push.enabled", true);

Is push disabled on beta somehow now?
Attachment #8674515 - Flags: review?(bkelly) → review+
(In reply to Ben Kelly [:bkelly] from comment #3)
> Can we use RELEASE_BUILD in mobile.js so its enabled on aurora fennec as
> well?  Or does no one actually use aurora fennec?

I'm not sure what Fennec plans to do.  Last time we talked mfinkle had some concerns, and we didn't get to any conclusion.  Mark, how do you feel about letting service worker fetch interception ride the trains on Fennec similarly to desktop?

(Depending on the response, I will file a follow-up, don't want to block on it here...)

> Otherwise, r=me.
> 
> ::: browser/app/profile/firefox.js
> @@ +1951,3 @@
> >  
> >  // Enable Push API.
> >  pref("dom.push.enabled", true);
> 
> Is push disabled on beta somehow now?

Yep, I did that in bug 1208560.
Flags: needinfo?(mark.finkle)
Comment on attachment 8674515 [details] [diff] [review]
Disable service workers and push notifications on non-release builds until we're ready to ship them

Approval Request Comment
This disables service workers for Firefox 43.  Not answering the normal questions because they don't make sense here.  :-)
Attachment #8674515 - Flags: approval-mozilla-aurora?
(In reply to Ehsan Akhgari (don't ask for review please) from comment #4)
> (In reply to Ben Kelly [:bkelly] from comment #3)
> > Can we use RELEASE_BUILD in mobile.js so its enabled on aurora fennec as
> > well?  Or does no one actually use aurora fennec?
> 
> I'm not sure what Fennec plans to do.  Last time we talked mfinkle had some
> concerns, and we didn't get to any conclusion.  Mark, how do you feel about
> letting service worker fetch interception ride the trains on Fennec
> similarly to desktop?

Let's ride the trains with Desktop. I don't see any reason to treat Fennec differently at this point.
Flags: needinfo?(mark.finkle)
See Also: → 1215601
(In reply to Mark Finkle (:mfinkle) from comment #7)
> (In reply to Ehsan Akhgari (don't ask for review please) from comment #4)
> > (In reply to Ben Kelly [:bkelly] from comment #3)
> > > Can we use RELEASE_BUILD in mobile.js so its enabled on aurora fennec as
> > > well?  Or does no one actually use aurora fennec?
> > 
> > I'm not sure what Fennec plans to do.  Last time we talked mfinkle had some
> > concerns, and we didn't get to any conclusion.  Mark, how do you feel about
> > letting service worker fetch interception ride the trains on Fennec
> > similarly to desktop?
> 
> Let's ride the trains with Desktop. I don't see any reason to treat Fennec
> differently at this point.

Thanks!  Filed bug 1215601 for that.
Comment on attachment 8674515 [details] [diff] [review]
Disable service workers and push notifications on non-release builds until we're ready to ship them

Approved for uplift to aurora. SW and push API stuff will remain enabled on 43 now until it moves to beta. Then they'll be disabled.
Attachment #8674515 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Edited the bug summary to be less confusing!  Between the bug title and the ifndefs I was losing my mind here :)
Summary: Disable service workers and push notifications on non-release builds for now → Disable service workers and push notifications on release builds for now
https://hg.mozilla.org/mozilla-central/rev/ffd7535a96ba
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
this failed to apply:

grafting 309327:ffd7535a96ba "Bug 1215230 - Disable service workers and push notifications on non-release builds until we're ready to ship them; r=bkelly"
merging browser/app/profile/firefox.js
warning: conflicts during merge.
merging browser/app/profile/firefox.js incomplete! (edit conflicts, then use 'hg resolve --mark')
merging dom/tests/mochitest/general/test_interfaces.html
warning: conflicts during merge.
Flags: needinfo?(ehsan)
Belatedly tracking this since it's a feature change for 43. If there are problems please re-open. Thanks!
I've started updating the SW compat information on MDN finally. A lot of the pages still say that SWs are preffed off in Fx/FxOS by default. Just to be clear, have I basically got it right in this page:

https://developer.mozilla.org/en-US/docs/Web/API/Cache#Browser_compatibility

?

Let me know, and I'll start getting them all updated.
Flags: needinfo?(ehsan)
Chris, Cache API is available without a pref as of FF39.  It can be used outside of service workers.
What Ben said!
Flags: needinfo?(ehsan)
(In reply to Ben Kelly [:bkelly] from comment #16)
> Chris, Cache API is available without a pref as of FF39.  It can be used
> outside of service workers.

Dangit, I completely forgot that Cache was an exception to the rule I was trying to verify ;-) Right, I'll alter the information on those pages, but I'll put the info about enabling SWs in all the other pages (with different version numbers where appropriate...)
I have gone through all the SW pages and updated the Fx compat info, to the best of my knowledge, but a quick review would be a good idea, as there is at least some stuff I'm not sure on

https://developer.mozilla.org/en-US/docs/Web/API/Service_Worker_API

I've also put a note in the relevant release notes about SW finally being enabled (\o/)

https://developer.mozilla.org/en-US/Firefox/Releases/44#Service_Workers
Chris, I know we try to show stuff behind pref's in these compat tables, but I really don't think it makes sense for SW.  A lot of stuff was completely broken in 33.  If you turned on the pref you really couldn't use the feature.  I think we should just say 44 across the board for SW.  Its whats really useful to end developers.  Just my opinion, though.
Flags: needinfo?(cmills)
(In reply to Ben Kelly [:bkelly] from comment #20)
> Chris, I know we try to show stuff behind pref's in these compat tables, but
> I really don't think it makes sense for SW.

+1
+1
(In reply to Ben Kelly [:bkelly] from comment #20)
> Chris, I know we try to show stuff behind pref's in these compat tables, but
> I really don't think it makes sense for SW.  A lot of stuff was completely
> broken in 33.  If you turned on the pref you really couldn't use the
> feature.  I think we should just say 44 across the board for SW.  Its whats
> really useful to end developers.  Just my opinion, though.

Generally, we like to be "as correct as correct can be", but in cases like this where some stuff was supported at a certain point, some stuff wasn't, and some stuff was but it was broken, I think it is a lot easier, more useful and less confusing to just state it as you describe. I've updated the following as examples:

https://developer.mozilla.org/en-US/docs/Web/API/Service_Worker_API
https://developer.mozilla.org/en-US/docs/Web/API/ExtendableEvent
https://developer.mozilla.org/en-US/docs/Web/API/ExtendableEvent/ExtendableEvent
https://developer.mozilla.org/en-US/docs/Web/API/ExtendableEvent/waitUntil

Do these look ok now, or is there anything you'd do differently? Just checking before I go and make the updates.

Cheers.
Flags: needinfo?(cmills)
Chris, can you change these changes as well?

- On ServiceWorker API page, please set install/activate events and fetch event entries to 44.
- On ExtendableEvent/waitUntil, please remove the reference to 33

Thanks!
Flags: needinfo?(cmills)
(In reply to Ben Kelly [:bkelly] from comment #24)
> Chris, can you change these changes as well?
> 
> - On ServiceWorker API page, please set install/activate events and fetch
> event entries to 44.
> - On ExtendableEvent/waitUntil, please remove the reference to 33
> 
> Thanks!

Ack, dammnit. I meant to fix those, but missed them somehow. They are definitely fixed now.

So, I'll assume those pages are now correct, and go update the rest to suit.

Cheers for the input!
Flags: needinfo?(cmills)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: