> 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.
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 PromiseFlatCStr ing(aType) ).get() ; nsMIMEInfoBase> miByExt = ion(PromiseFlat CString( aFileExt) ); >CopyBasicDataT o(miByExt) ; octet-stream.
- ext is for example .pdf
- nsMIMEInfoBase* retval = GetFromType(
succeeds but has no default handler
- nsRefPtr<
GetFromExtens
succeeds and returns a valid mimeinfo containing even a default handler
-> now we call retval-
->
so it seems we end up with a mimeinfo which actually contains a default handler
but again uses type application/
That also means that every call to GetHasDefaultHa ndler() 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.