852540 - [Linux] Layout broken for open file dialog - default handler no longer presented

Status: VERIFIED FIXED

(Core Graveyard :: File Handling, defect)

Description

[Virgil Dicu [:virgil] [QA]]

Attachment: Screenshot

Mozilla/5.0 (X11; Linux i686; rv:22.0) Gecko/20130319 Firefox/22.0

STR:
1. Visit http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-trunk/
2. Select any tar file

Actual result: Display broken for the open with entry - see screenshot in ubuntu

Recent regression:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=e7632ab657e5&tochange=b0e08db3bc2a


Last good nightly: 2013-02-27
First bad nightly: 2013-02-28

Used tinderbox builds to narrow down. This leads to bug 444440.
f118eb2326f7 - broken
c87c6b23794e - good

Comment 1

[Mats Palmgren (inactive)]

This seems like a lower level problem that would occur anywhere, not just in
the file handling dialog.  IIRC, the widget theming code should provide a
minimum size - maybe it's wrong somehow?  or XUL layout isn't using it
correctly?  Let's start in widget:gtk for further triage...

Comment 2

[Boris Zbarsky [:bzbarsky]]

Well, so... It seems like there might be a problem where we used to have something in the dropdown but now have nothing?  Virgil, what did it use to say in there?

Comment 3

[Virgil Dicu [:virgil] [QA]]

Attachment: File handling dialog - normal behavior

That would be the Archive Manager.

FTR, this doesn't occur with simple docs (Libre Office is displayed as default)

Comment 4

[Chris Coulson]

In the example given in comment 0, the mimetype is "application/x-bzip2", and so the handler lookup by mimetype in nsOSHelperAppService::GetMIMEInfoFromOS() fails because nothing on the system handles this. It falls back to looking up by extension (bz2), which finds a valid handler. However, the file extension is never stored in the returned nsMimeInfoUnix.

What this means is that nsMIMEInfoUnix::GetHasDefaultHandler() now just falls all the way through to the end without finding a default handler (because the mimetype lookup fails), whereas it used to fall through to nsMIMEInfoImpl::GetHasDefaultHandler() which would say there is a default handler because the description is set (to "Archive Manager").

Comment 5

[Chris Coulson]

Attachment: Store the file extension on the nsMIMEInfoUnix returned from nsGNOMERegistry::GetFromExtension

This unbreaks nsMIMEInfoUnix::GetHasDefaultHandler() by making sure that it can fall back to looking up by file extension

Comment 6

[Karl Tomlinson (:karlt)]

Comment on attachment 727023 [details] [diff] [review]
Store the file extension on the nsMIMEInfoUnix returned from nsGNOMERegistry::GetFromExtension

>@@ -125,17 +125,22 @@ nsGNOMERegistry::GetFromExtension(const 

>     if (NS_FAILED(gnomevfs->GetMimeTypeFromExtension(aFileExt, mimeType)) ||
>         mimeType.EqualsLiteral("application/octet-stream"))
>       return nullptr;
>   }
> 
>-  return GetFromType(mimeType);
>+  nsRefPtr<nsMIMEInfoBase> mi = GetFromType(mimeType);
>+  if (mi) {
>+    mi->AppendExtension(aFileExt);
>+  }
>+
>+  return mi.forget();

I'm not following why this is necessary.

If GetFromType() returns non null, then it has set mSchemeOrType on the result to mimeType.
So when nsMIMEInfoUnix::GetHasDefaultHandler() calls nsGNOMERegistry::GetFromType() again with mSchemeOrType, why is it not still returning non-NULL?

(In reply to Chris Coulson from comment #4)
> In the example given in comment 0, the mimetype is "application/x-bzip2",
> and so the handler lookup by mimetype in
> nsOSHelperAppService::GetMIMEInfoFromOS() fails because nothing on the
> system handles this. It falls back to looking up by extension (bz2), which
> finds a valid handler. However, the file extension is never stored in the
> returned nsMimeInfoUnix.

When it finds a valid handler for the extension, isn't it doing that by finding a mime type for the extension, and then a handler for that mime type?

Comment 7

[Chris Coulson]

(In reply to Karl Tomlinson (:karlt) from comment #6)
> 
> If GetFromType() returns non null, then it has set mSchemeOrType on the
> result to mimeType.
> So when nsMIMEInfoUnix::GetHasDefaultHandler() calls
> nsGNOMERegistry::GetFromType() again with mSchemeOrType, why is it not still
> returning non-NULL?
> 
> When it finds a valid handler for the extension, isn't it doing that by
> finding a mime type for the extension, and then a handler for that mime type?

You're right. There is something else going on here as well. It looks like mSchemeOrType gets set back to the original type just here:

http://hg.mozilla.org/mozilla-central/file/1d6fe70c79c5/uriloader/exthandler/unix/nsOSHelperAppService.cpp#l1515

Comment 8

[Karl Tomlinson (:karlt)]

Comment on attachment 727023 [details] [diff] [review]
Store the file extension on the nsMIMEInfoUnix returned from nsGNOMERegistry::GetFromExtension

(In reply to Chris Coulson from comment #7)
> It looks like mSchemeOrType gets set back to the original type just here:

Ah, OK, thanks.  Setting the extension seems reasonable to me, then, and this seems consistent with what nsOSHelperAppService::GetFromExtension() does with an nsMIMEInfoUnix from a mailcap entry.

I suspect this is a fix for only some of the problems, as CopyBasicDataTo() from GetMIMEInfoFromOS() will overwrite this extension in some cases (bug 327323 comment 36), but should be a step in the right direction.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/626c4a0f2047

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/626c4a0f2047

Comment 11

[Virgil Dicu [:virgil] [QA]]

Mozilla/5.0 (X11; Linux i686; rv:22.0) Gecko/20130322 Firefox/22.0

Looks good.