nsFilePicker should use IC service not direct calls

RESOLVED FIXED in mozilla0.9.6

Status

()

Core
XUL
P1
normal
RESOLVED FIXED
17 years ago
17 years ago

People

(Reporter: Mike Pinkerton (not reading bugmail), Assigned: Mike Pinkerton (not reading bugmail))

Tracking

Trunk
mozilla0.9.6
PowerPC
Mac System 8.5
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

17 years ago
nsFilePicker directly calls IC to do file mappings. Instead, it should be using 
our IC service.
(Assignee)

Updated

17 years ago
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9.5
One problem with the IC Service is that it's in appshell which should not be
needed in an embedding context. Since nsLocalFileMac already uses IC Service,
we're deeply dependent on it so this change isn't going to make matters worse.
It would be nice to find a better home for IC Service and we could rid embedding
of another component which goes mostly unused.

Comment 2

17 years ago
Sounds like the IC service needs to move to embedcomponents, or somesuch.
(Assignee)

Updated

17 years ago
Priority: -- → P3
(Assignee)

Comment 3

17 years ago
Created attachment 51872 [details] [diff] [review]
remove direct IC calls
(Assignee)

Comment 4

17 years ago
needing r/sr. cc'ing dagley since he's familiar w/ the code.
+ mFlatFilters.AppendCString ( nsCString((char*)&typeTemp[1]) );

That's constructing another whole nsCString which allocates storage. You could
do better with nsDependentCString

Other than that, looks good. r=ccarlen

This is beside the point of this patch, but I raised the point before: Isn't
there a better place for the IC service to live than in appshell? I really want
to avoid needing that component for embedding. EmbedComponents is getting a
little crowded. If we had an OSServices component, we could put IC in it.
Anything else fit in that category? If so, let's make a new bug and do it.
(Assignee)

Comment 6

17 years ago
conrad & i talked about this, mFlatFilters is an array, not a string, so
creating a new string here is appropriate.

bug 102920 filed on conrad's components question:
http://bugzilla.mozilla.org/show_bug.cgi?id=102920
(Assignee)

Updated

17 years ago
Priority: P3 → P1

Comment 7

17 years ago
I'd like to see some evidence of testing. IC can return empty mime types, and
probably empty type/creators sometimes.
(Assignee)

Comment 8

17 years ago
For unmapped extensions (.shtml is an example), the ICService returns a null
mime info, which we check for. We have the extension in our list (which we use
if there is no matching file type), but we're not putting garbage into our macos
type list. Finally, i stepped through it to make sure the string foo is correct
(don't walk off the end of things, etc).

anything else i should test?

Comment 9

17 years ago
I'm convinced. sr=sfraser
(Assignee)

Updated

17 years ago
Target Milestone: mozilla0.9.5 → mozilla0.9.6
(Assignee)

Comment 10

17 years ago
landed on trunk
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED

Updated

16 years ago
Blocks: 101830
You need to log in before you can comment on or make changes to this bug.