Closed Bug 453689 Opened 12 years ago Closed 11 years ago

Firefox needs to register the proper name with session management for restart

Categories

(Toolkit :: Startup and Profile System, defect)

x86
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla1.9.2a1
Tracking Status
status1.9.1 --- .2-fixed

People

(Reporter: orion, Assigned: stransky)

References

(Blocks 1 open bug)

Details

(Keywords: verified1.9.1)

Attachments

(3 files, 2 obsolete files)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.16) Gecko/20080715 Fedora/2.0.0.16-1.fc8 Firefox/2.0.0.16
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.16) Gecko/20080715 Fedora/2.0.0.16-1.fc8 Firefox/2.0.0.16

Firefox currently registers the full path to the binary executable with Unix session management.  This causes problems because this skips running the usual shell wrapper script.  I think Firefox should register "firefox" with session management and let it be found in the search path.

Reproducible: Always

Steps to Reproduce:
1.
2.
3.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: wanted1.9.0.x?
Flags: wanted-firefox3.1?
Flags: blocking-firefox3.1?
Doesn't really meet the "wanted" criteria (security, stability, regression from maintenance release) for 1.9.0.x. However, we'll look at a reviewed and baked patch.
Flags: wanted1.9.0.x? → wanted1.9.0.x-
Originates from bug #262258#c40
What about this solution? MOZILLA_APP_LAUNCHER env variable (or something) gives an opportunity to set different application launcher (e.g. in some system-wide start up script).
I just tripped over this in Fedora 10. Any chance of a fix or workaround anytime soon?
Flags: blocking-firefox3.1? → blocking-firefox3.1-
I guess this would fix my problems on debian lenny, too.
And the same problem exists with icedove (thunderbird) - maybe a core problem?
Component: Shell Integration → Startup and Profile System
Flags: wanted-firefox3.1?
Flags: blocking-firefox3.1-
Product: Firefox → Toolkit
QA Contact: shell.integration → startup
Version: unspecified → Trunk
Blocks: 233462
The patch attached needs to be reviewed for inclusion to the the tree. Please ask for review from one of the toolkit developers http://www.mozilla.org/projects/toolkit/review.html 

https://developer.mozilla.org/en/Getting_your_patch_in_the_tree
Attachment #352541 - Flags: review?(benjamin)
Comment on attachment 352541 [details] [diff] [review]
Env variable can suppress the default mozilla binary

Benjamin, can you please check this one?
Comment on attachment 352541 [details] [diff] [review]
Env variable can suppress the default mozilla binary

>diff -U10 -up src/toolkit/xre/nsNativeAppSupportUnix.cpp.old src/toolkit/xre/nsNativeAppSupportUnix.cpp

>+  #define ARGC	1
>+  char* argv[ARGC];

The array is unnecessary (and the #define is ugly!). You should be able to do:

char *argv1 = nsnull;
And then just pass &argv1 whenever you need an argv.
Attachment #352541 - Flags: review?(benjamin) → review-
Thanks for looking at it!

There's an updated one there, it fixes one more problem in the original patch (gnome_client_set_restart_command() was executed only when MOZILLA_APP_LAUNCHER was not set).
Attachment #352541 - Attachment is obsolete: true
Attachment #375966 - Flags: review?(benjamin)
Attachment #375966 - Flags: review?(benjamin) → review+
Comment on attachment 375966 [details] [diff] [review]
v2
[Checkin: Comment 13]

Thanks! Is sr+ requested here?
Set checkin-needed, regards to https://developer.mozilla.org/en/Getting_your_patch_in_the_tree
sr+ is not requested here (no cross-module, API change or security).
Flags: wanted1.9.1?
Flags: wanted1.9.0.x?
Flags: wanted1.9.0.x-
Keywords: checkin-needed
Assignee: nobody → stransky
http://hg.mozilla.org/mozilla-central/rev/68e92b917783

Thanks, sorry about the delay.
Status: NEW → RESOLVED
Closed: 11 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Would be nice to get this into 1.9.1 at some point?
And one nit even if it is too late:
"MOZILLA_APP_LAUNCHER" almost every other env variable which is used within mozilla is just prefixed with "MOZ_". That looks quite inconsistent ;-)
You're right, here is the change.
Attachment #387405 - Flags: review?(benjamin)
Comment on attachment 387405 [details] [diff] [review]
change "MOZILLA_APP_LAUNCHER" -> "MOZ_APP_LAUNCHER"

