Closed Bug 634942 Opened 11 years ago Closed 11 years ago
App launch is broken with GIO
If GIO (instead of GnomeVFS) is used for HelperApp handling the Launch() method fails to do the right thing. https://mxr.mozilla.org/mozilla-central/source/toolkit/system/gnome/nsGIOService.cpp#135 this method is using g_app_info_launch_uris() which expects a list of URIs (not files). But https://mxr.mozilla.org/mozilla-central/source/uriloader/exthandler/unix/nsMIMEInfoUnix.cpp#137 simply calls the method with a string containing the native local path. The matching GnomeVFS method converts the path to a uri apparently: https://mxr.mozilla.org/mozilla-central/source/toolkit/system/gnome/nsGnomeVFSService.cpp#105 I haven't found a fitting gio function to do a similar thing. I have a candidate patch but it's currently changing the caller instead of the callee. I'm not sure what's the correct way to fix it. The interface description suggests it should be called via uri and not file path but then it has been wrong for years because the callee fixed it silently.
This one fixes the caller as described and it isn't very intrusive leaving the old non-gio code alone. As I said I'm unsure on which side to fix it in the end.
Comment on attachment 513194 [details] [diff] [review] possible patch Karl, asking for review but as written I'm not sure which side to fix.
Attachment #513194 - Flags: review?(karlt)
Comment on attachment 513194 [details] [diff] [review] possible patch This makes sense to me. >+ nsCOMPtr<nsIIOService> ioservice = do_GetService(NS_IOSERVICE_CONTRACTID); >+ nsCOMPtr<nsIURI> uri; >+ ioservice->NewFileURI(aFile, getter_AddRefs(uri)); Should null check ioservice, I think. >+ uri->GetSpec(uriSpec); I saw another NewFileURI use where uri was not null-checked, so I don't know whether a null check on uri is necessary or not.
Attachment #513194 - Flags: review?(karlt) → review+
same with two NS_ENSURE_SUCCESS checks
Comment on attachment 513194 [details] [diff] [review] possible patch That one is basically NPOTB (unless --enable-gio is used) and the original code just does not work. Not sure if I need approval in that case?
Attachment #513194 - Flags: approval2.0?
Comment on attachment 513194 [details] [diff] [review] possible patch a=beltzner
Attachment #513194 - Flags: approval2.0? → approval2.0+
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0
You need to log in before you can comment on or make changes to this bug.