249654 - Open finished download not working on linux

Status: RESOLVED FIXED

(Toolkit :: Downloads API, defect, P2)

Description

[Angelo]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7) Gecko/20040629 Firefox/0.9.1
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7) Gecko/20040629 Firefox/0.9.1

Trying to open a finished download doesn't work on linux.

It doesn't work using open link and neither using open in the context menu

Reproducible: Always
Steps to Reproduce:
1. Download a file
2. Once finished try to open via the link
3. Try also using the context menu

Actual Results:  
Nothing happens

Expected Results:  
Use default program to open it or popup a program selection dialog

Comment 1

[Ali Ebrahim]

Yep.

Comment 2

[Ali Ebrahim]

Just to clarify, neither double click nor context menu options work to open the
file.

Comment 3

[Ali Ebrahim]

*** Bug 249786 has been marked as a duplicate of this bug. ***

Comment 4

[Ali Ebrahim]

Related to bug 232138?

Comment 5

[sairuh (rarely reading bugmail)]

I don't think this is a problem on Mac. but is this an issue on Windows?

Comment 6

[Tracy Walker [:tracy]]

Windows works.

Comment 7

[Mike Connor [:mconnor]]

nsIFile.reveal doesn't work on GTK

If I can find time, I'll post the incomplete diff that fixes show folder in the
dlmgr and in options, but opening the file directly needs some mucking around.

Comment 8

[Magnus Melin [:mkmelin]]

I'm guessing bug 67001 is quite related to this one.

Comment 9

[Ben Goodger (use ben at mozilla dot org for email)]

Can we get an alternate patch to disable all this UI for GTK? 

Comment 10

[Peter Henderson]

I know we try to stay away from Gnome and just use GTK, but a solution without
too much "mucking around" for Gnome users might be to call the gnome-open
program. It takes an argument of an URL to open (in this case a file: url) and
then opens it with Gnome's default handler for that type. It's what Gaim uses to
open links in Gnome's default browser.

The source is here, but if we used the source rather than calling it we'd have
to link with more than libgtk:
http://cvs.gnome.org/viewcvs/libgnome/libgnome/gnome-open.c?view=markup

This is comparable to KDE's `kfmclient exec file:/foo` but I haven't tracked
that one down in KDE CVS yet. I imagine it would be easy to call out to either
of these programs in much the same way that we call out to whatever program when
the user clicks on an externally-handled link.

Comment 11

[Dan Mosedale (:dmosedale, :dmose)]

Taking, at Ben's request.

Comment 12

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: patch to disable "open", v1

This patch will disable both the link and the context menu item for "open" on
GNOME platforms.

Comment 13

[Ben Goodger (use ben at mozilla dot org for email)]

I believe dmose is looking at the "Show" button in dl prefs, too. 

Comment 14

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: patch v2: fix "show folder" in prefs; disable open

Comment 15

[Mike Connor [:mconnor]]

http://lxr.mozilla.org/aviarybranch/source/toolkit/mozapps/downloads/content/downloads.js#815
also needs to be fixed (open containing folder in the DM context menu) in the
same way as Show Folder was.  That, or hidden/disabled a la Open.

It might be worthwhile to consider doing the nsIFile.reveal() bits in a
try/catch, so that if we in future do get working nsIFile.reveal, we'll pick
that up for free.  However, odds are that it would be partial, GTK-version
limited fix like the filepicker etc, so we'd already have the fallback in place
for ANY time the reveal call fails/is unsupported.

Comment 16

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: patch v3: fix "show folder"; disable "open"

Excellent idea re try/catch, Mike.  The code is definitely cleaner this way. 
I've fixed the other instance of reveal that you pointed out as well.  Once
this is reviewed, should I land it on the trunk first and then the branch?

Comment 17

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: patch v4: fix "show folder"; disable "open"

Fixed a couple of comments.

Comment 18

[Peter Henderson]

Wouldn't it make more sense to do something along the lines of:

#ifdef XP_GNOME
<!-- reveal is disabled in GNOME -->
#else
real implementation goes here
#endif

That way, the comment shows up in the GNOME build rather in all the non-GNOME
builds, where it wouldn't be relevant.

Comment 19

[Mike Connor [:mconnor]]

the less the actual code is different between platforms, the better, IMO.  Plus,
if reveal gets implemented on GTK, we pick this up for free, while leaving the
fallback for cases like the qt port, which isn't GNOME and might get support for
Reveal sooner or later than GTK.

Comment 20

