282958 - toolkit/mozapps/downloads/content uses wrong contractid for the MIME service

Status: RESOLVED FIXED

(Core Graveyard :: File Handling, defect)

Description

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

toolkit/mozapps/downloads/content uses
"@mozilla.org/uriloader/external-helper-app-service;1" where
"@mozilla.org/mime;1" would be correct, since the code just cares about the mime
service functionality.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Note that this is particularly a problem if an extension wants to drop in a
better MIME service....  It can override "@mozilla.org/mime;1" without too much
trouble, whereas overriding the other contractid is a pain (since it's used from
inside uriloader and takes a lot more effort to do right).

Comment 2

[Christian Persch (GNOME) (away; not receiving bug mail)]

Attachment: patch

Comment 3

[Shawn Wilsher :sdwilsh]

I'm not sure we want to change *all* of those.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Sure looks to me like you do, if all you use them for is nsIMIMEService.

Comment 5

[Christian Persch (GNOME) (away; not receiving bug mail)]

All those files only call methods from nsIMIMService on them, so I think switching them all should be fine.

I discovered one more place that might need to change: nsExternalHelperAppService.cpp::DoContent calls GetFromTypeAndExtension. That's a nsIMIMEService method, but it calls it on itself (the same class is implementing the nsIMIMService). I think to make nsIMIMService really replaceable, these need to be switched to get the MIME service by its contract and use that, right?

Comment 6

[Boris Zbarsky [:bzbarsky]]

Yeah, that would probably need to happen.

Comment 7

[Christian Persch (GNOME) (away; not receiving bug mail)]

Attachment: updated patch

Comment 8

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

Comment on attachment 288136 [details] [diff] [review]
updated patch

+  nsCOMPtr<nsIMIMEService> mimeSvc (do_GetService (NS_MIMESERVICE_CONTRACTID));

please remove the two spaces here

Comment 9

[Christian Persch (GNOME) (away; not receiving bug mail)]

Attachment: final patch: remove those spaces

Comment 10

[Reed Loden [:reed]]

Comment on attachment 289477 [details] [diff] [review]
final patch: remove those spaces

Fixes several uses of the mime service that were using the wrong contract ID to refer to it.

Comment 11

[Reed Loden [:reed]]

Note that I asked gavin if he thought somebody from toolkit/browser/etc. should review it, and he said that biesi's review should be enough, since the only change is fixing the contract ID.

Comment 12

[Reed Loden [:reed]]

Checking in uriloader/exthandler/nsExternalHelperAppService.cpp;
/cvsroot/mozilla/uriloader/exthandler/nsExternalHelperAppService.cpp,v  <--  nsExternalHelperAppService.cpp
new revision: 1.356; previous revision: 1.355
done
Checking in uriloader/exthandler/nsHandlerService.js;
/cvsroot/mozilla/uriloader/exthandler/nsHandlerService.js,v  <--  nsHandlerService.js
new revision: 1.16; previous revision: 1.15
done
Checking in toolkit/mozapps/downloads/content/downloads.js;
/cvsroot/mozilla/toolkit/mozapps/downloads/content/downloads.js,v  <--  downloads.js
new revision: 1.103; previous revision: 1.102
done
Checking in toolkit/mozapps/downloads/content/helperApps.js;
/cvsroot/mozilla/toolkit/mozapps/downloads/content/helperApps.js,v  <--  helperApps.js
new revision: 1.13; previous revision: 1.12
done
Checking in toolkit/mozapps/downloads/src/nsHelperAppDlg.js.in;
/cvsroot/mozilla/toolkit/mozapps/downloads/src/nsHelperAppDlg.js.in,v  <--  nsHelperAppDlg.js.in
new revision: 1.58; previous revision: 1.57
done
Checking in mail/components/preferences/downloadactions.js;
/cvsroot/mozilla/mail/components/preferences/downloadactions.js,v  <--  downloadactions.js
new revision: 1.6; previous revision: 1.5
done
Checking in uriloader/exthandler/tests/unit/test_handlerService.js;
/cvsroot/mozilla/uriloader/exthandler/tests/unit/test_handlerService.js,v  <--  test_handlerService.js
new revision: 1.14; previous revision: 1.13
done
Checking in browser/components/preferences/applications.js;
/cvsroot/mozilla/browser/components/preferences/applications.js,v  <--  applications.js
new revision: 1.28; previous revision: 1.27
done

Comment 13

[Joe Sabash [:JoeS1]]

Any potential impact on Thunderbird here.
Why I raise the issue, is that I see no effect of mime type on plugin instantiation or calling a helper app in windows TB. Seems the file extension prevails.

Comment 14

[Christian Persch (GNOME) (away; not receiving bug mail)]

(In reply to comment #13)
> Any potential impact on Thunderbird here.
> Why I raise the issue, is that I see no effect of mime type on plugin
> instantiation or calling a helper app in windows TB. Seems the file extension
> prevails.

This patch shouldn't have any effect, since it only changes from which contract ID the MIME service is accessed.