172290 - fail gracefully when associated helper app is not executable

Status: NEW

(Firefox :: File Handling, defect)

Description

[Jeremy M. Dolan]

In the top right:

WATCH/LISTEN
 AUDIO CHOICE
Statue of former President Kim Il Sung
North Korea changes ways in fear of US

Is a link to:

http://news.bbc.co.uk/media/audio/38293000/rm/_38293903_nkorea02_grammaticas.ram

I don't have Real Player installed. Clicking this link pops up a dialog saying
an unknown error has occured, and to try saving the file to disk and then
running it.

Linux 2002100208

Comment 1

[benc]

plugins?

Comment 2

[Darin Fisher]

reporter: you just need to install realplayer to handle the .ram file format.

Comment 3

[Jeremy M. Dolan]

The point is, i'm getting an "unknown error", and mozilla is leaving a file
around in /tmp and never cleaning it up. Both of those things are bugs, no
matter what plugins I may have installed.

Comment 4

[Darin Fisher]

i see.. ok, though i cannot reproduce the problem, let's see what the file
handling folks think.

-> file handling

Comment 5

[Darin Fisher]

-> file handling

Comment 6

[Boris Zbarsky [:bzbarsky]]

I assume that you never get a filepicker, just the dialog?  What is the exact
text of the dialog?  What are your exact http settings (pipelining, etc).  Do
you use a proxy?

Finally, do you build?  If so, I can post some changes you could apply to your
tree to generate debugging output. Needless to say, I can't reproduce this
myself, with the 2002-09-30-21 nightly (and I have no way of getting anything
newer for a bit).

Comment 7

[Jeremy M. Dolan]

Think I figured out where this is coming from. This profile is from an older OS
that had realplayer installed. There's an association in helper apps pointing to
a non-existant file to run for this.

Mozilla should fail more gracefully under these circumstances. An "Unknown
error" is always a bug.

Not sure what component to send this to.

Comment 8

[Boris Zbarsky [:bzbarsky]]

This is the right component.  I'll take this, since this is linux-specific code.
 I've been meaning to clean up the error handling anyway....

Comment 9

[Christian :Biesinger (don't email me, ping me on IRC)]

bz: is this still a problem? iirc, yesterday's helper apps checkin includes a
check if the helper app is executable. (hm, maybe not if the executable flag
changes during the download, though; and maybe not if the helper app dialog is
set to not show)

Comment 10

[Boris Zbarsky [:bzbarsky]]

Exactly.  This bug is about the case when the helper app dialog never shows.

Comment 11

[Christian :Biesinger (don't email me, ping me on IRC)]

bz: Is fixing this as simple as adding an IsExecutable check here:
http://lxr.mozilla.org/seamonkey/source/uriloader/exthandler/unix/nsOSHelperAppService.cpp#188

the calling code seems to check for error codes from this function and puts up
an alert if needed

Comment 12

[Boris Zbarsky [:bzbarsky]]

Um.. We already return an error code and it's already reported (see comment 0).  So:

1)  What error code is returned?  (I suspect it's NS_ERROR_FILE_EXECUTION_FAILED,
    looking at nsProcessCommon::Run).
2)  We need to handle that error code.

Comment 13

[Boris Zbarsky [:bzbarsky]]

Not quite so simple, actually....  it looks like we trigger the:

    if (!application)
      return NS_ERROR_FAILURE;

clause.  We need to rework error handling for launching stuff in general.... :(

Comment 14

[Christian :Biesinger (don't email me, ping me on IRC)]

bz: ok, one more stupid question. could that application being null be caused by
this:
http://lxr.mozilla.org/seamonkey/source/uriloader/exthandler/unix/nsOSHelperAppService.cpp#1301

windows, for example, does not do such an exists check:
http://lxr.mozilla.org/seamonkey/source/uriloader/exthandler/win/nsOSHelperAppService.cpp#238
neither does OS/2:
http://lxr.mozilla.org/seamonkey/source/uriloader/exthandler/os2/nsOSHelperAppService.cpp#1434
nor mac:
http://lxr.mozilla.org/seamonkey/source/uriloader/exthandler/mac/nsOSHelperAppService.cpp#151

Beos does check:
http://lxr.mozilla.org/seamonkey/source/uriloader/exthandler/beos/nsOSHelperAppService.cpp#330

Comment 15

[Boris Zbarsky [:bzbarsky]]

No, that check is definitely needed; we need it elsewhere (eg see
GetMIMEInfoForExtensionFromOS).

We should probably have some logic to either not call LaunchWithApplication with
no app or to have it deal.

Comment 16

[Boris Zbarsky [:bzbarsky]]

I have no idea when I'll get to this....

Comment 17

[Christian :Biesinger (don't email me, ping me on IRC)]

testcase seems to be:
touch /tmp/myapp
set /tmp/myapp as helper app for some content
load such content