The OS/2 helper app code sucks. On OS/2, we have no place to store global mime info. I'm going to pretty much take the Unix code lock stock and barrel (thanks to bzbarsky for that rewrite) The only Os/2 specific stuff is when we Open an App with a temp file, and the protocol handler stuff.
Created attachment 81877 [details] New h and C files I've attached the new C and header file, since the diff isn't really manageable. The only functions that are different between OS/2 and Unix are: NS_IMETHODIMP nsOSHelperAppService::LaunchAppWithTempFile(nsIMIMEInfo * aMIMEInfo, nsIFile * aTempFile) NS_IMETHODIMP nsOSHelperAppService::ExternalProtocolHandlerExists(const char * aProtocolScheme, PRBool * aHandlerExists)
And nsresult nsOSHelperAppService::GetFileTokenForPath(const PRUnichar * platformAppPath, nsIFile ** aFile) I took the Windows code for this because I had no idea why the Unix stuff was so complicated.
QA Contact: sairuh → nobody
GetFileTokenForPath() is so complicated on Unix so it can return a useful nsIFile when the path is "telnet" (as in, it supports not-fully-qualified paths). I see one possible problem with the code you attached... nsOSHelperAppService::LoadUrl sticks all the parameters in a single string and then passes that as the (single) argument to the app when you call Run() (this is the case when |rv = NS_NewLocalFile(applicationName, PR_FALSE, getter_AddRefs(application));| succeeded). I don't know whether this will do the right thing on OS/2; I know that would break on Unix.... Other than that, looks like it should work. ;)
In our code in NSPR to launch a process, we just put it all into a big string anyway - we don't pass the parameters individually. So we should be OK with our file launching code.
Ok, in that case r=bzbarsky on that patch. I'll likely be making some improvements to this code this summer... I'll keep you posted, ok?
> I'll likely be making some improvements to this code this summer... > I'll keep you posted, ok? That would be great. Thanks!
*** Bug 144323 has been marked as a duplicate of this bug. ***
Created attachment 83830 [details] [diff] [review] sync with latest trunk and darin's comments from telnet buig
Attachment #81877 - Attachment is obsolete: true
Comment on attachment 83830 [details] [diff] [review] sync with latest trunk and darin's comments from telnet buig r=bzbarsky; this code is still as ugly as I recall it being.... :)
Attachment #83830 - Flags: review+
Comment on attachment 83830 [details] [diff] [review] sync with latest trunk and darin's comments from telnet buig sr=blizzard (platform specific code)
Attachment #83830 - Flags: superreview+
Checked into trunk and branch a=chofmann
Status: NEW → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
Mass verify. These are fixed.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.