Comment 21 for bug 589236

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

Created an attachment (id=334557)
patch #2

> I'm not sure I follow this patch. Apart from crashing when miByExt is null,
> what happens? Currently, if miByExt has a default handler there are two
> options:

oops, sorry for the crash. Should be fixed now.
I think this patch works but probably you see a better way to fix it.

> 1) Have a retval. In this case, we CopyBasicDataTo and return miByExt.
> 2) No retval. In this case we're hitting the SetMIMEType code already.
>
> So the patch only affects behavior in the case when we have both retval and
> miByExt. But all it changes is to call SetMIMEType instead of
> CopyBasicDataTo,
>
> Is CopyBasicDataTo clobbering the default app somehow in this case?

Not the default app but ...

Let me explain what currently happens using the special case this bug is about:

- type is application/octet-stream
- ext is for example .pdf
- nsMIMEInfoBase* retval = GetFromType(PromiseFlatCString(aType)).get();
  succeeds but has no default handler
- nsRefPtr<nsMIMEInfoBase> miByExt =
  GetFromExtension(PromiseFlatCString(aFileExt));
  succeeds and returns a valid mimeinfo containing even a default handler
-> now we call retval->CopyBasicDataTo(miByExt);
->
so it seems we end up with a mimeinfo which actually contains a default handler
but again uses type application/octet-stream.

That also means that every call to GetHasDefaultHandler() will return false again since it only checks for a handler for the type again.

Later in nsHelperAppDlg.js we handle application/octet-stream filetypes which have no default handler so that no helper apps are offered at all.