Closed Bug 347230 Opened 18 years ago Closed 18 years ago

Minimal "Save"-only dialog shown for files with discoverable types (known file extensions)

Categories

(Firefox :: File Handling, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED

People

(Reporter: deanis74, Assigned: emk)

References

(Blocks 1 open bug)

Details

(Keywords: regression, verified1.8.1.2)

Attachments

(5 files, 1 obsolete file)

Bug 315536 comment 46:
> If I'm seeing this dialog for the majority of my downloads, regardless of
> what's configured in my options.  Does this mean something's configured
> incorrectly on my company's proxy server?

Oh wow, this is annoying.  I can't even open a WAV file now.  I'd much prefer the old dialog, with the option disabled than having to save everything before opening it.
Flags: blocking-firefox2?
OS: Windows XP → All
Hardware: PC → All
Version: unspecified → 2.0 Branch
The old dialog (see attachment 202230 [details]) has no options other than "Save" enabled, so I don't think I understand what advantage it would give you in the scenario you describe.
Status: NEW → RESOLVED
Closed: 18 years ago
Flags: blocking-firefox2?
Resolution: --- → INVALID
Hrm.  Maybe it's that the MozOpenDownload extension doesn't work anymore.  I'll have to dig some more.
k; feel free to re-open this if I'm totally off base! :)
Attached image save prompt
Mike, this is what I'm talking about.  Although this could be a separate issue that I'm just noticing more now that the dialog has changed.
(In reply to comment #2)
> Hrm.  Maybe it's that the MozOpenDownload extension doesn't work anymore.  I'll
> have to dig some more.
> 

I'm not sure what MozOpenDownload does, but it's likely that any extension that hooks-in to the old download dialog will need to update to work with Fx2, by hooking in to the new one as well. I've noted on bug 347232 that it'd be good to create a single point for extensions to hook in to the download dialog, a "protected space".
(In reply to comment #1 [Mike Beltzner])
> The old dialog (see attachment 202230 [details] [edit]) has no options other than
> "Save" enabled, so I don't think I understand what advantage it would give
> you in the scenario you describe.

Only true if the file extension is not recognized.  Try this URL in 1.5:
  http://www.hixie.ch/tests/adhoc/http/content-type/010.png
(From bug 229688)

Bug 346332 is about this bug's same problem for Thunderbird, where the 
issue is much more pressing because it's more common for attachments to be 
application/octet-stream (thanks to Outlook Express) than stuff coming off 
the web.
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
Blocks: 346332
Depends on: 315536
As can be tested e.g. by opening 
https://bugzilla.mozilla.org/attachment.cgi?id=232436&action=view

After the fix, I don't get offered to open the doc straight away, as I did before.
i can confirm this.
and it happens also with other extensions, as compressed files (zip, rar, ecc), often i download i rarred archive with open, to extract only a part of its contents, and discard the ramaining part. Now i have to save, extract the part and then delete the file... 
it's annoying and confusing novice users...

should be fixed for final release, is a regression from 1.5 behaviour, imho
(In reply to comment #8)
> Now i have to save, extract the part and then delete the file... 
> it's annoying and confusing novice users...
> 
> should be fixed for final release, is a regression from 1.5 behaviour, imho

Some users (myself included) had a lengthy and heated discussion about the change over on MozillaZine forums in topic:  http://forums.mozillazine.org/viewtopic.php?t=458336

Some users scolded others for having referred to it as a 'regression'..(*I* see your "IMHO", and I agree).

Before the thread got locked, I feel we had covered quite a lot of ground - including the fact that the change affected far more than just .exe files, including the fact that the wording in the dialog is now contradictory (Bug 352137) and also including a look at the way other browsers handle file opening, and what, if any, methods they take to warn and protect users about the risks involved with opening files from the Web. There is certainly stuff there to be learnt, mimicked and even improved upon. Though of course it was falling on deaf/non-present ears.

I don't know if this idiot 'blackwizard', who showed up there late and then proceeding to take a massive rambling spazz, is a Mozilla developer or not - but he says this is a *trivial* change that only a few users will notice, and that opening rar, mp3, wav and other files via the browser is a security risk: http://forums.mozillazine.org/viewtopic.php?t=458336&postdays=0&postorder=asc&postsperpage=15&highlight=zaw&start=90

I pointed out that a virus or trojan-laden file is still malicious whether a user opened it from an 'Open with' dialog in Firefox or if it was Saved and then opened from the Desktop (or other FF-mandated 'download to' location) - if there is no on-access AV installed on a user's system.

He also extolled the virtues of the change being due to what I suppose you could call a 'buffer' - ie: the time between when a user clicks "Save" (rather than the now non-existent 'Open with') and the time they click 'Open' in the Download Manager once the transfer is complete. Apparently, this 'buffer' gives them enough time to 2nd-guess the file's contents/honour and therefore allows them to decide "Woah, better not open that. Whoops!"

In the words of John McEnroe: "You can't be serious!"!
Firefox should be moving forward with innovation instead of retardation.

I wonder - would it help things if there was a 'Scan when download is complete using..' item added to the Options, with a Browse button so users could locate their on-demand virus scanner executable file (similar to the implementation in the Download Statusbar extension)?

This way, Firefox users can open their RAR files, their WAV files, their MP3 files, their BMP files, their PDF files, etc from their browser, and be rest-assured that the file will be scanned for nasties before it is opened.

(Dare I say it?).. You could then even Open .exe's!
We could select helper Apps on Fx 1.5 even if default App is not present. Now we can't select them.
If I understand correctly, this is a regression because bug 315536 didn't intend to change the behavior.
Assignee: dietrich → VYV03354
Status: REOPENED → ASSIGNED
Attachment #243964 - Flags: review?(mconnor)
I want to land this on branch because this is a regression.
Flags: blocking1.8.1.1?
Keywords: regression
(In reply to comment #6)
> Only true if the file extension is not recognized.  Try this URL in 1.5:
>   http://www.hixie.ch/tests/adhoc/http/content-type/010.png
> (From bug 229688)

(In reply to comment #7)
> As can be tested e.g. by opening 
> https://bugzilla.mozilla.org/attachment.cgi?id=232436&action=view

Using 2.0 final, I get the normal download dialog, with all options enabled, for both of these examples. Which platform are you on, and are you still getting the minimal dialog for these links?

(In reply to comment #9)
> Some users (myself included) had a lengthy and heated discussion about the
> change over on MozillaZine forums in topic: 
> http://forums.mozillazine.org/viewtopic.php?t=458336
> 

Using the link referenced in that thread (1), I'm able to trigger the minimal dialog. However, the same link in 1.5.0.8 does not offer any other options other than "save file", which makes both dialogs functionally identical.

1. http://xsms.nm.ru/custombuttons/files/custombuttons.xpi
Comment on attachment 243964 [details] [diff] [review]
Setting up simple ui only if "Open With" is really disabled

Pending review, this patch should be considered for branch, as it uses the proper method for determining whether a helper app is executable. It takes platform into account, and may be the cause of the variability in the reports of this bug.

>Index: toolkit/mozapps/downloads/src/nsHelperAppDlg.js.in
>===================================================================
>RCS file: /cvsroot/mozilla/toolkit/mozapps/downloads/src/nsHelperAppDlg.js.in,v
>retrieving revision 1.41
>diff -u -8 -p -r1.41 nsHelperAppDlg.js.in
>--- toolkit/mozapps/downloads/src/nsHelperAppDlg.js.in	11 Oct 2006 13:02:27 -0000	1.41
>+++ toolkit/mozapps/downloads/src/nsHelperAppDlg.js.in	29 Oct 2006 07:30:48 -0000
>@@ -417,23 +417,22 @@ nsUnknownContentTypeDialog.prototype = {
>       // Put content type, filename and location into intro.
>       this.initIntro(url, fname, displayName);
> 
>       var iconString = "moz-icon://" + fname + "?size=16&contentType=" + this.mLauncher.MIMEInfo.MIMEType;
>       this.dialogElement("contentTypeImage").setAttribute("src", iconString);
> 
>       // if always-save and is-executable and no-handler
>       // then set up simple ui
>-      var defaultApp = this.mLauncher.MIMEInfo.preferredApplicationHandler;
>-      var noDefaultApp = (!defaultApp || !defaultApp.path);
>+      var openWithDefaultOK = this.openWithDefaultOK();
>       var mimeType = this.mLauncher.MIMEInfo.MIMEType;
>       var shouldntRememberChoice = (mimeType == "application/octet-stream" || 
>                                     mimeType == "application/x-msdownload" ||
>                                     this.mLauncher.targetFile.isExecutable());

As openWithDefaultOK() does the isExecutable() check, doing it again here is redundant.

>-      if (shouldntRememberChoice && noDefaultApp) {
>+      if (shouldntRememberChoice && !openWithDefaultOK) {
>         // hide featured choice 
>         this.mDialog.document.getElementById("normalBox").collapsed = "true";
>         // show basic choice 
>         this.mDialog.document.getElementById("basicBox").collapsed = "false";
>         // change button labels
>         this.mDialog.document.documentElement.getButton("accept").label = this.dialogElement("strings").getString("unknownAccept.label");
>         this.mDialog.document.documentElement.getButton("cancel").label = this.dialogElement("strings").getString("unknownCancel.label");
>         // hide other handler
Comment on attachment 243964 [details] [diff] [review]
Setting up simple ui only if "Open With" is really disabled

(In reply to comment #13)
> As openWithDefaultOK() does the isExecutable() check, doing it again here is
> redundant.
This is not redundant. If you removed it and mimeType is neither application/octet-stream nor application/x-msdownload, simple UI will not be set up even if targetFile is executable. Please notice the difference between "&&" and "||".
Also, shouldntRememberChoice is reused after this.
>        if (shouldntRememberChoice) {
>          rememberChoice.checked = false;
>          rememberChoice.disabled = true;
>        }
Moreover, openWithDefaultOK() does isExecutable() check only on Windows.
> #ifdef XP_WIN
(snip)
>         return !tmpFile.isExecutable();
> #else
(snip)
>         return this.mLauncher.MIMEInfo.hasDefaultHandler;
> #endif
I want to keep exactly the same condition as what there is in initAppAndSaveToDiskValues.
>      if (this.mLauncher.targetFile.isExecutable() || (
>          (mimeType == "application/octet-stream" ||
>           mimeType == "application/x-msdownload") && 
>           !openWithDefaultOK)) {
We should be extremely careful here, especially if we are considering land this on branch. Otherwise we are confused by behavior change again.
(In reply to comment #12)
> (In reply to comment #6)
> > Only true if the file extension is not recognized.  Try this URL in 1.5:
> >   http://www.hixie.ch/tests/adhoc/http/content-type/010.png
> 
> Using 2.0 final, I get the normal download dialog, with all options enabled,
> for both of these examples. Which platform are you on, and are you still
> getting the minimal dialog for these links?

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8.1) Gecko/20061026 BonEcho/2.0
Windows 2000
When I first re-checked this today, I checked Options|Content|File Types and saw .PNG listed as the extension for image/png and image/x-png, both handled by the Quicktime plugin.  I hadn't expected that, so I reconfigured the plugin and restarted Fx -- no longer had PNG associations, and still got the minimal Save/Cancel dialog.

Just to be sure: you don't have an entry in file types associated with
application/octet-stream, do you?  (Not normally possible, but you could have an old setting or have somehow hacked mimeTypes.rdf.)
I'm seeing the same behavior in Thunderbird 2.0 nightly builds. Should this be moved to "core" instead? In Thunderbird, the file type associations are all blank, but were not previously. I was leaving it alone figuring that it was a bug after which the associations would be restored to their former selves. 
(In reply to comment #12)
> >   http://www.hixie.ch/tests/adhoc/http/content-type/010.png
> > https://bugzilla.mozilla.org/attachment.cgi?id=232436&action=view
> 
> Using 2.0 final, I get the normal download dialog, with all options enabled,
> for both of these examples. Which platform are you on, and are you still
> getting the minimal dialog for these links?

Try a new Profile.
On my regular profile, in 2.0 I get Save/Cancel for the former and the open with [app] dialog; on a new profile I get the Save/Cancel for both. Neither 'png' or 'doc' is present in my Options->Content->File Types.
WinXP-Pro here. 

> Using the link referenced in that thread (1), I'm able to trigger the minimal
> dialog. However, the same link in 1.5.0.8 does not offer any other options
> other than "save file", which makes both dialogs functionally identical.
> 1. http://xsms.nm.ru/custombuttons/files/custombuttons.xpi

Don't have 1.5.0.8, but just tested the file in 1.5.0.7 (new Profile) - see attached jpg screencap of open with option - it suggests Netscape, which I also have installed. (And 'xpi' is not present in the FF Options > File Types). Possibly of interest: my Windows Folder Options>File Types shows that I've registered 'xpi' to 'WinRAR archiver', so it obviously wasn't pulling the helper app info from the OS. ;-)

Thanks Masatoshi and everyone else for working on this issue! There have been quite a few unhappy posts on message boards about this since 2.0 was released.
(In reply to comment #12)
> > As can be tested e.g. by opening 
> > https://bugzilla.mozilla.org/attachment.cgi?id=232436&action=view
> 
> Using 2.0 final, I get the normal download dialog, with all options enabled,
> for both of these examples. Which platform are you on, and are you still
> getting the minimal dialog for these links?

Still getting the minimal dialog, using Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1) Gecko/20061102 BonEcho/2.0 ID:2006110204.
Comment on attachment 243964 [details] [diff] [review]
Setting up simple ui only if "Open With" is really disabled

Mano please look, and see Dietrich's comments
Attachment #243964 - Flags: review?(mconnor) → review?(mano)
Not a blocker at present, but we do want to track this down for the branch.
Flags: wanted1.8.1.x+
Flags: blocking1.8.1.1?
Flags: blocking1.8.1.1-
*** Bug 359948 has been marked as a duplicate of this bug. ***
(In reply to comment #21)
> *** Bug 359948 has been marked as a duplicate of this bug. ***

As Phil said in the duplicate comment from bug 359948:
I hope this gets fixed pretty soon, before I have to track down the
near-unfindable bug 347230 again.

What about a more expressive summary??!!

(In reply to comment #22)
> What about a more expressive summary??!!

How's this?
Summary: fix for bug 315536 makes working with files painful → Minimal "Save"-only dialog shown for files with discoverable types (known file extensions)
Comment on attachment 243964 [details] [diff] [review]
Setting up simple ui only if "Open With" is really disabled

>Index: toolkit/mozapps/downloads/src/nsHelperAppDlg.js.in
>===================================================================
>@@ -417,23 +417,22 @@ nsUnknownContentTypeDialog.prototype = {
>       // Put content type, filename and location into intro.
>       this.initIntro(url, fname, displayName);
> 
>       var iconString = "moz-icon://" + fname + "?size=16&contentType=" + this.mLauncher.MIMEInfo.MIMEType;
>       this.dialogElement("contentTypeImage").setAttribute("src", iconString);
> 
>       // if always-save and is-executable and no-handler
>       // then set up simple ui
>-      var defaultApp = this.mLauncher.MIMEInfo.preferredApplicationHandler;
>-      var noDefaultApp = (!defaultApp || !defaultApp.path);
>+      var openWithDefaultOK = this.openWithDefaultOK();
>       var mimeType = this.mLauncher.MIMEInfo.MIMEType;
>       var shouldntRememberChoice = (mimeType == "application/octet-stream" || 
>                                     mimeType == "application/x-msdownload" ||
>                                     this.mLauncher.targetFile.isExecutable());
>-      if (shouldntRememberChoice && noDefaultApp) {
>+      if (shouldntRememberChoice && !openWithDefaultOK) {

Make that if (shouldntRememberChoice && !this.openWithDefaultOK()). That would make it so we don't go through openWithDefaultOK if the less-expensive check fails.

Having said that, I'm not very familiar with this code and gavin or mconnor should sanity-check this too.
Attachment #243964 - Flags: review?(mano) → review-
I'm not sure if this be filed as a separate Bug, but it still doesn't work properly when files with incorrectly-assigned MimeTypes are encountered:
- the 'always do this' option is broken as it refuses to remember the previous action.

Same test-case file as before:
http://xsms.nm.ru/custombuttons/files/custombuttons.xpi
- I wanted it to always open using WinRAR, instead it always opens up the dialog box asking what to do with the file. 

The 'rememberChoice' section of code is inside the IF statement being changed here. I'm not a proficient coder, but it seems to me a few things are out of place or inefficient and that the entire file needs a going-over.
Asking gavin for review per comment #24.
Attachment #243964 - Attachment is obsolete: true
Attachment #245756 - Flags: review?(gavin.sharp)
(In reply to comment #25)
I'm also frustrated by your problem. However, it occurs even on Fx 1.5. Therefore it isn't a regression. Plaease file separate bug.
(In reply to comment #25)
> I'm not sure if this be filed as a separate Bug, but it still doesn't work
> properly when files with incorrectly-assigned MimeTypes are encountered:
> - the 'always do this' option is broken as it refuses to remember the previous
> action.
The perfect example is here:

http://ftp.mozilla.org/pub/mozilla.org/seamonkey/nightly/latest-trunk/
seamonkey-1.5a.en-US.linux-i686.tar.bz2 16-Nov-2006 02:26   13M  
seamonkey-1.5a.en-US.win32.installer.exe 15-Nov-2006 10:33   15M
comment 25 is what happens when the server sends text/plain for binary, and we detect that its binary data and munge the mimetype to application/octet-stream

since we can't actually determine what type of data we have until we download the file, we can't give a sane option, since many different types of files might get munged to application/octet-stream... (you don't want your mp3s feeding into WinRAR, f.e.)
*** Bug 362100 has been marked as a duplicate of this bug. ***
Comment on attachment 245756 [details] [diff] [review]
resolved review comment

Sorry for the delay, this looks fine, and fixes the cases given above.
Attachment #245756 - Flags: review?(gavin.sharp) → review+
Thank you, gavin. Could you check in the patch?
I can check it in once the tree reopens, sure.
Whiteboard: [checkin needed]
Tree is now open. Please check in.
mozilla/toolkit/mozapps/downloads/src/nsHelperAppDlg.js.in 	1.43 
Status: ASSIGNED → RESOLVED
Closed: 18 years ago18 years ago
Resolution: --- → FIXED
Whiteboard: [checkin needed]
Comment on attachment 245756 [details] [diff] [review]
resolved review comment

This is needed to fix thunderbird bug 346332.
Attachment #245756 - Flags: approval1.8.1.1?
Version: 2.0 Branch → Trunk
Triage team: this patch fixes a prominent regression in thunderbird were we only offer to save mail attachments instead of offering to open them with the appropriate default application. Please consider this for 1.8.1.2 (I think we are done accepting 1.8.1.1 fixes)
Flags: blocking1.8.1.2?
Attachment #245756 - Flags: approval1.8.1.1? → approval1.8.1.2?
Flags: blocking1.8.1.2? → blocking1.8.1.2+
Comment on attachment 245756 [details] [diff] [review]
resolved review comment

Approved for 1.8 branch, a=jay for drivers.
Attachment #245756 - Flags: approval1.8.1.2? → approval1.8.1.2+
Whiteboard: [checkin needed (1.8 branch)]
Attached patch branch patchSplinter Review
Sorry, I didn't notice attachment 245766 [details] couldn't apply to the branch.
Dietrich:
Why the trunk patch quotes true/false while the branch patch doesn't?
>          this.mDialog.document.getElementById("normalBox").collapsed = "true";
>          this.mDialog.document.getElementById("normalBox").collapsed = true;
checked-in to 1.8 branch.
Keywords: fixed1.8.1.2
Whiteboard: [checkin needed (1.8 branch)]
v.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20061215 Minefield/3.0a1

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.2pre) Gecko/20070102 BonEcho/2.0.0.2pre
Status: RESOLVED → VERIFIED
Um... So isn't the real issue here that the code used to be looking at the _preferred_ application handler, not the _default_ one?

That's the change that really fixed this bug -- looking at the default app instead of the preferred one (there might be no preferred app, even though there is a default one).

On the other hand, on Windows this will use the normal box even if there is no default handler, as long as the file is not executable.  Is there a further check for an actual default handler on Windows or something?
I should also note that the checked-in code doesn't match the comment.  Of course neither did the code that was there...
Depends on: 369431
I filed bug 369431 on the issues this patch introduced.
No longer depends on: 369431
Depends on: 369431
No longer depends on: 369431
Blocks: 556369
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: