Closed Bug 941099 Opened 6 years ago Closed 6 years ago

Clicking on downloaded zip file does not open file for auto-unzip

Categories

(Toolkit :: Downloads API, defect)

26 Branch
x86
macOS
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla28
Tracking Status
firefox25 --- unaffected
firefox26 + verified
firefox27 + verified
firefox28 + verified
b2g-v1.2 --- fixed

People

(Reporter: Felipe, Assigned: Felipe)

References

()

Details

Attachments

(2 files, 1 obsolete file)

On OS X, when you launch a downloaded zip file, the default handler from the OS will extract the file.

This doesn't happen with the new Downloads API anymore.

The difference seems to be that in the new API, the default launcherPath for application/zip is detected as /Applications/Preview.app. On the previous API, the "preferredApplication" field in the DB is blank.

Interestingly, on the Save Dialog window, Archive Utility is shown as the default option in the "Open" option. But no matter if Open or Save is chosen, launcherPath gets Preview.app


Easy test url: https://people.mozilla.org/~fgomes/hello.zip
Blocks: 906139
Blocks: 907082
No longer blocks: 906139
Preview is the application that the mimeService tells us is the preferred (and only one) available for zip files:

let mimeService = Cc["@mozilla.org/mime;1"].getService(Ci.nsIMIMEService);
let handlerInfo = mimeService.getFromTypeAndExtension('application/zip', 'zip');

> handlerInfo.preferredApplicationHandler.name
Preview

> handlerInfo.possibleApplicationHandlers.length
1

So where does the Save Dialog get a different list from?
There's no other source of info, the difference is this:  for an application/zip mimeType, the "default application handler" is different than the "preferred application handler". 

See:
> handlerInfo.defaultDescription    (comes from mDefaultApplication)
Archive Utility

> handlerInfo.preferredApplicationHandler.name  (comes from mPreferredApplication)
Preview

(mDefaultApplication is not exposed to JS, only through the defaultDescription attribute)

So is OSX providing the wrong preferredApplication for zip?


Previously when the default action was chosen, we did not store the launcherPath of the path, but now we do, by getting it through preferredApplication. IIRC this was a deliberate change in order to avoid some behavior mismatches between just downloaded files and things in the library.

Paolo, any ideas on what to do here?
Flags: needinfo?(paolo.mozmail)
I just tested it with a new profile + Nightly, and the problem didn't happen, so it sounds like my profile is partially at fault here for having Preview in the list of apps that can handle zip files.

Maybe this also happened in previous versions if this is the case, let me recheck.
Yeah, somehow I got Preview.app in my list of app handlers for zip, but that's not default and with it the same thing happens in older versions.

So i'm gonna call this one PEBKAC ;)
Status: NEW → RESOLVED
Closed: 6 years ago
Flags: needinfo?(paolo.mozmail)
Resolution: --- → INVALID
Felipe, do you think this change in behavior could be related to bug 934155? It would be great if you could take a look to that bug based on the discoveries made here.

I'm not sure if the change we made in the new API will cause frequent breakage or if those are a few rare cases, and how the user could fix any issue that might happen.
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
Attached patch Patch (obsolete) — Splinter Review
So while there was the user fault, there is indeed a change the change in API basically described in comment 2.

Intentionally or not, the preferredApplicationHandler is the application that the user chose to associate with this mimeType, *besides* the default application handler. So if the user leaves the choice as "Use system default", this field will not be set to the default app. The only useful information in our mimeInfo object, in this case, will be preferredAction = useSystemDefault.

So I think keeping the previous behavior of only storing the launcherPath if a custom app was chosen is the only way to make it work in the moment. Or, at least, the simplest one (we could redefine what preferredApplication means, or introduce a new field, but that has higher risks).

Paolo, what do you think?

The change in the test is because the only way to have launcherPath != null is with launchWhenSucceeded = true  (because otherwise the selected option would have been "Save to disk" which means an app choice was not made).
Attachment #8339616 - Flags: review?(paolo.mozmail)
Assignee: nobody → felipc
Attached patch PatchSplinter Review
(Sorry, missed qref for tests)
Attachment #8339616 - Attachment is obsolete: true
Attachment #8339616 - Flags: review?(paolo.mozmail)
Attachment #8339619 - Flags: review?(paolo.mozmail)
Comment on attachment 8339619 [details] [diff] [review]
Patch

This is definitely the right thing to do here, thanks a lot Felipe for figuring this out.

Since this regression affects the application used to open downloaded files when the system default is chosen, in all the cases where there is another non-default choice available, I believe this may have high user impact, because the non-default is chosen instead. Too bad we detected this so late in the cycle.

