756976 - Loading a .webapp URL in Firefox should prompt to install the app, not prompt to download the manifest

Status: RESOLVED WONTFIX

(Firefox Graveyard :: Web Apps, enhancement, P2)

Description

[Matthew N. [:MattN]]

STR:
1) Load a webapp URL directly (see URL field)

Actual result:
Firefox offers to download the manifest file

Expected result:
Firefox should offer to install the application (like we do for XPIs).

Offering a download of the manifest is not useful very useful for users or developers.  Developers would probably prefer to see the JSON response in the tab like we do for application/json. Users would probably want the app to install.

Comment 1

[:Felipe Gomes (needinfo for replies!)]

Not offering to install the app is done on purpose, as an app install has to be tied to an origin to be checked against the list of allowed origins on the manifest.

I suggest that we change this bug to be about displaying the manifest as plain text instead of offering to download it. This would ease development/debugging for developers.

Comment 2

[:Felipe Gomes (needinfo for replies!)]

Relevant links provided by bz and MattN:
http://hg.mozilla.org/mozilla-central/rev/10c2c74197fc
https://mxr.mozilla.org/mozilla-central/source/content/html/document/src/nsHTMLDocument.cpp#562

Comment 3

[Ian Bicking (:ianbicking)]

I would assume that installing this way would be equivalent to a self-install.

Comment 4

[[:fabrice] Fabrice Desré]

I agree with Ian. Let's add a content-type handler for that.

Comment 5

[Ian Bicking (:ianbicking)]

I think this proposal would conflict with the spirit of installs_allowed_from – a store could create a link labelled "install" that links directly to the manifest, triggering what looks exactly like a navigator.mozApps.install invocation.

Comment 6

[Myk Melez [:myk] [@mykmelez]]

Conversation is taking place in the dev-webapps mailing list.  Let's keep it there and reserve comments in this bug for implementation discussion.

Comment 7

[Jason Smith [:jsmith]]

Random question - An implementation of this bug would apply to all gecko platforms, not just desktop firefox? Or would this bug only apply to desktop firefox?

Comment 8

[[:fabrice] Fabrice Desré]

