766297 - Disable addons in webapps

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Web Apps (PWAs), defect)

Description

[Wesley Johnston (:wesj)]

We don't need addons in webapps and there's no UI to manage them anyway. Can disable addons entirely in this case (would be nice to try and get back the startup time as well if we can?)

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

AFAIK, we would like to support add-ons in webapps eventually, just not in the initial release. Keep that in mind when building a way to disable them.

Comment 2

[Wesley Johnston (:wesj)]

Attachment: Patch

Idea here is to match desktop behavior for now. This adds a default prefs.js to each webapps profile when its created. It also moves profile creation to when the app is installed (rather than when its first run).

I copied the prefs file from desktop:
http://mxr.mozilla.org/mozilla-central/source/webapprt/prefs.js?raw=1

Comment 3

[Wesley Johnston (:wesj)]

Whoops. I also left in some prefs I removed that I thought we didn't need. That probably shouldn't be here.... but:

pref("browser.tabs.warnOnClose", true); - not supported
pref("browser.tabs.remote", false); - not supported
pref("toolkit.screen.lock", false); - maemo only?

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 635129 [details] [diff] [review]
Patch

># HG changeset patch
># Parent 84272dbc8cb24fd0861379b62817d130c4a2f006
>
>diff --git a/mobile/android/app/mobile.js b/mobile/android/app/mobile.js

>-pref("browser.tabs.warnOnClose", true);
>-pref("browser.tabs.remote", false);
>-
>-pref("toolkit.screen.lock", false);

Do not remove browser.tabs.remote yet. That is covered in bug 732498

>diff --git a/mobile/android/base/GeckoAppShell.java b/mobile/android/base/GeckoAppShell.java

>+    public static void installWebApp(String aTitle, String aURI, String aUniqueURI, String aIconURL) {
>+        int index = WebAppAllocator.getInstance(GeckoApp.mAppContext).findAndAllocateIndex(aUniqueURI);
>+        GeckoProfile profile = GeckoProfile.get(GeckoApp.mAppContext, "webapp" + index);
>+        File prefs = profile.getFile("prefs.js");

What happens if we can't find the webapp?

>diff --git a/mobile/android/base/Makefile.in b/mobile/android/base/Makefile.in

>+RES_RAW = \
>+  res/raw/prefs_js \

webapp_prefs_js ?

>diff --git a/mobile/android/base/resources/raw/prefs_js b/mobile/android/base/resources/raw/prefs_js

>+/* This Source Code Form is subject to the terms of the Mozilla Public
>+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
>+ * You can obtain one at http://mozilla.org/MPL/2.0/. */

Not sure we should have the license in this file, but if it works, I don't really care. I guess you are just copying desktop.

r+, but look into the error handling in installWebapp to make sure we are robust. If we are not very robust anywhere we findAndAllocateIndex a webapp, then file a followup bug to improve the situation.

Comment 5

[Wesley Johnston (:wesj)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/04fef47c3fae

Not sure about findAndAllocate. It should be guaranteed not to fail. Why do we want error handling?

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Wesley Johnston (:wesj) from comment #5)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/04fef47c3fae
> 
> Not sure about findAndAllocate. It should be guaranteed not to fail. Why do
> we want error handling?

I just wanted you to audit the code for BOOM! CRASH! situations happening if the webapp was not found. If the code looks robust, then I'm fine with it landing.

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/04fef47c3fae

Comment 8

[Wesley Johnston (:wesj)]

Grr... I thought prefs like these would work, but Gavin tells me you can't do that. Alternatively, we have WebAppsRT.js. I guess we can set some prefs in the init section there. That runs before the telemetry prompt so this should be fine.