Benjamin, do you agree with this change?
Attachment #387405 - Flags: review?(benjamin) → review+
Attachment #375966 - Attachment description: v2 → v2 [Checkin: Comment 13]
Attachment #375966 - Flags: approval1.9.1.2?
Comment on attachment 387405 [details] [diff] [review]
change "MOZILLA_APP_LAUNCHER" -> "MOZ_APP_LAUNCHER"


Please, attach an hg patch.
Flags: wanted1.9.0.x?
Comment on attachment 375966 [details] [diff] [review]
v2
[Checkin: Comment 13]

Can you explain why this is wanted for 1.9.1? What benefits does it have? Please renominate the appropriate patch (this isn't it) for 1.9.1 along with a detailed message of what this does and why we want it and we'll reconsider.
Attachment #375966 - Flags: approval1.9.1.2?
Comment on attachment 389628 [details] [diff] [review]
complete 1.9.1 hg patch
[Checkin: Comment 24]

This one merges the two trunk patches into one, gives us ability to register one, system wide firefox launcher by MOZ_APP_LAUNCHER variable (like /usr/bin/firefox). 

It's critical for us because the launcher script sets up language packs, plug-ins directories and so on. 

Without the patch, firefox launched by gnome session (after user login or session restoration) is broken (it launches the binary from default install dir, like /usr/lib(64)/firefox-XXX/firefox).
Attachment #389628 - Flags: approval1.9.1.2?
Comment on attachment 389628 [details] [diff] [review]
complete 1.9.1 hg patch
[Checkin: Comment 24]

Approved for 1.9.1.2. a=ss for release-drivers

Please land on mozilla-1.9.1 and use the ".2-fixed" option of the "status1.9.1" flag.
Attachment #389628 - Flags: approval1.9.1.2? → approval1.9.1.2+
Flags: wanted1.9.1?
Target Milestone: --- → mozilla1.9.2a1
Attachment #389624 - Attachment description: hg patch → hg patch [Checkin: Comment 21]
Attachment #389628 - Attachment description: complete 1.9.1 hg patch → complete 1.9.1 hg patch [Checkin: Comment 24]
Attachment #387405 - Attachment is obsolete: true
Martin, could you help us verify this in the 3.5.2 (build1) candidates?
Something is wrong probably.
My testing wasn't successful on 3.5.2.

In my firefox startscript I did this before starting Firefox:

export MOZ_APP_LAUNCHER="fasel"

After starting Firefox and saving my session before quitting my WM it hasn't taken this command for the session information.
I also tried that patch with Thunderbird 3.0b and it's also not working there.
None of my tests had a real command saved in the session information.
I'm not sure if the callback is even working correctly currently in general?
When I tested the patch I run it as 

export MOZ_APP_LAUNCHER="/usr/bin/firefox"

because /usr/bin/firefox is the firefox system launcher in Fedora. And it worked fine (it was Fedora 10) but i can check it on some recent system.
Ok, I did a few more testing.
Some info about my environment: 
- Firefox is started through /usr/bin/firefox
- it's a shell script which starts /usr/lib64/firefox/firefox (which is actually  
  the xulrunner stub as Firefox is xulrunner based here)

Regardless if I set something for MOZ_APP_LAUNCHER or not the saved restart command is always "firefox".
Looking at http://mxr.mozilla.org/mozilla1.9.1/source/toolkit/xre/nsNativeAppSupportUnix.cpp#128 that should never happen at all as either the provided MOZ_APP_LAUNCHER definition is used or the full path to the called executable which would be /usr/lib64/firefox/firefox and not just firefox.
So my guess is that gnome_client_set_restart_command() is never called at all.
From just reading the patch that seems unlikely as long as the callback works in general.
It works as expected, I've tested it on F11/latest 1.9.1. The steps are:

1) export the right launcher (export MOZ_APP_LAUNCHER="/home/komat/tmp516/191src/dist/bin/firefox" in my case)
2) Set up gnome session management (to remember application after logout)
3) Logout
4) Login
5) Firefox is launched right after login by gnome session management, with application set by MOZ_APP_LAUNCHER.

Note: If you're interested in proper firefox restart command (because of new extension installation and so on), there's a different bug for it.
Thanks for help, Martin. Adding verified1.9.1 keyword per comment #30.
Keywords: verified1.9.1
You need to log in before you can comment on or make changes to this bug.