327323 - Can't "Open with" files that are send as application/octet-stream (or other "unknown to firefox" mime types)

Status: RESOLVED FIXED

(Core Graveyard :: File Handling, defect)

Description

[Jesper Juhl]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.0.1) Gecko/20060124 Firefox/1.5.0.1
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.0.1) Gecko/20060124 Firefox/1.5.0.1

Whenever I click a link to a file that the server sends as application/octet-stream (or some mime type that firefox knows nothing about) I don't get the "Open with" option, only the option to download the file.

This is a bug in my oppinion, since even if firefox can't tell what to do with the file, *I* might very well know what to do with it and should be given the option to select a program to open it with instead of being forced to save it first and then have to manually launch my program of choice and then open the file there.

In fact, I would personally prefer if firefox would *always* give me the option to open a file with an application of my choice, *even if* it thinks it already knows what to do, since I may not *always* want to open files with the same app even if I had configured one as the default for a certain file type.

I'll attach a screenshot to this bug-report showing a mpeg file being served to me by the server as application/octet-stream where firefox refuses to let me do anything other than save the file.


Reproducible: Always

Steps to Reproduce:
1. click on a link to a file that will be served with application/octet-stream.
2. observe that you are only allowed to save the file.

Actual Results:  
Only "Save to Disk" is possible.

Expected Results:  
"Open with" and "Save to Disk" should both be available.

Comment 1

[Jesper Juhl]

Attachment: Image showing that I can only "Save to Disk", not "Open with" when a server sends a file as application/octet-stream.

Comment 2

[Ria Klaassen (not reading all bugmail)]

The Open With extension has this functionality.

Comment 3

[Svetoslav Milenov]

This problem still exists in Firefox 3.0 on Linux (opensuse 11.0). With FF3 on windows, it allows to select "Open with..."

Comment 4

[Svetoslav Milenov]

Attachment: "Open" dialog after selecting known type, send as application/octet-stream

Still in Firefox 3 it recognizes the file type, but because of the bad mime type send (application/octet-stream) it does not provide "Open with..."

Comment 5

[Svetoslav Milenov]

I tested it on Ubuntu 8.04 live cd, and it has the same problem - so it looks like this Firefox 3.0 for linux only, as on windows it works OK.

Also, I did not have this problem with opensuse 10.3 and firefox 2.0, so looks like a regression.

Comment 6

[Chris]

I can confirm that this still isn't fixed under Ubuntu 8.04.1. Hope somebody can fix it as returning the office workstations to windows is undesirable! (Had a lot of complaints).

Comment 7

[Wolfgang Rosenauer [:wolfiR]]

Attachment: testdownload

Comment 8

[Wolfgang Rosenauer [:wolfiR]]

Ok, I had a short look at why application/octet-stream is handled that way:

Currently (Firefox 3.0.1) if the mime-type is application/octet-stream (or application/x-msdownload) a simplified download UI is used which only allows a download of the file if there is no default application defined (which is the case on Linux at least for application/octet-stream).

If that simplified wouldn't be used "Open" would still be denied because of:

      // We don't let users open .exe files or random binary data directly 
      // from the browser at the moment because of security concerns. 

