Closed
Bug 634942
Opened 13 years ago
Closed 13 years ago
HelperApp launch is broken with GIO
Categories
(Core Graveyard :: File Handling, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla2.0
People
(Reporter: wolfiR, Unassigned)
Details
Attachments
(2 files)
2.42 KB,
patch
|
karlt
:
review+
beltzner
:
approval2.0+
|
Details | Diff | Splinter Review |
2.58 KB,
patch
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Comment 1•13 years ago
|
||
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.
Reporter | ||
Comment 2•13 years ago
|
||
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 3•13 years ago
|
||
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+
Reporter | ||
Comment 4•13 years ago
|
||
same with two NS_ENSURE_SUCCESS checks
Reporter | ||
Comment 5•13 years ago
|
||
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 6•13 years ago
|
||
Comment on attachment 513194 [details] [diff] [review] possible patch a=beltzner
Attachment #513194 -
Flags: approval2.0? → approval2.0+
Reporter | ||
Comment 7•13 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/372eca6704c2
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0
Assignee | ||
Updated•8 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•