Closed
Bug 1173200
Opened 10 years ago
Closed 9 years ago
Apps with original URI schemes using web extensions (e.g. html) do not launch
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox42 fixed)
RESOLVED
FIXED
Firefox 42
Tracking | Status | |
---|---|---|
firefox42 | --- | fixed |
People
(Reporter: mcomella, Assigned: mcomella)
References
Details
Attachments
(2 files, 1 obsolete file)
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?
Flags: needinfo?(wjohnston)
Comment 1•9 years ago
|
||
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").
Flags: needinfo?(wjohnston)
Assignee | ||
Comment 2•9 years ago
|
||
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
Assignee | ||
Comment 3•9 years ago
|
||
(Assuming I set the WebIDE breakpoints properly) it doesn't appear the HelperApps.:
* getAppsForUri
* get defaultBrowsers
* get defaultHtmlHandlers
methods are ever called.
Assignee | ||
Comment 4•9 years ago
|
||
IntentHelper.handleMessage is also never called - I'm a little lost. :d
Assignee | ||
Comment 5•9 years ago
|
||
(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.
Assignee | ||
Comment 6•9 years ago
|
||
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
Assignee | ||
Comment 7•9 years ago
|
||
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
Assignee | ||
Comment 8•9 years ago
|
||
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
Assignee | ||
Comment 9•9 years ago
|
||
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
Flags: needinfo?(wjohnston)
Assignee | ||
Comment 10•9 years ago
|
||
Bug 1173200 - Use Intent.parseUri for opening uris in the default case. r=wesj
Attachment #8631140 -
Flags: review?(wjohnston)
Assignee | ||
Comment 11•9 years ago
|
||
Filed bug 1181698 for further cleanup.
Assignee | ||
Comment 12•9 years ago
|
||
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.
Assignee | ||
Comment 13•9 years ago
|
||
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
Assignee | ||
Comment 14•9 years ago
|
||
(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?
Assignee | ||
Comment 15•9 years ago
|
||
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>
Assignee | ||
Updated•9 years ago
|
Attachment #8631140 -
Attachment description: MozReview Request: Bug 1173200 - Use Intent.parseUri for opening uris in the default case. r=wesj → MozReview Request: Bug 1173200 - (WIP) Open file uris w/ mimetype, all others without. r=wesj
Assignee | ||
Comment 16•9 years ago
|
||
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
Assignee | ||
Updated•9 years ago
|
Attachment #8631140 -
Flags: review?(wjohnston) → feedback?(wjohnston)
Assignee | ||
Updated•9 years ago
|
Assignee | ||
Comment 17•9 years ago
|
||
(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)
Blocks: 1100100
Comment 18•9 years ago
|
||
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 :)
Flags: needinfo?(wjohnston)
Attachment #8631140 -
Flags: feedback?(wjohnston) → feedback+
Assignee | ||
Comment 19•9 years ago
|
||
(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)) {
Assignee | ||
Comment 20•9 years ago
|
||
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.
Assignee | ||
Comment 21•9 years ago
|
||
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.
Attachment #8631887 -
Flags: review?(wjohnston)
Assignee | ||
Updated•9 years ago
|
Blocks: android-intents
Updated•9 years ago
|
Attachment #8631887 -
Flags: review?(wjohnston) → review+
Assignee | ||
Comment 22•9 years ago
|
||
Here's a more self-contained patch, Wes.
Attachment #8633238 -
Flags: review?(wjohnston)
Assignee | ||
Updated•9 years ago
|
Attachment #8631887 -
Attachment is obsolete: true
Comment 23•9 years ago
|
||
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
Attachment #8633238 -
Flags: review?(wjohnston) → review+
Assignee | ||
Comment 24•9 years ago
|
||
url: https://hg.mozilla.org/integration/fx-team/rev/e59783c93ce6c17fab7f52c4ef0ce1d2180793f0
changeset: e59783c93ce6c17fab7f52c4ef0ce1d2180793f0
user: Michael Comella <michael.l.comella@gmail.com>
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•9 years ago
|
||
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox42:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 42
Assignee | ||
Comment 26•7 years ago
|
||
> 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
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•