(that's a source code comment and so it seems that this behaviour/regression was intended in the first place)

The overall question is if the HelperAppService is supposed to return a default handler for the recognized file extension if application/octet-stream is used as mime-type?

Anyway there is a quick'n'dirty workaround to get a usable download UI back:
Add for example the following line to your ~/.mailcap:

application/octet-stream;/bin/true "%u"

By doing that the HelperAppService returns a default application (/bin/true) and the above constraints don't apply anymore. So you'll also get back the other alternatives returned by the MIMEInfo.

Comment 9

[Wolfgang Rosenauer [:wolfiR]]

Given that on Windows FF3 offers immediately to open such a download with the right application (testdownload above is suggested to be opened with Adobe Reader) it's seems to be ok to fall back to the extension mapping and ignoring the application/octet-stream mimetype returned by the webserver.

So the solution to this issue on Linux would be to make sure that (in addition to mime-types/mailcap also Gnome VFS should return a default application when there is none for the server defined mime-type.

Comment 10

[Johannes Müller-Lahn]

Indeed, on Windows FF3 works as expected. Sorry, to say that, but your quick'n'dirty solution, doesn't work proberbly. I can see, that the Firefox suggets, /bin/true for the download, and if you press OK, nothing happens (the default application doesn't open it). 
Tested it on two fresh Installations of OpenSuSE 11.0 and Firefox 3.0.1.

Comment 11

[Wolfgang Rosenauer [:wolfiR]]

It depends. /bin/true doesn't open the application obviously but on my system I get a second offer (evince as PDF reader for the attached testcase) which works correctly. If you get no choice I wonder if you have a handler application which is known to gnome-vfs.

Comment 12

[Wolfgang Rosenauer [:wolfiR]]

Attachment: nspr logging for HelperAppService with the attached testcase

Just for completeness an NSPR log created with FF 3.0.1

Comment 13

[Wolfgang Rosenauer [:wolfiR]]

Comment on attachment 212006 [details]
Image showing that I can only "Save to Disk", not "Open with" when a server sends a file as application/octet-stream.

FF3 is showing a different dialog, so obsoleting this

Comment 14

[Wolfgang Rosenauer [:wolfiR]]

Comment on attachment 328399 [details]
"Open" dialog after selecting known type, send as application/octet-stream

Screenshot shows a dialog which is modified by an addon.

Comment 15

[Johannes Müller-Lahn]

Attachment: Picture with enabled /bin/true for octect-stream in ~/.mailcap

Comment 16

[Johannes Müller-Lahn]

I don't get a second choice for your testfile. I made a screenshot for that: https://bugzilla.mozilla.org/attachment.cgi?id=334460

>If you get no choice I wonder if you have a handler application which is known to gnome-vfs.

I don't use gnome-vfs at all, maybe you can tell me if I need to install another program for handling the files correctly from firefox!?

Comment 17

[Wolfgang Rosenauer [:wolfiR]]

Yes, sorry, I got a second choice for another reason.
I have a patch in hand which would fix this issue. I want to discuss it first with mozilla people though.

Comment 18

[Wolfgang Rosenauer [:wolfiR]]

Attachment: possible patch

Comment 19

[Wolfgang Rosenauer [:wolfiR]]

Comment on attachment 334521 [details] [diff] [review]
possible patch

This patch checks if we can get a default handler for the extension if we haven't found one for the type. And if so, we set the type to the extension match.
This is technically pretty obvious but I wonder if that is the "right thing" to overwrite a valid server provided type.

Comment 20

[Boris Zbarsky [:bzbarsky]]

I'm not sure I follow this patch.  Apart from crashing when miByExt is null, what happens?  Currently, if miByExt has a default handler there are two options:

1) Have a retval.  In this case, we CopyBasicDataTo and return miByExt.
2) No retval.  In this case we're hitting the SetMIMEType code already.

So the patch only affects behavior in the case when we have both retval and miByExt.  But all it changes is to call SetMIMEType instead of CopyBasicDataTo,

Is CopyBasicDataTo clobbering the default app somehow in this case?

As far as the "right thing" bit, the point of this function is to use both type and extension to get our best guess of how to handle.  So we certainly want to use the default app for the extension if there isn't one for the type!

Comment 21

[Wolfgang Rosenauer [:wolfiR]]

Attachment: patch #2

> I'm not sure I follow this patch.  Apart from crashing when miByExt is null,
> what happens?  Currently, if miByExt has a default handler there are two
> options:

oops, sorry for the crash. Should be fixed now.
I think this patch works but probably you see a better way to fix it.

> 1) Have a retval.  In this case, we CopyBasicDataTo and return miByExt.
> 2) No retval.  In this case we're hitting the SetMIMEType code already.
>
> So the patch only affects behavior in the case when we have both retval and
> miByExt.  But all it changes is to call SetMIMEType instead of 
> CopyBasicDataTo,
>
> Is CopyBasicDataTo clobbering the default app somehow in this case?

Not the default app but ...

Let me explain what currently happens using the special case this bug is about:

