Comment 36 for bug 589236

Revision history for this message
In , Mozilla (mozilla) wrote :

(In reply to comment #32)
> What's the point of the CopyBasicDataTo change? The idea of that function is
> to copy whatever information you have, no? The header certainly documents it
> as copying that info. Look up why that was done originally? In any case, in
> your situation I can't see why removing this line is needed.

The latest patch should explain why it's needed but I'm doing it differently now and not touching the cross platform method at all.
In written words: First we are getting a mimeinfo by type with no "default description" and because of that we try using the extension and get a valid result containing a default description. But afterwards we are overwriting the by-ext-mimeinfo with "basic" data in CopyBasicDataTo() including the empty default description. The new version of the patch sets the default description we just got also in the by-type-mimeinfo to preserve it after the copy.

> I'm not happy with the random object-creation stuff going on here. Why not
> change nsGNOMERegistry to expose functions to get a helper by type or extension
> and use them both in your new code and in the existing nsGNOMERegistry code?

I've read this a few times but I don't get what you are proposing in the end.
There are functions to get helpers by type or extension which are used.
Currently I have only changed the obvious code to use nsGNOMERegistry but also kept one use of nsIGnomeVFSService in LaunchDefaultWithFile(). Is that what you mean what should be changed?

> Always setting the extension on the MIME info bothers me, though I can't figure
> out why. This is a situation where we got a MIME info for the MIME type,
> right? And the only issue was that this MIME info didn't have a default
> handler, which is why we fell back on getting one by extension? I guess we
> need to set the extension there to make the whole "get default app" thing work
> later...

My intention was to get a "valid" mimeinfo which "fits" the actual file.

> Could we fix that by actually storing the default app in the MIMEInfo at
> construction time instead of messing around with hitting the GNOME registry
> every single time someone asks something of the MIME info? Or is there a major
> problem with that approach? I'm not sure why it wasn't taken here to start
> with.

I don't really understand what you mean. My intention was to fix that bug (hopefully without introducing new ones) w/o much refactoring of the code. I'm happy to try to improve it further but I'm not that familiar with all of that code.