292469 - Can't run XULRunner applications from a path with spaces in it

Status: RESOLVED FIXED

(Toolkit Graveyard :: XULRunner, defect)

Description

[Frank Wein [:mcsmurf]]

I wanted to start a XULRunner app from scratch, the path to the XULRunner app
had spaces in it, so i used something like this to start it (WinDbg confirms this):
D:\mozilla\tree-main\mozilla\obj-xul\dist\bin\xulrunner.exe -app "D:\my
app\application.ini"
First it finds the app, but after the internal restart of xulrunner it just dumps
"Error: Invalid or missing application data!"
to console. WinDbg told me that XULRunner restarts itself like this:
D:\mozilla\TREE-M~1\mozilla\obj-xul\dist\bin\XULRUN~1.exe -app D:\my
app\application.ini
Note the missing quotes.
If i first start the app from a path without spaces and then move the app to a
path with spaces, everything is fine (i guess because then a profile has already
been created and there is no need to restart XULRunner internally).

Comment 1

[Darin Fisher]

It seems like this might be an issue for Firefox as well.  Try passing a file
path to Firefox on the command line.  It should suffer the same problem.

  mkdir test
  .\firefox.exe -profile test "c:\my test\file.html"

In firefox with a new profile, I get two tabs: one tries to load "c:\my" and the
other tries to load "test/file.html" as a URL relative to the current directory.

Comment 2

[Darin Fisher]

This is likely to be less noticed in Firefox since people rarely pass URLs to
Firefox on the command line, and they even more rarely do so when Firefox needs
to restart itself.

-> me

Comment 3

[Frank Wein [:mcsmurf]]

Confirming, i also see that problem with Firefox. Also i get another strange
problem with FF trunk here, it alerts that D:/my/ cannot be found, if you OK
that you dont get any window, but firefox.exe keeps running in background. I'll
put that issue to another bug.

Comment 4

[Darin Fisher]

When creating gRestartArgv, I think we probably want to add quotes around
elements that have spaces in them.  It looks like the MS CRT code that generates
__argv and __argc will strip the quotes (see parse_cmdline in stdargv.c).  This
may be the right thing to do for argv[0] instead of what is done here:
http://lxr.mozilla.org/seamonkey/source/toolkit/xre/nsAppRunner.cpp#974

Comment 5

[Darin Fisher]

Frank: if the patch for this bug fixes that other problem with firefox, then
maybe no need for another bug.

Comment 6

[Frank Wein [:mcsmurf]]

(In reply to comment #5)
> Frank: if the patch for this bug fixes that other problem with firefox, then
> maybe no need for another bug.

That bug also happens if you pass something like D:\nofilehere. But i just saw,
the FF window launches some seconds later (so some delay)...

Comment 7

[Darin Fisher]

Attachment: v1 patch

This seems to do the trick.

Comment 8

[Darin Fisher]

BTW, I decided not to bother with changing the code that makes exePath use the
short path form.  I figure that that code works so why tempt fate.

Comment 9

[Benjamin Smedberg]

Comment on attachment 182289 [details] [diff] [review]
v1 patch

I'm really confused as to why this is necessary. If we are relaunching with a
remotely unix-like execv, we shouldn't need the quotes, because we aren't being
passed through the shell escaping mechanisms.

Comment 10

[Darin Fisher]

Bsmedberg:

The problem is that microsoft's execv seems to assume that arguments do not have
spaces in them.  It's a really broken assumption.  Just take a look at the
implementation of execv when you get a chance (included with any copy of visual
studio).  It just concatenates the arguments together before calling
CreateProcess!  So, without quoting, there is no way to discern argument boundaries.

Fortunately, when constructing __argv, the MS CRT strips double quotes.  So, we
can double quote any parameters passed to execv if we want them to appear as
single entries in __argv.

Comment 11

[Benjamin Smedberg]

Comment on attachment 182289 [details] [diff] [review]
v1 patch

Wow, ok. The windows POSIX subsystem isn't as capable as I had thought. Please
add a comment to this hack to indicate why it's necessary, and reference this
bug. Also, we should check with mkaply, since this same hack might be necessary
for OS/2.

Comment 12

[Darin Fisher]

Comment on attachment 182289 [details] [diff] [review]
v1 patch

This patch affects Firefox too, but shows up more easily under XULRunner.

Comment 13

[chris hofmann]

Comment on attachment 182289 [details] [diff] [review]
v1 patch

a=chofmann

Comment 14

[Jens Bannmann]

Checked in by darin at 2005-05-17 14:55, resolving.

Comment 15

[Frank Wein [:mcsmurf]]

BTW: I asked mkaply about OS/2, he said everything is ok there (it only opens
one tab with c:\my test\file.html in it.