(In reply to Jason Smith [:jsmith] from comment #7)
> Random question - An implementation of this bug would apply to all gecko
> platforms, not just desktop firefox? Or would this bug only apply to desktop
> firefox?

We can do both easily, but I don't see the point of doing it only for desktop.

Comment 9

[Jennifer Arguello :ticachica]

Marking this a P2 as it is a good feature enhancement that supports easier sharing of apps across the web. The feature here would be to add app install support for a manifest URL.

Re: comment 5, the implementation of this bug will adhere to the use of "installs_allowed_from" in the manifest.  The argument surrounding "installs_allowed_from" is a separate issue, outside of scope of this bug's implementation and can be followed in the dev-webapps mailing list.

Comment 10

[Matthew N. [:MattN]]

Seems like we can implement nsIContentHandler[1] for application/x-web-app-manifest+json similar to how it's done for XPIs: 
https://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/amContentHandler.js#18

[1] https://mxr.mozilla.org/mozilla-central/source/uriloader/base/nsIContentHandler.idl

Comment 11

[Marco Castelluccio [:marco]]

Attachment: WIP

This is the "infrastructure" of the patch.
I need to find a way to actually install the app given its manifest URL (I can't simply use mozIDOMApplicationRegistry.install because it assumes a Window.

Comment 12

[Marco Castelluccio [:marco]]

Attachment: Patch

Comment 13

[Marco Castelluccio [:marco]]

There's a problem with the patch, that I still need to solve.
If you're on a page (for example "about:home") and insert an URL to a manifest that is on a different domain (for example "http://marco-c.github.com/ceferinoweb/ceferino.webapp"), the notification string contains the wrong URL (it contains "about:home" instead of "marco-c.github.com").

Comment 14

[Myk Melez [:myk] [@mykmelez]]

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

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

::: toolkit/webapps/WebappsContentHandler.js
@@ +7,5 @@
> +const Cc = Components.classes;
> +const Ci = Components.interfaces;
> +const Cr = Components.results;
> +
> +const WEBAPPMANIFEST_CONTENT_TYPE = "application/x-web-app-manifest+json";

Nit: for consistency with the constant defined in netwerk/mime/nsMimeTypes.h, separate "WEBAPP" and "MANIFEST" with an underscore in this constant, i.e.:

    const WEBAPP_MANIFEST_CONTENT_TYPE = "application/x-web-app-manifest+json";

@@ +11,5 @@
> +const WEBAPPMANIFEST_CONTENT_TYPE = "application/x-web-app-manifest+json";
> +
> +Components.utils.import("resource://gre/modules/XPCOMUtils.jsm");
> +
> +function WebappsContentHandler() {

Nit: the word "webapp" should be singular rather than plural in the name of this object (and the file that contains it), as such words are rarely pluralized when used in compound nouns in English, even when the compound itself is pluralized (which it isn't in this case); and the singular form is easier to type, remember, and keep consistent with the compound names of other objects (although we have some unfortunate existing inconsistencies in the code); i.e.:

    function WebappContentHandler() {

Comment 15

[Marco Castelluccio [:marco]]

(In reply to Marco Castelluccio [:marco] from comment #13)
> There's a problem with the patch, that I still need to solve.
> If you're on a page (for example "about:home") and insert an URL to a
> manifest that is on a different domain (for example
> "http://marco-c.github.com/ceferinoweb/ceferino.webapp"), the notification
> string contains the wrong URL (it contains "about:home" instead of
> "marco-c.github.com").

I've fixed this issue. But I was wondering what's actually the desired behaviour. What should we show in the confirmation popup? The domain of the manifest or the domain of the referer?
I'd rather show the domain of the manifest.

Comment 16

[Myk Melez [:myk] [@mykmelez]]

(In reply to Marco Castelluccio [:marco] from comment #15)
> I've fixed this issue. But I was wondering what's actually the desired
> behaviour. What should we show in the confirmation popup? The domain of the
> manifest or the domain of the referer?

We should show the domain of the referrer, just like we do when you install an app from the Marketplace, an addon from AMO, or an addon from any other site via a URL to a XPI (f.e. https://people.mozilla.com/~myk/r2d2b2g/).

Comment 17

[Marco Castelluccio [:marco]]

(In reply to Myk Melez [:myk] [@mykmelez] from comment #16)
> We should show the domain of the referrer, just like we do when you install
> an app from the Marketplace, an addon from AMO, or an addon from any other
> site via a URL to a XPI (f.e. https://people.mozilla.com/~myk/r2d2b2g/).

What should we do if the referrer is a local page (file://...) or when an user manually types a manifest URL?

In the first case also the popup we're showing with mozIDOMApplicationRegistry.install is wrong, as it says: "Do you want to install "AppName" from this site ()?".

In case of a manually typed URL, I'd say we should show the manifest domain (but I still have to figure out a way to understand whether an URL was manually typed or not).

Comment 18

[Myk Melez [:myk] [@mykmelez]]

(In reply to Marco Castelluccio [:marco] from comment #17)
> What should we do if the referrer is a local page (file://...) or when an
> user manually types a manifest URL?

We should skip the confirmation prompt, just like we do for addons.

Comment 19

[Myk Melez [:myk] [@mykmelez]]

(In reply to Myk Melez [:myk] [@mykmelez] from comment #18)
> We should skip the confirmation prompt, just like we do for addons.

But: we must still respect installs_allowed_from.

Comment 20

[Myk Melez [:myk] [@mykmelez]]

Per bug 1238079, we're going to disable the desktop web runtime and remove it
from the codebase, so we won't fix these bugs in the integration between Firefox and the runtime.