223990 - Should have a getPrimaryExtensionForType function

Status: RESOLVED FIXED

(Core Graveyard :: File Handling, defect)

Description

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

the pattern of getting a mimeinfo just to get to the primary extension occurs
several times in mozilla code
it would be good if a fast way for that would be provided, like for
extension->type (GetTypeFromExtension)

like this:
    /** Returns the primary extension for a given mimetype/extension combination.
     * The returned extension may be identical to the passed in extension */
    wstring getPrimaryExtensionForType(in string aMIMEType, in wstring aExtension);

alternatively I can use wstring if people would prefer that

Comment 1

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

also, while I'm touching this interface anyway, the methods should be changed to
start with a lowercase letter per the convention for idl method names

Comment 2

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

Attachment: patch part 1

api change, impl new function, fix callers using uppercase functions

Comment 3

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

Attachment: patch part 2

make use of the new method

Comment 4

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

Comment on attachment 134355 [details] [diff] [review]
patch part 1

I noticed that GetFromTypeAndExtension still has an uppercase start letter, but
the callers were changed to use a lowercase one... I changed this locally to
have a lowercase start letter always.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Do those three callers really justify adding another function to this API? 
Especially if this function does nothing different under the hood (unlike
getTypeFromExtension)?

Comment 6

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

well, it would be a convenience function mostly. Also, I was thinking to maybe
implement a faster version of it in the oshelperappservices to not have to
create a nsIMIMEInfo for this and fill it with all the stuff that's not needed
in this case.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Ah, ok.  Yeah, that last would be worth it.  I'll try to look at this soon.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 134355 [details] [diff] [review]
patch part 1

>+    /**
>+     * Given a Type/Extension combination, returns the default extension
>+     * for this type. This may be identical to the passed-in extension.
>+     *
>+     * @param aMIMEType The Type to get information on. Must not be null.

Add an NS_ENSURE_ARG_POINTER to that effect in the impl, and r=bzbarsky.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 134358 [details] [diff] [review]
patch part 2

r=bzbarsky.  Gotta love that leak you fixed in mimemoz2.cpp....

Let me know if you need help driving these patches in (e.g. if you won't have
time before the freeze on Tuesday night).

Comment 10

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

Comment on attachment 134358 [details] [diff] [review]
patch part 2

that's tuesday already?
ok... I'll send you mail if I don't have time to check this in myself

Comment 11

[Boris Zbarsky [:bzbarsky]]

Yeah, hence the sudden flurry of reviews... ;)

Comment 12

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

ok, as the sr happened so fast (thanks!) I was able to check this in today.

FIXED, filed Bug 225618 to implement this more efficiently

Comment 13

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

turns out I missed a few callers that could make use of this function.
I filed bug 225807 for them

Comment 14

[Steffen Wilberg]

This regressed bug 225958.
Well, renaming GetFromTypeAndExtension to getFromTypeAndExtension in
toolkit/mozapps/downloads/content/helperApps.js and
browser/components/prefwindow/content/plugins.js was easy, but Firebird's
plugins window (Tools-Options-Downloads-plugins) is still empty. Now the js
console shows an error upon opening it, see bug 225958 comment 1.
Please help.

Comment 15

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

>This regressed bug 225958.

doesn't really surprise me.