Closed
Bug 223990
Opened 21 years ago
Closed 21 years ago
Should have a getPrimaryExtensionForType function
Categories
(Core Graveyard :: File Handling, defect)
Core Graveyard
File Handling
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla1.6beta
People
(Reporter: Biesinger, Assigned: Biesinger)
Details
(Keywords: memory-footprint)
Attachments
(2 files)
7.83 KB,
patch
|
bzbarsky
:
review+
darin.moz
:
superreview+
|
Details | Diff | Splinter Review |
5.03 KB,
patch
|
bzbarsky
:
review+
darin.moz
:
superreview+
|
Details | Diff | Splinter Review |
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
Assignee | ||
Comment 1•21 years ago
|
||
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
Assignee | ||
Comment 2•21 years ago
|
||
api change, impl new function, fix callers using uppercase functions
Assignee | ||
Comment 3•21 years ago
|
||
make use of the new method
Assignee | ||
Comment 4•21 years ago
|
||
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.
Attachment #134355 -
Flags: review?(bz-vacation)
Assignee | ||
Updated•21 years ago
|
Attachment #134358 -
Flags: review?(bz-vacation)
Assignee | ||
Updated•21 years ago
|
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.6beta
Comment 5•21 years ago
|
||
Do those three callers really justify adding another function to this API? Especially if this function does nothing different under the hood (unlike getTypeFromExtension)?
Assignee | ||
Comment 6•21 years ago
|
||
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•21 years ago
|
||
Ah, ok. Yeah, that last would be worth it. I'll try to look at this soon.
Comment 8•21 years ago
|
||
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.
Attachment #134355 -
Flags: review?(bz-vacation) → review+
Comment 9•21 years ago
|
||
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).
Attachment #134358 -
Flags: review?(bz-vacation) → review+
Assignee | ||
Updated•21 years ago
|
Attachment #134355 -
Flags: superreview?(darin)
Assignee | ||
Comment 10•21 years ago
|
||
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
Attachment #134358 -
Flags: superreview?(darin)
Updated•21 years ago
|
Attachment #134355 -
Flags: superreview?(darin) → superreview+
Updated•21 years ago
|
Attachment #134358 -
Flags: superreview?(darin) → superreview+
Comment 11•21 years ago
|
||
Yeah, hence the sudden flurry of reviews... ;)
Assignee | ||
Comment 12•21 years ago
|
||
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
Status: ASSIGNED → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 13•21 years ago
|
||
turns out I missed a few callers that could make use of this function. I filed bug 225807 for them
Comment 14•21 years ago
|
||
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.
Assignee | ||
Comment 15•21 years ago
|
||
>This regressed bug 225958.
doesn't really surprise me.
Updated•8 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•