109236 - External helper app service ignores extensions from helper app preferences

Status: VERIFIED FIXED

(Core Graveyard :: File Handling, defect, P1)

Description

[Boris Zbarsky [:bzbarsky]]

When we do GetFromExtension() we never use the user-defined RDF datasource for
information.  We should.  Patch coming up.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch to add a way to look up extensions in the RDF datasource

Comment 2

[Boris Zbarsky [:bzbarsky]]

Reviews?  This is a long-standing gripe, but I could find no bugs on this...

Comment 3

[Boris Zbarsky [:bzbarsky]]

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

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 57187 [details] [diff] [review]
Patch to add a way to look up extensions in the RDF datasource

tingley pointed out that there needs to be a
success check on mimeinfo creation.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: Safer patch

Comment 6

[Mike Kaply [:mkaply]]

Any idea what the difference between GetFromExtension and GetTypeFromExtension 
is?

I added similar code for plugins to GetTypeFromExtension. (which is bad 
incidentally because pluginhost is not threadsafe)

Do we want to create a hierarchy for what type gets used? For instance, if 
someone has the same extension in plugin and helpers with two different mime 
types, which gets used?

Comment 7

[Boris Zbarsky [:bzbarsky]]

GetFromExtension creates a MIME info object.  This includes a type, extension
list, how to handle it (eg helper, save to disk, etc).

GetTypeFromExtension takes an extension and returns a MIME type.  This type is
later passed to GetFromMimeType to get a mime info object.  GetFromExtensions
first tries to get a mime info object for that extension and then looks at the
type of that.  If this fails, it tries plugins.  So currently, for loads from
ftp/file we do the following (with this patch):

1) check mime cache for extension
2) check helper app prefs for extension
3) check plugin list for extension

For loads from http we do, as far as I can tell:

1) check plugin list for type (this happens _outside_ the mime service)
2) check mime cache for type
3) check helper app prefs for type
4) check mime cache for extension
5) check helper app prefs for extension

Resolving the order discrepancy would involve one of two things.  Either
checking plugins before mime cache (poor, imo) or integrating the plugin
mimetype stuff with the mime service (why is it not that way already??) so that
GetFromExtension and GetFromMimeType both ask cache, plugins, user prefs in the
same order.  That's beyond the scope of this bug, imo.

Comment 8

[Boris Zbarsky [:bzbarsky]]

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

Comment 9

[Bill Law]

Comment on attachment 57197 [details] [diff] [review]
Safer patch

r=law

This sounds like a good thing, I just hope it doesn't cause pain in some
yet-to-be-found cases...

I think mscott would be a good super-reviewer, BTW.

Comment 10

[Bill Law]

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

Comment 11

[Scott MacGregor]

Comment on attachment 57197 [details] [diff] [review]
Safer patch

sr=mscott although i'm nervous 'cause these kinds of changes to the exthandler
always seem to manage to break something....we'll just have to keep our eyes
peeled.

Comment 12

[Boris Zbarsky [:bzbarsky]]

Yep.  Will do...

Comment 13

[Boris Zbarsky [:bzbarsky]]

checked in

Comment 14

[Boris Zbarsky [:bzbarsky]]

reopening.  Backed out due to mac build bustage

Comment 15

[Boris Zbarsky [:bzbarsky]]

Checked in again.  Marking fixed again.  This time I waited till it was all done
building...

Comment 16

[sairuh (rarely reading bugmail)]

is there are particular test case[s] i should run for verifying this? or should
i just give it an rs verify?

Comment 17

[Mike Kaply [:mkaply]]

I'll check this.

OS/2 had it worse than any other platform.

Comment 18

[Boris Zbarsky [:bzbarsky]]

To test, on OS/2 or Linux,

1)  Create a file with some extension
2)  In helper app prefs create a helper for some type, mapped to that extension,
    with some handler app.
3)  Load the file via a file:// url and see whether the handler you set up is
    used.

Don't do this to Postscript or PDF (or HTML) files because we content-sniff
those...  I suggest some plaintext file, give it a funky extension and set up a
text editor as the helper.

Comment 19

[Mike Kaply [:mkaply]]

Yep, works like a charm.

Note this is only on binary files.

Mozilla still tries to load text files regardless of the extension.

Comment 20

[Stanisa Amidzic]

I think something was missed here. If you refer to bug 99244 (which accidently
was marked as a duplicate of this one), you will find that the original bug was
in the way Mozilla deals with these files, ie. text files are open in the
browser regarless of their extension type. Is this being looked at?

Comment 21

[Boris Zbarsky [:bzbarsky]]

Text files should _not_ be opening in the browser regardless of extension.  I
talked to Michael today and he says that the text-file stuff works correctly for
him once he tried a build from today (includes a fix for some case-sensitivity
problems that I landed yesterday).

If you have a current (today or later) build, you have just created your helper
app entry, and you're having a problem with text files, please comment in this
bug with more info...