Regression: Application Launch dialog is empty on Linux

VERIFIED FIXED in Firefox 31

Status

Core Graveyard
File Handling
VERIFIED FIXED
4 years ago
2 years ago

People

(Reporter: Mathieu Comandon, Assigned: Ritesh Khadgaray)

Tracking

({regression})

Trunk
mozilla31
x86
Linux
regression

Firefox Tracking Flags

(firefox31 verified)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

4 years ago
User Agent: Mozilla/5.0 (X11; Linux i686) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/29.0.1547.65 Safari/537.36

Steps to reproduce:

Browsing on websites with a custom URL scheme which should open an external application when clicked (such as apt://, steam:// or magnet:// links).


Actual results:

Clicking on such links opens an empty dialog box and the user has to browse the file system to look for the executable, no default choice is proposed (since Firefox 22)


Expected results:

The Application Launch dialog used to show default application that could handle specific types of url schemes (Transmission for magnet: links, software center for apt: link, ...).

I ran a git bisect of revisions between Firefox 21 and Firefox 22 and found out that this was caused by the commit fixing bug #444440 (https://hg.mozilla.org/mozilla-central/rev/f118eb2326f7), reverting the commit did put the expected applications back in the dialog box, but doing so would reopen bug 444440. As I'm not familiar with the Firefox codebase, I didn't go as far as trying to write a patch that would solve both bugs, but ideally that's what would be needed here.
(Reporter)

Updated

4 years ago
Keywords: regression
Summary: Application Launch dialog is empty on Linux → Regression: Application Launch dialog is empty on Linux
(Reporter)

Updated

4 years ago
Blocks: 444440

Updated

4 years ago
Mike?
Flags: needinfo?(mh+mozilla)

Updated

4 years ago
Component: Untriaged → File Handling
Product: Firefox → Core
(Assignee)

Comment 2

4 years ago
This seems to be a bug with gio handler

                                        
NS_IMETHODIMP                                                           
nsMIMEInfoUnix::GetHasDefaultHandler(bool *_retval)                     
{                                                                       
...
  
  *_retval = false;
  nsRefPtr<nsMIMEInfoBase> mimeInfo = nsGNOMERegistry::GetFromType(mSchemeOrType);
<--- we check for mime type, and not for protocol handler. The older code always would fall back to protocol handler based of mailcap.

  We need to fix GetFromType(mSchemeOrType), to handle scheme ( protocol). 


This is seen from here 


NS_IMETHODIMP
nsGIOService::GetAppForMimeType(const nsACString& aMimeType,
                                nsIGIOMimeApp**   aApp)
{
...
  GAppInfo *app_info = g_app_info_get_default_for_type(content_type, false);
...
<-- We probably need to add g_app_info_get_default_for_uri_scheme () , if app_info is null. 

untested though .
Ritesh, do you want to test your idea? https://developer.mozilla.org/en/Simple_Firefox_build should get you started, if you don't already have a local build.
Flags: needinfo?(khadgaray)
(Assignee)

Comment 4

4 years ago
This indeed is the issue. Initial "patch" . Will post the final patch in a while ( rebuilding is a sloow process ).


--- a/uriloader/exthandler/unix/nsGNOMERegistry.cpp     2014-02-24 23:38:31.303287298 +0530
+++ b/uriloader/exthandler/unix/nsGNOMERegistry.cpp     2014-02-25 00:04:12.643350922 +0530
@@ -148,7 +148,12 @@ nsGNOMERegistry::GetFromType(const nsACS
     nsCOMPtr<nsIGIOMimeApp> gioHandlerApp;
     if (NS_FAILED(giovfs->GetAppForMimeType(aMIMEType, getter_AddRefs(gioHandlerApp))) ||
         !gioHandlerApp) {
-      return nullptr;
+        // Unable to find mime handler, probably a protocol. moz bz#947868
+        // Need a better way to identify if this is a scheme - check for nsMIMEInfoBase::eProtocolInfo ?
+        if (NS_FAILED(giovfs->GetAppForURIScheme(aMIMEType, getter_AddRefs(gioHandlerApp))) ||
+            !gioHandlerApp) {
+            return nullptr;
+        }
     }
     gioHandlerApp->GetName(name);
     giovfs->GetDescriptionForMimeType(aMIMEType, description);
Flags: needinfo?(khadgaray)
(Assignee)

Comment 5

4 years ago
Created attachment 8381136 [details] [diff] [review]
proposed patch

nsGNOMERegistry::GetFromType only understands with mime detection, and not scheme  handler detection. 

Check if we are handling a scheme, if yes, call nsGNOMERegistry::HandlerExists

Updated

4 years ago
(Assignee)

Comment 6

4 years ago
Created attachment 8381157 [details] [diff] [review]
protocol.patch
Attachment #8381136 - Attachment is obsolete: true
Comment on attachment 8381157 [details] [diff] [review]
protocol.patch

Thanks Ritesh! hg tells me that Karl has reviewed changes to this file in the past, so let's see what he thinks.
Attachment #8381157 - Flags: review?(karlt)
Comment on attachment 8381157 [details] [diff] [review]
protocol.patch

Looks good, thanks.  Can you adjust the spacing a bit to match the surrounding code, as noted below, please?

>+
>+

Only one blank line, please.

>+  if ( mClass ==  eProtocolInfo ) {

No space after '(' or before ')'.
Only one space after "==".

>+    *_retval = nsGNOMERegistry::HandlerExists (mSchemeOrType.get());

No space before '('.
Attachment #8381157 - Flags: review?(karlt) → review+
(Assignee)

Comment 9

4 years ago
Created attachment 8383597 [details] [diff] [review]
protocol.patch

fixed.
Attachment #8381157 - Attachment is obsolete: true

Updated

4 years ago
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/e1450dd6a960

Thanks for the patch, Ritesh! One small request - for future patches, please make sure that the commit information is included when you request checkin. Makes life easier for those landing on your behalf. Thanks again!
https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F
Assignee: nobody → khadgaray
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/e1450dd6a960
Status: UNCONFIRMED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
Flags: needinfo?(mh+mozilla)
status-firefox31: --- → fixed
Keywords: verifyme
Verified as fixed using the following environment:

FF 31.0b6
Build Id:20140630185627
OS: Ubuntu 14.04 x32
Status: RESOLVED → VERIFIED
status-firefox31: fixed → verified
Keywords: verifyme

Comment 13

4 years ago
Landed in Xubuntu 14.04, thanks !

Now if you guys would like to have a look at the related painful bug 397700, so that Firefox finally wholly enters the 21st century on Linux... :)
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.