- type is application/octet-stream
- ext is for example .pdf
- nsMIMEInfoBase* retval = GetFromType(PromiseFlatCString(aType)).get();
  succeeds but has no default handler
- nsRefPtr<nsMIMEInfoBase> miByExt =   
  GetFromExtension(PromiseFlatCString(aFileExt));
  succeeds and returns a valid mimeinfo containing even a default handler
-> now we call retval->CopyBasicDataTo(miByExt);
-> 
so it seems we end up with a mimeinfo which actually contains a default handler
but again uses type application/octet-stream.

That also means that every call to GetHasDefaultHandler() will return false again since it only checks for a handler for the type again.

Later in nsHelperAppDlg.js we handle application/octet-stream filetypes which have no default handler so that no helper apps are offered at all.

Comment 22

[Boris Zbarsky [:bzbarsky]]

Ending up with a MIME info that has type application/octet-stream and has a default handler is correct.

Sounds to me like that impl of GetHasDefaultHandler is buggy.

Comment 23

[Wolfgang Rosenauer [:wolfiR]]

Attachment: another approach

That is a fix for nsMIMEInfoUnix::GetHasDefaultHandler() to actually also check for the extension if it fails for the type lookup.
(not familiar with nsRefPtr so I might be wrong with it)

But for that to actually offer the default application I had to change CopyBasicDataTo since it overwrites the default application after all.

Comment 24

[Johannes Müller-Lahn]

Attachment: Missing helper application error

Comment 25

[Johannes Müller-Lahn]

I just applied your patches, and now I get an error, that the helper application for the file is missing. Tried it also on an image file, same problem on that one.

Comment 26

[Wolfgang Rosenauer [:wolfiR]]

