Last Comment Bug 772638 - Disable webapps support on Firefox 15
: Disable webapps support on Firefox 15
Status: VERIFIED FIXED
[qa!]
:
Product: Firefox Graveyard
Classification: Graveyard
Component: Web Apps (show other bugs)
: unspecified
: All All
: P1 normal
: Firefox 15
Assigned To: Myk Melez [:myk] [@mykmelez]
: Jason Smith [:jsmith]
:
Mentors:
Depends on:
Blocks: 765063
  Show dependency treegraph
 
Reported: 2012-07-10 13:55 PDT by Jason Smith [:jsmith]
Modified: 2016-02-04 15:00 PST (History)
11 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch v1: disable navigator.mozApps and webapp runtime (1.84 KB, patch)
2012-07-12 14:56 PDT, Myk Melez [:myk] [@mykmelez]
felipc: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Splinter Review
patch v2: fixes test failures (1.77 KB, patch)
2012-07-14 15:37 PDT, Myk Melez [:myk] [@mykmelez]
felipc: review+
lukasblakk+bugs: approval‑mozilla‑beta+
Details | Diff | Splinter Review

Description Jason Smith [:jsmith] 2012-07-10 13:55:57 PDT
Per discussion in today's triage, we have decided to disable webapps support on FF 15. Will add rationale for this shortly in a followup comment. This bug tracks to disable webapps for FF 15.
Comment 1 Jason Smith [:jsmith] 2012-07-10 14:18:54 PDT
The major rationale for disabling web apps on FF 15 is that there are still blocker patches that was not uplifted, one in particular that was a fundamental API change to desktop web apps as a whole. The docs themselves are not ready for general consumption either, although that affects this decision, but does not finalize it to disable. There's also a bunch of bugs that were not uplifted in general to Aurora, so they still sit in Nightly. There's still outstanding blockers too even right now for targeting FF 16 (currently 6). Therefore, we're hesitant to leave this functionality active and want to disable it.

References:

Blockers that are not uplifted - https://bugzilla.mozilla.org/buglist.cgi?order=Bug%20Number;resolution=FIXED;status_whiteboard_type=allwordssubstr;query_format=advanced;status_whiteboard=[blocking-webrtdesktop1%2B];bug_status=RESOLVED;bug_status=VERIFIED;bug_status=CLOSED;component=Web%20Apps;component=Webapp%20Runtime;product=Firefox;target_milestone=Firefox%2016;list_id=3666368
Other bugs that are not uplifted - https://bugzilla.mozilla.org/buglist.cgi?order=Bug%20Number;resolution=FIXED;query_format=advanced;bug_status=RESOLVED;bug_status=VERIFIED;bug_status=CLOSED;component=Web%20Apps;component=Webapp%20Runtime;product=Firefox;target_milestone=Firefox%2016;list_id=3666367
Remaining blockers in webrt - https://bugzilla.mozilla.org/buglist.cgi?order=Assignee;resolution=---;status_whiteboard_type=allwordssubstr;status_whiteboard=[blocking-webrtdesktop1%2B];list_id=3665880
Comment 2 Myk Melez [:myk] [@mykmelez] 2012-07-10 14:22:06 PDT
Note: the desktop runtime feature drivers are the ones proposing that webapps be disabled, but it's the Firefox channel drivers who ultimately make the decision.  lmandel is going to bring it up in their next regularly-scheduled meeting.
Comment 3 Lukas Blakk [:lsblakk] use ?needinfo 2012-07-10 14:59:38 PDT
lmandel did bring it up at the channel meeting and we're good with this decision. Will track this bug to make sure we have everything lined up either pre or post merge on the 16th.
Comment 4 Myk Melez [:myk] [@mykmelez] 2012-07-12 14:56:40 PDT
Created attachment 641613 [details] [diff] [review]
patch v1: disable navigator.mozApps and webapp runtime
Comment 5 Myk Melez [:myk] [@mykmelez] 2012-07-13 01:11:51 PDT
Comment on attachment 641613 [details] [diff] [review]
patch v1: disable navigator.mozApps and webapp runtime

[Approval Request Comment]
Bug caused by (feature/regressing bug #): webapps feature
User impact if declined: Users will be exposed to an incomplete/buggy feature.
Testing completed (on m-c, etc.): This is equivalent to the patch we previously landed to disable webapps for Firefox 14 (bug 750936), and it is well tested on Firefox 14 beta builds.
Risk to taking this patch (and alternatives if risky): There is little risk and no alternatives.
String or UUID changes made by this patch: None.
Comment 6 Alex Keybl [:akeybl] 2012-07-13 13:50:13 PDT
Comment on attachment 641613 [details] [diff] [review]
patch v1: disable navigator.mozApps and webapp runtime

[Triage Comment]
Approved for Aurora 15 - please land ASAP to make Monday's merge.
Comment 7 Myk Melez [:myk] [@mykmelez] 2012-07-13 14:10:42 PDT
https://hg.mozilla.org/releases/mozilla-aurora/rev/c2504f7b08b3
Comment 8 Myk Melez [:myk] [@mykmelez] 2012-07-13 18:22:27 PDT
Reopening, as I backed out the change:

https://tbpl.mozilla.org/?tree=Mozilla-Aurora&rev=c2c2c087c4aa

due to test failures:

https://tbpl.mozilla.org/?tree=Mozilla-Aurora&rev=c2504f7b08b3

because I forgot to disable tests for the functionality being disabled.
Comment 9 Myk Melez [:myk] [@mykmelez] 2012-07-14 15:37:21 PDT
Created attachment 642278 [details] [diff] [review]
patch v2: fixes test failures

Here's an updated patch that fixes the test failures.  It disables the navigator.mozApps API tests, but it leaves the nsIAppsService tests enabled, so it leaves dom_apps.xpt alone (the patch for Firefox 14 removed it, and my first patch for this bug did the same).

dom_apps.xpt provides nsIDOMApplicationRegistry.idl and nsIAppsService.idl (in Firefox 14 it only provided the former, which is why the nsIAppsService tests didn't fail there with the equivalent patch), neither of which seem necessary with webapps disabled.  But it also doesn't seem necessary to disable them, as it is sufficient to disable the mozApps API.  And disabling them is riskier than disabling the runtime, which has a simple build flag for that purpose.

Thus, in the interest of making the lowest-risk change to disable webapps, this patch disables the mozApps API and the runtime, leaving those XPIDL files alone.

I built and tested with this patch (I wish we had a TryServer for Aurora!), and all mochitests now pass.  I then packaged the installer, installed Aurora with it, and confirmed that the mozApps API and the runtime are not available in the installation.
Comment 10 :Felipe Gomes (needinfo me!) 2012-07-15 15:08:29 PDT
Comment on attachment 642278 [details] [diff] [review]
patch v2: fixes test failures

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

Thanks Myk, sorry for not remembering about the tests on the previous patch
Comment 11 Myk Melez [:myk] [@mykmelez] 2012-07-15 18:54:05 PDT
Comment on attachment 642278 [details] [diff] [review]
patch v2: fixes test failures

Requesting approval of the updated patch, which fixes the test failures from the previously approved patch that caused me to back out that patch.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): webapps feature
User impact if declined: Users will be exposed to an incomplete/buggy feature.
Testing completed (on m-c, etc.): This is equivalent to the patch we previously landed to disable webapps for Firefox 14 (bug 750936), and it is well tested on Firefox 14 beta builds.
Risk to taking this patch (and alternatives if risky): There is little risk and no alternatives.
String or UUID changes made by this patch: None.
Comment 12 Myk Melez [:myk] [@mykmelez] 2012-07-16 13:39:57 PDT
Comment on attachment 642278 [details] [diff] [review]
patch v2: fixes test failures

Requesting approval to land this on beta instead now that the merge from aurora to beta has happened.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): webapps feature
User impact if declined: Users will be exposed to an incomplete/buggy feature.
Testing completed (on m-c, etc.): This is equivalent to the patch we previously landed to disable webapps for Firefox 14 (bug 750936), and it is well tested on Firefox 14 beta builds.
Risk to taking this patch (and alternatives if risky): There is little risk and no alternatives.
String or UUID changes made by this patch: None.
Comment 13 Lukas Blakk [:lsblakk] use ?needinfo 2012-07-16 15:08:51 PDT
Comment on attachment 642278 [details] [diff] [review]
patch v2: fixes test failures

Approving, please get this in asap so we can have this off in our first beta.
Comment 14 Myk Melez [:myk] [@mykmelez] 2012-07-16 15:55:52 PDT
https://hg.mozilla.org/releases/mozilla-beta/rev/ad743c3a7d20
Comment 15 Lukas Blakk [:lsblakk] use ?needinfo 2012-07-24 11:16:17 PDT
Changing status to 'fixed' since the goal of this bug was to disable, and that was accomplished.  In other bugs 'disabled' is a state between reported and fixed (hope that makes sense).

Note You need to log in before you can comment on or make changes to this bug.