294683 - xulrunner should use env var to remember path to appdata file

Status: RESOLVED FIXED

(Toolkit Graveyard :: XULRunner, defect)

Description

[Darin Fisher]

xulrunner should use env var to remember path to appdata file

right now, we augment the command line with a "-app" command line flag, but this
might interfere with other command line handling done by the application.  it'd
be better to strip the path-to-appdata-file command line argument and stash it
in an env var for use when we need to restart the application.

Comment 1

[Benjamin Smedberg]

Attachment: Add XUL_APP-FILE envvar, rev. 1

Comment 2

[Darin Fisher]

Comment on attachment 184510 [details] [diff] [review]
Add XUL_APP-FILE envvar, rev. 1

>Index: toolkit/xre/nsAppRunner.cpp

>+        PR_SetEnv("XUL_APP_FILE=");

nit: why bother clearing this env var?	it seems that nsAppRunner.cpp
should not know about details of nsXULRunnerApp.cpp.


>Index: xulrunner/app/nsXULRunnerApp.cpp

>+  const char *appDataFile = PR_GetEnv("XUL_APP_FILE");
> 
>+  if (! (appDataFile && *appDataFile)) {

nit: kill the space after the "!"


>+    if (IsArg(argv[1], "app")) {
>+      if (argc == 2) {
>+        Output(PR_TRUE, "Error: APP-FILE must be specified!\n");

nit: maybe this should be Usage() ?


r=darin

Comment 3

[Benjamin Smedberg]

> nit: why bother clearing this env var?	it seems that nsAppRunner.cpp
> should not know about details of nsXULRunnerApp.cpp.

We need to clear the envvar for the same reason we need to clear the other
"restart variables", because otherwise one xulapp cannot launch another one (the
envvar would override. I thought about adding a nsXULAppAPI hook for "clearing
stuff after we've restarted" or something like that, but it sounded like overkill.

Comment 4

[Benjamin Smedberg]

Comment on attachment 184510 [details] [diff] [review]
Add XUL_APP-FILE envvar, rev. 1

xulrunner only fixup.

Comment 5

[Darin Fisher]

Agreed on clearing the env var.  Thanks for reminding me of that reason.

Comment 6

[Asa Dotzler [:asa]]

Comment on attachment 184510 [details] [diff] [review]
Add XUL_APP-FILE envvar, rev. 1

a=asa

Comment 7

[Benjamin Smedberg]

fixed on trunk