Rewrite OS/2 helper application code

VERIFIED FIXED

Status

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

People

(Reporter: mkaply, Assigned: mkaply)

Tracking

({qawanted})

Trunk
x86
OS/2
qawanted

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

16 years ago
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.
(Assignee)

Comment 1

16 years ago
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)
(Assignee)

Comment 2

16 years ago
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.
Keywords: qawanted
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.  ;)
(Assignee)

Comment 4

16 years ago
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?
(Assignee)

Comment 6

16 years ago
> I'll likely be making some improvements to this code this summer...
> I'll keep you posted, ok?

That would be great. Thanks!
(Assignee)

Comment 7

16 years ago
*** Bug 144323 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 8

16 years ago
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+
(Assignee)

Comment 10

16 years ago
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+
(Assignee)

Comment 11

16 years ago
Checked into trunk and branch
a=chofmann
Status: NEW → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
(Assignee)

Comment 12

16 years ago
Mass verify. These are fixed.
Status: RESOLVED → VERIFIED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.