1173200 - Apps with original URI schemes using web extensions (e.g. html) do not launch

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

It's been suggested that apps that use an original URI scheme (e.g. myapp://...) with a non-web file extension (e.g. .xyz, .yyy) launch successfully but apps with a web extension, (e.g. .jpg, .html, and .gif), do not launch successfully.

Investigate!

Wes, I believe you have some knowledge about how these uri flows work - do you have any thoughts here?

Comment 1

[Wesley Johnston (:wesj)]

What do people mean by "do not launch successfully"? They're probably being filtered out here:

http://mxr.mozilla.org/mozilla-central/source/mobile/android/modules/HelperApps.jsm#51

(i.e. most apps that register for only .html files are browsers, and we filter them out of our list of apps, since otherwise every single page would be "openable in a different app").

Comment 2

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

I can confirm this issue. STR:

1) Install my test application [1] (I load it into Android Studio and press the run button)
2) Navigate to [2] in fennec
3) Click a lol.* above the web extension header (test application opens)
4) Hit back to return to fennec
5) Click a lol.* below the web extension header

Expected: The test application opens
Actual: "Cannot open link" toast is displayed

I tried the following extensions and none of them work:
  * .htm
  * .html
  * .jpg
  * .gif

Wes' comment 1 seems to specifically handle .html so maybe there's something else at work here.

[1]: https://github.com/mcomella/CustomURIScheme
[2]: https://people.mozilla.org/~mcomella/test/custom_uri.html

Comment 3

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(Assuming I set the WebIDE breakpoints properly) it doesn't appear the HelperApps.:
  * getAppsForUri
  * get defaultBrowsers
  * get defaultHtmlHandlers

methods are ever called.

Comment 4

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

IntentHelper.handleMessage is also never called - I'm a little lost. :d

Comment 5

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(In reply to Michael Comella (:mcomella) from comment #4)
> IntentHelper.handleMessage is also never called - I'm a little lost. :d

Even when selecting a link that correctly opens an application registered to a custom URI scheme.

Comment 6

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

In ContentDispatchChooser [1]: 

44     // The current list is based purely on the scheme. Redo the query using the url to get more
45     // specific results.
46     aHandler = this.protoSvc.getProtocolHandlerInfoFromOS(aURI.spec, {});
47 
48     // The first handler in the set is the Android Application Chooser (which will fall back to a default if one is set)
49     // If we have more than one option, let the OS handle showing a list (if needed).
50     if (aHandler.possibleApplicationHandlers.length > 1) {

---
In the non-web extension case, the list contains two items: the Android Application Chooser and my CustomURIScheme application.

In the web extension case, only the former exists, then it goes to IntentHelper.openNoHandler (which fails and throws up the error toast).

Time to dig into `this.protoSvc.getProtocolHandlerInfoFromOS(aURI.spec, {});`

[1]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/components/ContentDispatchChooser.js?rev=7c445f47c4d6#50

Comment 7

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

So we call:

nsOSHelperAppService::getProtocolHandlerInfoFromOS [1]
 nsMIMEInfoAndroid::GetMimeInfoForURL
  AndroidBridge::GetHandlersForURL
   GeckoAppShell::GetHandlersForURLWrapper
    (-> Java) GeckoAppShell.getHandlersForURL

Notably, when we run queryIntentActivities [2], Intent.mType is different for the different links, e.g. "text/html" for `.htm` links and no results will be returned. Thus, the issue here is the mime type - time to dig into where that is set.

[1]: https://mxr.mozilla.org/mozilla-central/source/uriloader/exthandler/android/nsOSHelperAppService.cpp?rev=dd937e69cd40#54
[2]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/GeckoAppShell.java?rev=3527e6013de9#991

Comment 8

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

I think [1] is causing the issue:

 final String mimeType2 = getMimeTypeFromExtension(extension);

[1]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/GeckoAppShell.java?rev=3527e6013de9#1257

Comment 9

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Our current flow is roughly:
  * if Intent.ACTION_SEND, create & return custom Share Intent
  * normalizeUriScheme
  * if mimeType arg is non-empty, create custom Intent w/ MimeType arg
  * Return null Intent if uri scheme is unsafe (i.e. some tel & sms)
  * If "intent" scheme, use Intent.parseUri & return results
  * Create custom intent w/ action, uri, and inferred mimeType
    * If "vnd.youtube" scheme, create & return custom intent based off ^
    * (NOTE) If not "sms" scheme, return the custom intent ^
    * If "sms" scheme, create & return custom intent based off ^ w/ sms_body extra

NOTE: When passing this custom Intent back to Android, if there is an inferred mimeType (e.g. because of a *.html ending), the list of returned applications is much smaller, if non-existent. Notably, Intent.parseUri does not infer this mimeType and correctly opens *.html links.

It seems a lot of this code comes from the original codebase [1] and can be uninformed or outdated - I wonder what here could be replaced with Intent.parseUri.

In the simplest form, I propose returning the results of Intent.parseUri at NOTE, rather than our custom URI.

Wes, since I think you have some context here, do you see any problems with this? I'll post a patch so you have a clearer idea of what I'm talking about.

[1]: http://hg.mozilla.org/mozilla-central/file/02f6f69e8e2f/mobile/android/base/GeckoAppShell.java

Comment 10

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: MozReview Request: Bug 1173200 - (WIP) Open file uris w/ mimetype, all others without. r=wesj

Bug 1173200 - Use Intent.parseUri for opening uris in the default case. r=wesj

Comment 11

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Filed bug 1181698 for further cleanup.

Comment 12

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Margaret pointed out bug 1100100 for `mimeType2 =` in the blame. It looks like this bug is a regression from bug 1100100 (though I have not conclusively tested). My patch in comment 10 regresses bug 1100100 so I'll look for an alternative fix.

Comment 13

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

There's useful info in the Intent Filter matching docs [1]:

An intent that contains neither a URI nor a MIME type passes the test only if the filter does not specify any URIs or MIME types.

  a) An intent that contains a URI but no MIME type (neither explicit nor inferable from the URI) passes the test only if its URI matches the filter's URI format and the filter likewise does not specify a MIME type.
  ...
  d) An intent that contains both a URI and a MIME type (either explicit or inferable from the URI) passes the MIME type part of the test only if that type matches a type listed in the filter. It passes the URI part of the test either if its URI matches a URI in the filter or if it has a content: or file: URI and the filter does not specify a URI. In other words, a component is presumed to support content: and file: data if its filter lists only a MIME type.

---
That leads to the possibility that the original reporters of this bug are not using <data> tags with MIME types (as I neglected to do in my application).

[1]: http://developer.android.com/guide/components/intents-filters.html#DataTest

Comment 14

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(In reply to Michael Comella (:mcomella) from comment #13)
> That leads to the possibility that the original reporters of this bug are
> not using <data> tags with MIME types (as I neglected to do in my
> application).

However, in Chrome's behavior:
  * it does not set explicit mimeTypes on custom URI schemes (I changed my application to only accept mimeType="text/html" and it does not open).
  * it defers to the built-in Download Manager application for their downloads. Both the Download Manager notification and activity set explicit mimeTypes on the "file" scheme (I downloaded the peace.zip file and it correctly opened in Wordex from bug 1100100, which doesn't open if no mimeType is set).

To maintain parity, it seems we should not set mimeTypes on custom URI schemes and set explicit mimeTypes on "file" schemes. For other schemes, since Intent.parseUri does not set explicit mimeTypes, I'd imagine the standard is to not use explicit mimeTypes (but have we seen any other issues? Maybe we should maintain the status quo).

Wes, any thoughts?

Comment 15

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Alternatively, the caller can fix the issue by specifying a filter with the specific mimeTypes to handle and one without:

<intent-filter>
  ...
  <data android:scheme="customScheme" ... />
</intent-filter>
<intent-filter>
  ...
  <data android:scheme="customScheme" ... android:mimeType="text/html" />
  ...
</intent-filter>

Comment 16

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8631140 [details]
MozReview Request: Bug 1173200 - (WIP) Open file uris w/ mimetype, all others without. r=wesj

Bug 1173200 - (WIP) Open file uris w/ mimetype, all others without. r=wesj

Comment 17

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(See comment 12 though I still have not conclusively tested - if pages can't open because of the mimetype, it makes sense to me the code to add the mimetype would be the regression)

Comment 18

[Wesley Johnston (:wesj)]

Comment on attachment 8631140 [details]
MozReview Request: Bug 1173200 - (WIP) Open file uris w/ mimetype, all others without. r=wesj

Despite its claims otherwise, I can't login to reviewboard with my bugzilla credentials, so I can't review anything there. I'm not sure why you moved the intent uri handling code into the sms handling code or how its related to this bug. The mimetype change seems reasonable to me though.

Alternatively, you might run getHandlersForIntent and then check the one with the best results. If they both match different things, you could even explicitly show a chooser with both intents: http://developer.android.com/reference/android/content/Intent.html#EXTRA_INITIAL_INTENTS

We do that in the FilePicker code for instance: http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/FilePicker.java#201

but I'd take your current patch as an easy first fix :)

Comment 19

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(In reply to Wesley Johnston (:wesj) from comment #18)
> I'm not sure why you moved
> the intent uri handling code into the sms handling code or how its related
> to this bug.

The if block I run the Intent URI handling in is an early return. Anything after the if block is the sms handling code:

 if (!"sms".equals(scheme)) {

Comment 20

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Also worth mentioning that I removed the restriction that the intents accessed there must have "intent" schemes - Intent.parseUri seems to be able to handle any type of Uri you throw at it.

Comment 21

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: Open file uris w/ mimetype, all others without

Using my test page (https://people.mozilla.org/~mcomella/test/uri.html), I have
confirmed that the following schemes still work:
  * intent
  * android-app
  * custom schemes registered w/ Android (i.e. "mcomella")
  * tel
  * mailto

This gives me confidence that all links should work.

Comment 22

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: Open file uris w/ mimetype, all others without

Here's a more self-contained patch, Wes.

Comment 23

[Wesley Johnston (:wesj)]

Comment on attachment 8633238 [details] [diff] [review]
Open file uris w/ mimetype, all others without

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

Ahh, that's more what I originally expected :) Thanks

Comment 24

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

url:        https://hg.mozilla.org/integration/fx-team/rev/e59783c93ce6c17fab7f52c4ef0ce1d2180793f0
changeset:  e59783c93ce6c17fab7f52c4ef0ce1d2180793f0
user:       Michael Comella <[e-mail]>
date:       Mon Jul 13 16:30:42 2015 -0700
description:
Bug 1173200 - Open file uris w/ mimetype, all others without. r=wesj

Using my test page (https://people.mozilla.org/~mcomella/test/uri.html), I have
confirmed that the following schemes still work:
  * intent
  * android-app
  * custom schemes registered w/ Android (i.e. "mcomella")
  * tel
  * mailto

Additionally, I downloaded a PDF and successfully opened it via both the
notifications tray and about:downloads.

This gives me confidence that all links should work.

Comment 25

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/e59783c93ce6

Comment 26

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

> Using my test page (https://people.mozilla.org/~mcomella/test/uri.html)

My test page has been moved:
  https://mcomella.github.io/test/uri.html