Can you apply this patch on Beta and start a tryserver build from the Beta tree, that the affected users and QA can use to verify whether the issue is fixed and this does not introduce other regressions? I expect this change to fix the issue without further impact, but given the nature of the change we might want thorough verification and manual testing on different systems.
Attachment #8339619 - Flags: review?(paolo.mozmail) → review+
Blocks: 934155
Comment on attachment 8339619 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 852482 (but only enabled on Fx26 by bug 907082) 
User impact if declined: Some users might be unable to open some types of downloaded files by directly clicking them on the panel (or being auto-opened if the "Open File" option was used)
Testing completed (on m-c, etc.): Landed on inbound, downloads API is covered well by tests. Tested 3 similar bug reports and they were all fixed by this patch (this bug, bug 934155 and bug 935974)
Risk to taking this patch (and alternatives if risky): minimal. If the patch broke something it would be missing setting a custom app to launch a file, which is less bad than not being able to auto-launch it with the default app 
String or IDL/UUID changes made by this patch: none
Attachment #8339619 - Flags: approval-mozilla-beta?
Attachment #8339619 - Flags: approval-mozilla-aurora?
Blocks: 935974
(In reply to :Felipe Gomes from comment #11)
> Tested 3 similar bug reports and they were all fixed
> by this patch (this bug, bug 934155 and bug 935974)

Given that this is actually a regression, affects default behavior for simple downloads, and we have a well-defined patch that fixed three independently reported bugs, if we have enough QA resources my opinion is that we should try to get this fix in Firefox 26. Setting needinfo on Lukas and Alex to get their evaluation as well.

I admit I should have given higher priority to this investigation, despite this did not seem to affect default behavior on first glance. Many thanks to Felipe for figuring out the exact dynamics, and that this did actually affect default behavior.

I've requested manual QA on the test case in bug 934155.
Flags: needinfo?(lsblakk)
Flags: needinfo?(akeybl)
https://hg.mozilla.org/mozilla-central/rev/f3226e0a76df
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Will approve this for Aurora so it might land prior to Monday's go to build on Beta - verification and bake time on both Nightly/Aurora will help with the decision on Monday but I'd lean towards taking it as it's low risk and not taking it would cause a lot of user confusion.
Flags: needinfo?(akeybl)
Attachment #8339619 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 8339619 [details] [diff] [review]
Patch

No reports of breakage or unexpected landing issues so approving for final beta.
Attachment #8339619 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
I've been testing this using the latest older/newest Aurora builds as well as the try-server beta builds from comment #9 with the sample test case as well as others such as PDF files/handlers.

My default handler for zip files on Mac is the Archive Utility and whenever I went to the test URL I get prompted to:
- open it with the (Archive Utility | or other from the drop-down)
- save it
- do this automatically ... from now on

Ultimately the zip file is handled as expected, however I haven't been able to specify "GUI Tar.app" as the default handler unless I specify that in conjunction with "do this automatically ... from now on." Clicking on a file in the download manager will always default to opening using Archive Utility.

Is that latter part expected?
Flags: needinfo?(felipc)
Juan, by "Download Manager" you mean the downloads list in the Library window, right? Not the download panel that drops down from the toolbar button?

If so, yes this is expected, in the sense that it's a bug that has always existed (you can check that the same thing happens in Fx25) and the downloads list in the Library window was not part of the new Downloads API work.
Only the panel code was changed and it gets the correct result now: clicking on the file in the dropdown panel should open it with the correct app.
Flags: needinfo?(felipc)
I primarily used the download panel, but the same behavior happens in both the library window and the the download panel.

As long as the application defaults to a handler that can deal with the file in question, we should be ok.
Keywords: verifyme
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:26.0) Gecko/20100101 Firefox/26.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:26.0) Gecko/20100101 Firefox/26.0

Couldn't reproduce on Nightly from 2013-11-20 and 2013-11-21 on Mac OS X 10.6.8 nor 10.8.5: all the zip files (including the test URL) are properly handled with the Archive Utility (the default handler). The same result is on Firefox 26 beta 10 (Build ID: 20131202182626). 

Exploratory testing was performed around Open downloaded files test and it's a PASS on all platforms - results can be seen here: https://moztrap.mozilla.org/results/cases/?filter-run=2832

Is there any workaround in order to reproduce this issue on older builds?
Flags: needinfo?(felipc)
Attached file mimeTypes.rdf
You should be able to reproduce the bug in older builds by replacing the mimeTypes.rdf file inside the profile with this one.
Flags: needinfo?(felipc)
(In reply to :Felipe Gomes from comment #23)
> You should be able to reproduce the bug in older builds by replacing the
> mimeTypes.rdf file inside the profile with this one.

Thanks for the attachment, I was able to reproduce this issue on Nightly from 2013-11-21.
Verified as fixed on Firefox 26 beta 10 (Build ID: 20131202182626).
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:27.0) Gecko/20100101 Firefox/27.0

Verified as fixed on Firefox 27 beta 1 (Build ID: 20131209204824).
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:28.0) Gecko/20100101

Verified fixed on latest Aurora 28.0a2 (Build ID: 20131212004008).
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.