(In reply to comment #25)
> I just applied your patches, and now I get an error, that the helper
> application for the file is missing. Tried it also on an image file, same
> problem on that one.

You only should use the "another approach" patch. Did you do that? I'm not sure how that would trigger your issue. And you get the error immediately when clicking on a download link?

Comment 27

[Johannes Müller-Lahn]

> You only should use the "another approach" patch. Did you do that? I'm not sure
> how that would trigger your issue.
I installed the mozilla-xulrunner190-1.9.0.1-4.1 from openSuSE Buildservice Mozilla Beta Repository, which you mentioned in the email, that you wrote me. I don't know which patch you applied for that one.

> And you get the error immediately when clicking on a download link?
No, when I klick on the testfile.pdf link, I do first get the download window, where the correct helper application is selected (haven't seen that before the patch). When I klick OK, the helper application (Acrobat Reader in this case) doesn't start and the error (screenshot) appears.

Comment 28

[Wolfgang Rosenauer [:wolfiR]]

Attachment: another approach #2

Comment 29

[Wolfgang Rosenauer [:wolfiR]]

Comment on attachment 334886 [details] [diff] [review]
another approach #2

>--- uriloader/exthandler/nsMIMEInfoImpl.cpp	19 Oct 2007 04:15:43 -0000	1.69
>+++ uriloader/exthandler/nsMIMEInfoImpl.cpp	20 Aug 2008 15:45:32 -0000
>@@ -408,13 +408,13 @@ nsMIMEInfoBase::LaunchWithURI(nsIURI* aU
> }
> 
> void
> nsMIMEInfoBase::CopyBasicDataTo(nsMIMEInfoBase* aOther)
> {
>   aOther->mType = mType;
>-  aOther->mDefaultAppDescription = mDefaultAppDescription;
>+  //aOther->mDefaultAppDescription = mDefaultAppDescription;

When it comes to gnome-vfs support overwriting mDefaultAppDescription is wrong in any case.
From looking at the code I'm not sure when it would make sense.


> NS_IMETHODIMP
> nsMIMEInfoUnix::GetHasDefaultHandler(PRBool *_retval)
> {
>   *_retval = PR_FALSE;
>-  nsCOMPtr<nsIGnomeVFSService> vfs = do_GetService(NS_GNOMEVFSSERVICE_CONTRACTID);
>-  if (vfs) {
>-    nsCOMPtr<nsIGnomeVFSMimeApp> app;
>-    if (NS_SUCCEEDED(vfs->GetAppForMimeType(mType, getter_AddRefs(app))) && app)
>-      *_retval = PR_TRUE;
>+  nsRefPtr<nsMIMEInfoBase> mimeInfo = nsGNOMERegistry::GetFromType(mType);
>+  if (!mimeInfo) {
>+     nsCAutoString ext;
>+     if (NS_SUCCEEDED(GetPrimaryExtension(ext)))
>+       mimeInfo = nsGNOMERegistry::GetFromExtension(ext);

I changed that to use nsGNOMERegistry since it's a bit shorter to implement.
I'm not sure why it was using the gnome-vfs component directly while the module already depends on nsGNOMERegistry but if there was a valid reason that could be changed.

>   if (vfs) {
>     nsCOMPtr<nsIGnomeVFSMimeApp> app;
>     if (NS_SUCCEEDED(vfs->GetAppForMimeType(mType, getter_AddRefs(app))) && app)
>       return app->Launch(nativePath);
>+
>+    // If we haven't got an app we try to get a valid one by searching for the
>+    // extension mapped type
>+    nsRefPtr<nsMIMEInfoBase> mimeInfo = nsGNOMERegistry::GetFromExtension(nativePath);
>+    if (mimeInfo) {
>+      nsCAutoString type;
>+      mimeInfo->GetType(type);
>+      if (NS_SUCCEEDED(vfs->GetAppForMimeType(type, getter_AddRefs(app))) && app)
>+        return app->Launch(nativePath);
>+    }

While commenting that part I actually think there should be a return value check for GetType()?


>     // Copy the attributes of retval onto miByExt, to return it
>     retval->CopyBasicDataTo(miByExt);
>+    // But set the extensions primary since CopyBasicDataTo overwrites the
>+    // list
>+    if (!aFileExt.IsEmpty())
>+      retval->SetPrimaryExtension(aFileExt);

This was actually a side effect by trying to fix the issue.
I _think_ that should be done but I might be wrong.

Comment 30

[Johannes Müller-Lahn]

So when can we expect to have it in the upstream release? The patch is working perfectly, and we need to have it the core!

Comment 31

[Wolfgang Rosenauer [:wolfiR]]

Comment on attachment 334886 [details] [diff] [review]
another approach #2

biesi probably has no time.
The patch might not apply to mozilla-central anymore but I have an updated one but probably take some criticism before ;-)

Comment 32

[Boris Zbarsky [:bzbarsky]]

What's the point of the CopyBasicDataTo change?  The idea of that function is to copy whatever information you have, no?  The header certainly documents it as copying that info.  Look up why that was done originally?  In any case, in your situation I can't see why removing this line is needed.

I'm not happy with the random object-creation stuff going on here.  Why not change nsGNOMERegistry to expose functions to get a helper by type or extension and use them both in your new code and in the existing nsGNOMERegistry code?

Always setting the extension on the MIME info bothers me, though I can't figure out why.  This is a situation where we got a MIME info for the MIME type, right?  And the only issue was that this MIME info didn't have a default handler, which is why we fell back on getting one by extension?  I guess we need to set the extension there to make the whole "get default app" thing work later...

Could we fix that by actually storing the default app in the MIMEInfo at construction time instead of messing around with hitting the GNOME registry every single time someone asks something of the MIME info?  Or is there a major problem with that approach?  I'm not sure why it wasn't taken here to start with.

Comment 33

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 334886 [details] [diff] [review]
another approach #2

r- pending answers to my questions.

Comment 34

[Aaron Sloman]

(In reply to comment #8)

> Anyway there is a quick'n'dirty workaround to get a usable download UI back:
> Add for example the following line to your ~/.mailcap:
> 
> application/octet-stream;/bin/true "%u"
> 
> By doing that the HelperAppService returns a default application (/bin/true)
> and the above constraints don't apply anymore. So you'll also get back the
> other alternatives returned by the MIMEInfo.

This works (for me) insofar as it provides a menu with the 'other' option, so that I can specify e.g. an editor to display the file. But it doesn't remember the editor. So, because my preferred editor is the poplog editor xved, I used the following in my .mailcap file:

application/octet-stream;/usr/local/bin/xved "%u"

I can now click on the individual files in here and read them in the editor, whereas previously I had to download and read:
http://ia350607.us.archive.org/3/items/PSYCHE-D/

I still have the 'other' menu option to use if necessary.

Thanks very much for the suggestion that led me to this.

Comment 35

[Wolfgang Rosenauer [:wolfiR]]

Attachment: updated patch

Updated to current mozilla-central and slightly changed to not change CopyBasicDataTo()

Comment 36

[Wolfgang Rosenauer [:wolfiR]]

(In reply to comment #32)
> What's the point of the CopyBasicDataTo change?  The idea of that function is
> to copy whatever information you have, no?  The header certainly documents it
> as copying that info.  Look up why that was done originally?  In any case, in
> your situation I can't see why removing this line is needed.

The latest patch should explain why it's needed but I'm doing it differently now and not touching the cross platform method at all.
In written words: First we are getting a mimeinfo by type with no "default description" and because of that we try using the extension and get a valid result containing a default description. But afterwards we are overwriting the by-ext-mimeinfo with "basic" data in CopyBasicDataTo() including the empty default description. The new version of the patch sets the default description we just got also in the by-type-mimeinfo to preserve it after the copy.

> I'm not happy with the random object-creation stuff going on here.  Why not
> change nsGNOMERegistry to expose functions to get a helper by type or extension
> and use them both in your new code and in the existing nsGNOMERegistry code?

I've read this a few times but I don't get what you are proposing in the end.
There are functions to get helpers by type or extension which are used.
Currently I have only changed the obvious code to use nsGNOMERegistry but also kept one use of nsIGnomeVFSService in LaunchDefaultWithFile(). Is that what you mean what should be changed?

> Always setting the extension on the MIME info bothers me, though I can't figure
> out why.  This is a situation where we got a MIME info for the MIME type,
> right?  And the only issue was that this MIME info didn't have a default
> handler, which is why we fell back on getting one by extension?  I guess we
> need to set the extension there to make the whole "get default app" thing work
> later...

My intention was to get a "valid" mimeinfo which "fits" the actual file.

> Could we fix that by actually storing the default app in the MIMEInfo at
> construction time instead of messing around with hitting the GNOME registry
> every single time someone asks something of the MIME info?  Or is there a major
> problem with that approach?  I'm not sure why it wasn't taken here to start
> with.

I don't really understand what you mean. My intention was to fix that bug (hopefully without introducing new ones) w/o much refactoring of the code. I'm happy to try to improve it further but I'm not that familiar with all of that code.

Comment 37

[Johannes Müller-Lahn]

So this bug is opened for 3 1/2 years, can we expect this somewhen in upstream releases!? 3.0.X is fixed on openSuSE repos, but the 3.5.X series is still broken. Thanks goes to Wolfgang Rosenauer for still contributing fixes to this bugzilla entry, but I really don't understand why Mozilla staff isn't willing to apply/work on this patch!

Comment 38

[Boris Zbarsky [:bzbarsky]]

This patch is on my list of things to review.  I estimate that it will take me several days of work to do so, since the code is underdocumented, fragile, and I haven't seriously looked at it in years.  In case you missed it, the patch was posted a few weeks ago; I was away for two weeks of that time.  Before that, there was a 7-month lag after my last round of review comments when nothing happened.

I _am_ hoping to get to this review in the next week.

If you want to help instead of just ranting, either take the higher-priority security bug work off my plate, or take some of the higher-priority reviews at http://bugzilla.mozilla.org/request.cgi?action=queue&requester=&product=&type=all&requestee=bzbarsky%40mit.edu&component=&group=requestee off my plate.  If you can't do either of those, and can't help in some other way that would free up some of my time (dealing with triage of incoming form controls, docshell, and xbl bugs, profiling various pageload issues, writing tests for the run-in section of the CSS2.1 specification, all come to mind as low-barrier-to-entry ways you could help there), then I'm not sure where your "I'm entitled to you not sleeping at night and instead reviewing this patch I happen to care about" attitude comes from.

> but I really don't understand why Mozilla staff isn't willing to apply

The earlier versions weren't applied because they were wrong.

> work on this patch!

I'm not sure what you think "mozilla staff" is.  This area of code is basically unowned; some Linux distro folks wrote it and then abandoned it.  I really appreciate Wolfgang stepping up to work on it, and he's the closest to "mozilla staff" as far as this code is concerned.

Comment 39

[Johannes Müller-Lahn]

>This patch is on my list of things to review.  I estimate that it will take me several days of work to do so, since the code is underdocumented, fragile, and I haven't seriously looked at it in years.  

I didn't know that the code isn't maintained!

>In case you missed it, the patch was posted a few weeks ago; I was away for two >weeks of that time.  Before that, there was a 7-month lag after my last >round of >review comments when nothing happened.

The workaround in openSuSE repos from Wolfgang Rosenauer worked so far, so I didn't check in here as well.

>I _am_ hoping to get to this review in the next week.

Thats great to hear.

>If you want to help instead of just ranting, either take the higher-priority
security bug work off my plate, or take some of the higher-priority reviews at
http://bugzilla.mozilla.org/request.cgi?action=queue&requester=&product=&type=all&requestee=bzbarsky%40mit.edu&component=&group=requestee
off my plate.  If you can't do either of those, and can't help in some other
way that would free up some of my time (dealing with triage of incoming form
controls, docshell, and xbl bugs, profiling various pageload issues, writing
tests for the run-in section of the CSS2.1 specification, all come to mind as
low-barrier-to-entry ways you could help there), then I'm not sure where your
"I'm entitled to you not sleeping at night and instead reviewing this patch I
happen to care about" attitude comes from.

It's not on you Boris, I know that you're fully loaded with work. It was just a question if this will be ever fixed. You know, in our case were are using a webased groupware system, where nearly 95 % of the email attachments are recognized as application/octet-stream. So without the patch from Wolfgang, it's a real mess to work with firework. So the fix works for 3.X series, but 3.5 doesn't. It's a blocker for me to migrate our network, approx. 100 workstations to a newer openSuSE version with it's new firefox 3.5. I'm not a coder at all, but if I could write code like you guys, I would really like to help you out.

>I'm not sure what you think "mozilla staff" is.  This area of code is basically
unowned; some Linux distro folks wrote it and then abandoned it.  I really
appreciate Wolfgang stepping up to work on it, and he's the closest to "mozilla
staff" as far as this code is concerned.

Like I said, I didn't know that the code isn't maintained. "Mozilla staff" are the guys like you, who are working on the browser.

I really appreciate the work you do. I just wanted to know if the problems will be ever fixed, because I think 3 1/2 years is a real long time. Sorry for being a lil bit rude.

Comment 40

[Boris Zbarsky [:bzbarsky]]

> I've read this a few times but I don't get what you are proposing in the end.

I think what I was proposing was that instead of using "can I get a MIME info object" as a proxy for "there is a default handler" we actually have a way to ask (probably the GNOME registry) "is there a handler for this type" and "is there a handler for this extension?"

I'm fine with this being a followup, I guess.

That might also be cheaper than creating the mime info objects, though I guess .hasDefaultHandler and .lanchDefaultWithFile are not exactly performance-sensitive.

> I don't really understand what you mean.

I mean, just cache a boolean in the mimeinfo for whether it has a default handler and cache the app to use in LaunchDefaultWithFile.  But I guess that would in fact take a bit more surgery (esp. for the latter), and not necessarily win us much other than code clarity....  Which is something this code doesn't have anyway, so it's not like this patch is making it worse.

Wolfgang, what worries me about this setup is that if I send an application/pdf file with a .pl extension your browser will happily run it through a perl interpreter with this patch, right?  And if a firewall was trying to enforce things based on MIME type, we lose.

Similarly, if I ask for the MIME info for a text/html file with a .cgi extension, and there is no default text/html handler but there _is_ a .cgi handler, we'll come back with a MIME info for text/html and extension .cgi.  Not that likely, since we have default handlers for text/html, but if you have a CGI that produces types you do _not_ have default handlers for (e.g. word documents or C++ files or whatever) then this could well affect the "save as" codepath: see the getDefaultExtension function in contentAreaUtils.js.  Automated testing coverage for this stuff is spotty; what sort of manual testing has been done here?

Johannes, if you want to help, can you help with testing this patch along the lines described above?

Comment 41

[Boris Zbarsky [:bzbarsky]]

And to be clear, what I'm trying to avoid is a user trying to save a word document (say) and getting foo.doc.cgi as the suggested filename.

Comment 42

[Wolfgang Rosenauer [:wolfiR]]

(In reply to comment #40)
> Wolfgang, what worries me about this setup is that if I send an application/pdf
> file with a .pl extension your browser will happily run it through a perl
> interpreter with this patch, right?  And if a firewall was trying to enforce
> things based on MIME type, we lose.

if there is not default handler for application/pdf but we know one for *.pl that will be the case indeed.
I see the issue but what would be the better option? I.e. what is the desired behaviour?
Only fall back to the extension for application/octet-stream?
 
> Similarly, if I ask for the MIME info for a text/html file with a .cgi
> extension, and there is no default text/html handler but there _is_ a .cgi
> handler, we'll come back with a MIME info for text/html and extension .cgi. 
> Not that likely, since we have default handlers for text/html, but if you have
> a CGI that produces types you do _not_ have default handlers for (e.g. word
> documents or C++ files or whatever) then this could well affect the "save as"
> codepath: see the getDefaultExtension function in contentAreaUtils.js. 
> Automated testing coverage for this stuff is spotty; what sort of manual
> testing has been done here?

Hmm, I haven't seen that issue with that patch but I don't think I've tried to actually trigger such a case.
The patch should still basically do the right thing without setting the primary extension though. I'll try to verify...

Comment 43

[Johannes Müller-Lahn]

I actually tried to test the new patch myselfs:

1.) Downloaded and extractet he source "firefox-3.5.3.source.tar.bz2"
2.) Applied the patch (updated patch (3.10 KB, patch)) :
patch -p1 < ../../../patch.diff
patching file uriloader/exthandler/unix/nsMIMEInfoUnix.cpp
Hunk #1 succeeded at 43 with fuzz 2 (offset -1 lines).
Hunk #3 succeeded at 110 with fuzz 2 (offset -4 lines).
patching file uriloader/exthandler/unix/nsOSHelperAppService.cpp
Hunk #1 succeeded at 1643 (offset 66 lines).

3.) configure with --enable-application=browser and run make

When I know run firefox and try to open those application/octet-stream files it doesn't work. I just get the save-to dialog, and no chance to open the file directly the correct application!

Did I do something wrong, or used the wrong source?

Comment 44

[Boris Zbarsky [:bzbarsky]]

> I.e. what is the desired behaviour?

I would be tempted to consider only falling back for application/octet-stream, yeah....  I guess we can try the way it is now and try to get a security audit of the setup done by someone who understands the tangled mess of GNOME/etc file associations?

> I'll try to verify...

That would be great.

> Did I do something wrong, or used the wrong source?

Seems like the right thing to me, pretty much...  I assume you did make sure you ran your build, not whatever firefox happened to be installed on your system (and already running)?

Comment 45

[Boris Zbarsky [:bzbarsky]]

Wolfgang, any luck on verifying per comment 42?  Also, any idea what's going on in comment 33?

Comment 46

[Wolfgang Rosenauer [:wolfiR]]

(In reply to comment #45)
> Wolfgang, any luck on verifying per comment 42?

I tried w/o setting the primary extension and I still get the default PDF viewer of the testcase in that bug.
I also talked to Johannes if he ever saw wrong extensions during his use and apparently he didn't. But the part of setting the primary extension was only a bonus and not mandatory to fix the issue.

> Also, any idea what's going on
> in comment 33?

You mean comment 43?
Not quite sure but I talked to Johannes about that also and he confirmed that my packages work for him iirc. Something with his compilation wasn't quite correct I assume.
I'll attach the current patch shortly for completeness but I only removed the SetPrimaryExtension call.
If you would feel a lot better I could also add the application/octet-stream filter while it would be nice (if security is good enough) to not having it limited to that.

Comment 47

[Johannes Müller-Lahn]

> I also talked to Johannes if he ever saw wrong extensions during his use and apparently he didn't. 

Thats right. He never selected the wrong helper application. Tested the patch for a couple of month, and I can confirm that it's fixed 100 %. 

> Not quite sure but I talked to Johannes about that also and he confirmed that
my packages work for him iirc. Something with his compilation wasn't quite
correct I assume.

I messed up with compiling from scratch. I tried it a couple of times, but I didn't succeed. Afterwars I use the openSuSE repos for Mozilla and installed the 3.5 series that are containg the patch from Wolfgang. The patch is also working perfectly like the ones from the default repos from openSuSE 11.0 (Firefox 3.0.X).

Comment 48

[Boris Zbarsky [:bzbarsky]]

Wolfgang, looking forward to that current patch.

Also, can you push yourself, or should I just push the patch once it's reviewed?

Comment 49

[Wolfgang Rosenauer [:wolfiR]]

Attachment: patch (mozilla-central)

Patch for mozilla-central w/o SetPrimaryExtension

(I have hg write access and can push it once it's done)

Comment 50

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 406796 [details] [diff] [review]
patch (mozilla-central)

Let's try this!

Comment 51

[Wolfgang Rosenauer [:wolfiR]]

http://hg.mozilla.org/mozilla-central/rev/99f24b52e463

Comment 52

[Tyler Gates]

Does mozilla 1.9.3 mean this will be fixed in firefox 3.7? I have 3.5.8 and 3.5.1 and this is still a problem. 
A long shot but does anyone have an up to date patch for 3.5.1? The latest provided here doesn't work and there have been quite a bit of changes to the code since then.

Comment 53

[Boris Zbarsky [:bzbarsky]]

> Does mozilla 1.9.3 mean this will be fixed in firefox 3.7?

Yes.

Comment 54

[Tyler Gates]

Does mozilla 1.9.3 mean this will be fixed in firefox 3.7? I have 3.5.8 and 3.5.1 and this is still a problem. 
A long shot but does anyone have an up to date patch for 3.5.1? The latest provided here doesn't work and there have been quite a bit of changes to the code since then.

Comment 55

[Tyler Gates]

Apologies for the double post.

Comment 56

[Tyler Gates]

Attachment: nsMIMEInfoUnix.patch -works with firefox 3.6.3

A working patch to fix this issue in firefox 3.6.3

Comment 57

[Tyler Gates]

Attachment: nsOSHelperAppService.patch -works with firefox 3.6.3

A working patch for this issue in firefox 3.6.3

Comment 58

[Murray]

Running Firefox 3.5, I've added this line to ~/.mailcap

application/octet-stream;/usr/bin/gnome-open "%u"

Comment 59

[Murray]

(In reply to comment #58)
> Running Firefox 3.5, I've added this line to ~/.mailcap
> 
> application/octet-stream;/usr/bin/gnome-open "%u"

Also for a all-user fix, add the line to a file under /usr/lib/mime/packages/ and run mime-update

Comment 61

[Chris Coulson]

Attachment: Patch for mozilla-1.9.2

Requesting for 1.9.2.10. This patch also indirectly fixes bug 455626, which I see a lot of people keep reporting in Ubuntu. Would be really nice to see that one fixed.

I had to drop the GIO parts for 1.9.2

Comment 62

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 471859 [details] [diff] [review]
Patch for mozilla-1.9.2

r=bzbarsky

Comment 63

[Daniel Veditz [:dveditz]]

Comment on attachment 471859 [details] [diff] [review]
Patch for mozilla-1.9.2

Approved for 1.9.2.11, a=dveditz for release-drivers (although code-freeze for 1.9.2.11 is tomorrow, Sept 28)

Comment 64

[Wolfgang Rosenauer [:wolfiR]]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/26701f428134