Closed Bug 1100100 Opened 10 years ago Closed 9 years ago

Downloaded file can't be opened from "Downloads" page

Categories

(Firefox for Android Graveyard :: Download Manager, defect)

ARM
Android
defect
Not set
normal

Tracking

(firefox34 wontfix, firefox35 wontfix, firefox36 verified, firefox37 verified, fennec35+)

VERIFIED FIXED
Firefox 37
Tracking Status
firefox34 --- wontfix
firefox35 --- wontfix
firefox36 --- verified
firefox37 --- verified
fennec 35+ ---

People

(Reporter: Sergey.Krivenkov, Assigned: wesj)

References

Details

(Keywords: reproducible)

Attachments

(1 file)

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:33.0) Gecko/20100101 Firefox/33.0
Build ID: 20141106120505

Steps to reproduce:

1. Downloaded a zip file
2. Install one application which can manage zip files




Actual results:

Downloaded file can't be opened from "Downloads" if there is only one application associated with this kind of file in the system.

I can't open downloaded *.zip file because I have only one installed application which open it. The problem is completelly solved when I install some second application for this kind of file (*.zip in my case). It happens because in first case FF launches the file directly, but in second case - thru Resolver activity.

Test 1: failed
11-07 23:49:54.912: I/ActivityManager(452): START u0 {act=android.intent.action.VIEW dat=file:///storage/emulated/0/Download/libffmpeg.neon.1.7.31.zip flg=0x4000000} from pid 2416 
11-07 23:49:54.927: E/GeckoConsole(2416): Error: openDownload() [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIFile.launch]" nsresult: "0x80004005 (NS_ERROR_FAILURE)" location: "JS frame :: chrome://browser/content/aboutDownloads.js :: dl_openDownload/< :: line 467" data: no]

Test 2: passed
11-07 23:51:00.472: D/GeckoHealthRec(2416): Recording session end: P 
11-07 23:51:00.472: I/ActivityManager(452): START u0 {act=android.intent.action.VIEW dat=file:///storage/emulated/0/Download/libffmpeg.neon.1.7.31.zip typ=application/zip flg=0x4000000 cmp=android/com.android.internal.app.ResolverActivity} from pid 2416 
11-07 23:51:00.482: I/ActivityManager(452): Start proc system:ui for activity android/com.android.internal.app.ResolverActivity: pid=10356 uid=1000 gids={41000, 1028, 1015, 3002, 3001, 3003} 
11-07 23:51:00.487: V/GeckoHealthRec(2416): Recorded session entry for env 12, current is 
12 11-07 23:51:00.487: D/GeckoSessInfo(2416): Recording session done: 1415397053051 
11-07 23:51:00.492: D/GeckoBrowserProvider(2416): Expiring history. 
11-07 23:51:00.492: D/GeckoBrowserProvider(2416): Expiring thumbnails. 
11-07 23:51:00.507: I/GeckoHealth(2416): firefox :: HealthReportBroadcastService :: Registering HealthReportPruneService. 
11-07 23:51:00.507: I/GeckoHealth(2416): firefox :: BackgroundService :: Setting inexact repeating alarm for interval 86400000 


Expected results:

Downloaded file can be opened from "Downloads" page
Nexus 10, Android 4.4.4, FF 33.0, locale English
OS: Windows 7 → Android
Hardware: x86_64 → ARM
Paolo any idea what's going on here?
Flags: needinfo?(paolo.mozmail)
Status: UNCONFIRMED → NEW
tracking-fennec: --- → ?
Ever confirmed: true
Flags: in-testsuite?
Version: Firefox 33 → Trunk
E/StrictMode(15185): file:// Uri exposed through Intent.getData()
E/StrictMode(15185): java.lang.Throwable: file:// Uri exposed through Intent.getData()
E/StrictMode(15185): 	at android.os.StrictMode.onFileUriExposed(StrictMode.java:1603)
E/StrictMode(15185): 	at android.net.Uri.checkFileUriExposed(Uri.java:2341)
E/StrictMode(15185): 	at android.content.Intent.prepareToLeaveProcess(Intent.java:7439)
E/StrictMode(15185): 	at android.app.Instrumentation.execStartActivity(Instrumentation.java:1479)
E/StrictMode(15185): 	at android.app.Activity.startActivityForResult(Activity.java:3736)
E/StrictMode(15185): 	at android.app.Activity.startActivityForResult(Activity.java:3697)
E/StrictMode(15185): 	at android.support.v4.app.FragmentActivity.startActivityForResult(FragmentActivity.java:840)
E/StrictMode(15185): 	at org.mozilla.gecko.GeckoActivity.startActivityForResult(GeckoActivity.java:68)
E/StrictMode(15185): 	at android.app.Activity.startActivity(Activity.java:4007)
E/StrictMode(15185): 	at android.app.Activity.startActivity(Activity.java:3975)
E/StrictMode(15185): 	at org.mozilla.gecko.GeckoActivity.startActivity(GeckoActivity.java:62)
E/StrictMode(15185): 	at org.mozilla.gecko.GeckoAppShell.openUriExternal(GeckoAppShell.java:1085)
E/StrictMode(15185): 	at org.mozilla.gecko.mozglue.GeckoLoader.nativeRun(Native Method)
E/StrictMode(15185): 	at org.mozilla.gecko.GeckoAppShell.runGecko(GeckoAppShell.java:373)
E/StrictMode(15185): 	at org.mozilla.gecko.GeckoThread.run(GeckoThread.java:190)
I/ActivityManager(  746): START u0 {act=android.intent.action.VIEW dat=file:///storage/emulated/0/Download/test-rar-j2ee_1.3-2.2.1.rar flg=0x4000000} from uid 10086 on display 0
I/GeckoConsole(15185): Error: openDownload() [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIFile.launch]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: chrome://browser/content/aboutDownloads.js :: dl_openDownload/< :: line 469"  data: no]
Keywords: reproducible
Assignee: nobody → wjohnston
tracking-fennec: ? → 35+
(In reply to Aaron Train [:aaronmt] from comment #2)
> Paolo any idea what's going on here?

Not really - I suspect the places to investigate are the Java code and the native methods it calls to open files, per comment 4.

It's also possible that the MIME type detection isn't working too well. There are in fact two code paths that may result in openUriExternal being called, nsIFile and the external helper app code:

http://mxr.mozilla.org/mozilla-central/source/xpcom/io/nsLocalFileUnix.cpp#2051
mxr.mozilla.org/mozilla-central/source/uriloader/exthandler/android/nsMIMEInfoAndroid.cpp#23
This is a misleading summary.
Flags: needinfo?(paolo.mozmail)
Are we going to be able to fix this for the Beta 35 period?
Flags: needinfo?(wjohnston)
I can't reproduce this. I downloaded a zip from:

http://www.colorado.edu/conflict/peace/download/peace.zip

and launched it with ES FileExplorer. Worked fine. Comment 4 shows a strict mode violation we should file, but is separate from this bug. Can I get better STR?
Flags: needinfo?(wjohnston)
Hi guys,
It's really annoying bug. And it's very sad to see how much time it takes to analyse and fix this easy issue.

It's still reproduced for me:

Pre: Android 5.0.1 (clean, after factory reset), installed FF 34.0, installed Wordex app (https://play.google.com/store/apps/details?id=club.bre.wordex) which can handle the zip.
Steps:
1. Clicked your link.
2. Downloaded the "peace.zip" with the FF
3. Menu -> Tools -> Downloads
4. Click the zip
Result: nothing happened

Logcat output for the click:
12-19 22:58:13.306: I/ActivityManager(401): START u0 {act=android.intent.action.VIEW dat=file:///storage/emulated/0/Download/peace.zip flg=0x4000000} from uid 10087 on display 0
12-19 22:58:13.332: E/GeckoConsole(4485): Error: openDownload() [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIFile.launch]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: chrome://browser/content/aboutDownloads.js :: dl_openDownload/< :: line 467"  data: no]
I can reproduce with the above steps on my Nexus 5.
Hmm.. Works on my Nexus 5 from about:downloads, but fails from the Notification we show. I'll look into that (it looks like we don't have the a mimetype there which is strange...)
Attached patch PatchSplinter Review
This tries to determine the mimetype in java if we can. Fixes the notification clicking for me. There are lots of other bugs and some cleanup needed here. I'll file bugs for them.
Attachment #8539522 - Flags: review?(michael.l.comella)
Comment on attachment 8539522 [details] [diff] [review]
Patch

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

I'm not sure why this fixes the issue - why do notifications appear to follow a different intent code path than about:downloads?

Plan: Since this affects 35, it's the holidays, and wesj is out today, I'll test this locally, ensure it works, land it, and we can look for a better solution in a followup.

::: mobile/android/base/GeckoAppShell.java
@@ +1189,2 @@
>          final Intent intent = getIntentForActionString(action);
> +        intent.setDataAndType(uri, mimeType2);

The Android system should infer this, right? Why is setting mimetype explicitly necessary?
(In reply to Michael Comella (:mcomella) from comment #13)
> I'm not sure why this fixes the issue - why do notifications appear to
> follow a different intent code path than about:downloads?

There are some comments here about why we don't use download.launch in both places:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/modules/DownloadNotifications.jsm#139

Fixing that would be nice though. The DownloadIntegration.launchDownload method does a lot of this for us. It would be nice to have some consistency.
http://mxr.mozilla.org/mozilla-central/source/toolkit/components/jsdownloads/src/DownloadIntegration.jsm#724
 
> The Android system should infer this, right? Why is setting mimetype
> explicitly necessary?

Android doesn't infer this for you, and won't match some apps otherwise (depending on the manifest):
http://androidxref.com/5.0.0_r2/xref/frameworks/base/core/java/android/content/Intent.java#5364
Comment on attachment 8539522 [details] [diff] [review]
Patch

This is tracking beta, so asking for approval there. I think we've had this long enough that I wouldn't mind letting it ride trains though.

Approval Request Comment
[Feature/regressing bug #]: I think this is very old. Although I pointed at some downloads.jsm code above, the existed before that landed.
[User impact if declined]:  Can't open some downloaded files
[Describe test coverage new/current, TBPL]: none. we need some :(
[Risks and why]: pretty low risk. We infer a mimetype now if we can. Should only improve things. We could try to fix this in the Downloads code (i.e. file.launch should return better information if it fails)
[String/UUID change made/needed]: none.
Attachment #8539522 - Flags: approval-mozilla-beta?
Attachment #8539522 - Flags: approval-mozilla-aurora?
Comment on attachment 8539522 [details] [diff] [review]
Patch

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

(In reply to Wesley Johnston (:wesj) from comment #14)
> There are some comments here about why we don't use download.launch in both
> places:
> http://mxr.mozilla.org/mozilla-central/source/mobile/android/modules/
> DownloadNotifications.jsm#139
> 
> Fixing that would be nice though. The DownloadIntegration.launchDownload
> method does a lot of this for us. It would be nice to have some consistency.
> http://mxr.mozilla.org/mozilla-central/source/toolkit/components/jsdownloads/
> src/DownloadIntegration.jsm#724

File please (mentorable)?

> > The Android system should infer this, right? Why is setting mimetype
> > explicitly necessary?
> 
> Android doesn't infer this for you, and won't match some apps otherwise
> (depending on the manifest):
> http://androidxref.com/5.0.0_r2/xref/frameworks/base/core/java/android/
> content/Intent.java#5364

I'm not sure that's true:

http://androidxref.com/5.0.0_r2/xref/frameworks/base/core/java/android/content/Intent.java#4529

The resolution code is in native code, however. But if the MimeTypeMap implementation works and prevents this from crashing, then WFM.

I'm concerned about how using a new resolver will alter how we handle certain filetypes so I'm against uplifting (at least to 35).

For the record, here is the list of resolutions in MimeTypeMap (see line 36): https://android.googlesource.com/platform/libcore/+/jb-mr2-release/luni/src/main/java/libcore/net/MimeUtils.java
Attachment #8539522 - Flags: review?(michael.l.comella) → review+
(In reply to Michael Comella (:mcomella) from comment #17)
> Comment on attachment 8539522 [details] [diff] [review]
> Patch
> 
> Review of attachment 8539522 [details] [diff] [review]:
> File please (mentorable)?

Looking into this a bit more, I think maybe this was fixed between then and the new downloads code. I filed bug 1116280 to try and flip it.

> The resolution code is in native code, however. But if the MimeTypeMap
> implementation works and prevents this from crashing, then WFM.

To me this looks like it tries to resolve the mimetype only if its a content uri from a contentResolver. Did i miss something here? Actually, glancing at our code, it looks like we try to resolve the URI a lot as well. i.e. nsIFile.launch does:

http://mxr.mozilla.org/mozilla-central/source/xpcom/io/nsLocalFileUnix.cpp#2051

as well as downloads.launch. Our internal "database" is tiny:
http://mxr.mozilla.org/mozilla-central/source/uriloader/exthandler/nsExternalHelperAppService.cpp#437

but we should fall back to asking Android, which should push us through this bit of ugly code eventually:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/GeckoAppShell.java#999

which basically does what we're doing anyway (just in a nutso fashion). I have no idea just reading it quickly why its failing... Maybe it isn't any more...

I still think this is a good thing for us to do. In every attempt I've made to use intent's without passing a mimetype, Android has failed to find apps that are available. We should always make a guess if we can.
(In reply to Wesley Johnston (:wesj) from comment #18)
> To me this looks like it tries to resolve the mimetype only if its a content
> uri from a contentResolver.

Ah, yes - I'm skimmed over that [1]. -_-

Then yes, sounds like a good idea to me and I approve of uplifting (though I'd still a little cautious about uplifting to Beta).

[1]: http://androidxref.com/5.0.0_r2/xref/frameworks/base/core/java/android/content/Intent.java#4534
https://hg.mozilla.org/mozilla-central/rev/de7d50fd87f8
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 37
Attachment #8539522 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Verified as fixed following the steps from comment 9, on latest Nightly(2014-01-05) on Nexus 5(Android 5.0.1).
Comment on attachment 8539522 [details] [diff] [review]
Patch

This is tracking fennec 35 but not overall tracked and since it's coming up so late (final mobile beta) combined with having already shipped with this issue in the past, I'd like to see us get a full beta cycle of user testing on this in 36 rather than cram it in so close to release.
Attachment #8539522 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
Verified as fixed following the steps from comment 9, on Firefox 36 Beta 2 on Nexus 7 (Android 5.0.1).
Status: RESOLVED → VERIFIED
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.