97333 - nsFilePicker should use IC service not direct calls

Status: RESOLVED FIXED

(Core :: XUL, defect, P1)

Description

[Mike Pinkerton (not reading bugmail)]

nsFilePicker directly calls IC to do file mappings. Instead, it should be using 
our IC service.

Comment 1

[Conrad Carlen (not reading bugmail)]

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

[Simon Fraser [no longer active]]

Sounds like the IC service needs to move to embedcomponents, or somesuch.

Comment 3

[Mike Pinkerton (not reading bugmail)]

Attachment: remove direct IC calls

Comment 4

[Mike Pinkerton (not reading bugmail)]

needing r/sr. cc'ing dagley since he's familiar w/ the code.

Comment 5

[Conrad Carlen (not reading bugmail)]

+ 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.

Comment 6

[Mike Pinkerton (not reading bugmail)]

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

Comment 7

[Simon Fraser [no longer active]]

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

Comment 8

[Mike Pinkerton (not reading bugmail)]

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

[Simon Fraser [no longer active]]

I'm convinced. sr=sfraser

Comment 10

[Mike Pinkerton (not reading bugmail)]

landed on trunk