[Asa Dotzler [:asa]]

Comment on attachment 162154 [details] [diff] [review]
patch v4: fix "show folder"; disable "open"

a=asa for aviary checkin pending the r=

Comment 21

[Ben Goodger (use ben at mozilla dot org for email)]

Comment on attachment 162154 [details] [diff] [review]
patch v4: fix "show folder"; disable "open"

This won't work in thunderbird... you need #ifdef MOZ_PHOENIX wrapping the
openDialog calls (just use "chrome://browser/content/browser.xul" - don't worry
about getting the pref - it's only in the Firefox case that you'll be able to
do this anyway... and in in the #else part use the External Protocol service to
open the path as it's done here:

http://lxr.mozilla.org/aviarybranch/source/toolkit/mozapps/extensions/content/e
xtensions.js#57

Comment 22

[chris hofmann]

dmose, other ideas?

Comment 23

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: patch v5: fix "show folder"; disable "open"

Fixed as per Ben's suggestion.

Comment 24

[Dan Mosedale (:dmosedale, :dmose)]

A funny thing happened on the way to this patch.  Using the external protocol is
the right thing to do, but for a different reason than expected.  It used the
default "file:" handler to open the directory... and the default handler was
Nautilus, not the browser.

So this makes me think that in fact we can fix both "show folder" and "open" by
sending them to the external handler all the time.  New patch forthcoming shortly.

Comment 25

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: patch v6: fix both "show folder" and "open"

The patch seems to work as expected in both Firefox and Thunderbird.

Comment 26

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 162640 [details] [diff] [review]
patch v6: fix both "show folder" and "open"

Requesting review; carrying forward pending approval.  I propose to check this
in on the trunk as well as the branch, since it actually fixes the real issue.

Comment 27

[Ben Goodger (use ben at mozilla dot org for email)]

Comment on attachment 162640 [details] [diff] [review]
patch v6: fix both "show folder" and "open"

r+a=[e-mail]

Comment 28

[chris hofmann]

dmose, is this checked in?   needs to go in soon to make the RC1 train

Comment 29

[Dan Mosedale (:dmosedale, :dmose)]

Fix checked in to the both the trunk and the branch.

Comment 30

[Tracy Walker [:tracy]]

verified with Linux FF branch build 2004-10-21-09-0.9

Clicking open in the download manager brings up an External Protocol Request
dialog. Clicking Launch application opens the page in FF, since I have set it to
be my default browser.

Comment 31

[Dan Mosedale (:dmosedale, :dmose)]

As Tracy points out, this now opens an External Protocol Handler warning dialog,
and people may well want to turn this off by default.  Since it's bad user
experience, and we don't want to train people to turn off this warning, dveditz
suggests whitelisting the file: protocol as far as these warnings go on Unix. 
Patch forthcoming.

Comment 32

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: patch to get rid of warning, v1

Comment 33

[Asa Dotzler [:asa]]

Comment on attachment 162954 [details] [diff] [review]
patch to get rid of warning, v1

a=asa pending reviews.

Comment 34

[Daniel Veditz [:dveditz]]

Comment on attachment 162954 [details] [diff] [review]
patch to get rid of warning, v1

r=dveditz. I don't think this is an area of combined r/sr but sr= too if it is.

Comment 35

[Ali Ebrahim]

*** Bug 249786 has been marked as a duplicate of this bug. ***

Comment 36

[Dan Mosedale (:dmosedale, :dmose)]

Fixed on the branch.  Awaiting trunk opening.

Comment 37

[Wolfgang Rosenauer [:wolfiR]]

hmm, I know that Linux is a problem here because its different windowmanagers
and desktop environments. The problem now is that it makes heavy use of GNOME in
GTK2 builds. At least this makes problems if you don't use GNOME at all.
If I use "Show folder" now in Firefox with my windowmanager "WindowMaker" it will
start up many gnome stuff and change my windowmanager's behaviour so it is
unusable without killing those stuff again.
I have no better idea at this point but the current solution makes Firefox
almost a GNOME-only application :-(

Comment 38

[Dan Mosedale (:dmosedale, :dmose)]

Fixed on the trunk.

Wolfgang, the problem is that there's currently no standard way to do this.  It
could perhaps be implemented in a KDE-specific way as an extension.  At some
point, there will hopefully be a standard way to get this info.  The
shared-mimetypes stuff on freedesktop.org is at least a start in the right
direction.  Feel free to file a separate bug on that, if there isn't one already.