639090 - Safe Mode dialog doesn't allow the user to restart in normal mode

Status: VERIFIED FIXED

(Toolkit :: Startup and Profile System, defect)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b13pre) Gecko/20110303 Firefox/4.0b13pre ID:20110303122430

Starting Firefox in safe mode, and selecting "Make changes and restart" in the Safe Mode dialog, restarts the browser into the Safe Mode dialog again. There is no way for the user to start Firefox in normal mode again. Instead the Quit button has to be clicked and Firefox manually started.

Steps:
1. Start Firefox 4
2. Select Help | Restart with Add-ons Disabled"
3. In the Safe Mode dialog select to disable all add-ons
4. Click on Make Changes and Restart

After step 4 you will end-up again in step 3. There is no way to leave the Safe Mode dialog to restart in normal mode.

This is a regression from Firefox 3.6. Not sure yet if this has been caused by bug 568691.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2]]

I don't think it's a blocker for Firefox 4 but it should probably fixed in a dot-release. Should we relnote?

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Also the -safe-mode switch on OS X doesn't work anymore after beta 6. Same for the option key. Not sure if both behaviors are related, but with beta 7 we have introduced the new restart in safe mode menu entry under the help menu.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Ok, since we have the menu entry it's not possible at least on OS X to restart in normal mode. The user is stuck in safe mode. The -safe-mode command line option still works on that day. So I think it should be moved to another bug.

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Filed bug 639101 for the -safe-mode option.

Comment 5

[Dave Townsend [:mossop]]

This problem will be unique to the implementation of the menu I think, we set an env variable and unless we unset it we'd continuously restart in safe mode

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Does that mean we never reach that code when Firefox gets restarted via the menu?

http://hg.mozilla.org/mozilla-central/diff/084a262560eb/toolkit/xre/nsAppRunner.cpp

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Updated steps without using the menu in a running Firefox instance:

1. export MOZ_SAFE_MODE_RESTART=1
2. Start Firefox with the -safe-mode option
3. In the Safe Mode dialog select to disable all add-ons
4. Click on Make Changes and Restart

So the issue should be part of the Safe Mode dialog.

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+2]]

This is only reproducible on OS X and Linux, but not Windows. As used in the code "MOZ_SAFE_MODE_RESTART=" doesn't unset the environment variable for me.

Comment 9

[Dave Townsend [:mossop]]

Attachment: patch rev 1

Yeah problem here is that setting "MOZ_SAFE_MODE_RESTART=" doesn't actually unset the variable it just sets it to a blank string, and the test only cares whether the variable is set.

This patch adds a helper for testing whether an environment variable is set with an actual value and then uses it in a couple of place where this is useful, one or two were places where we weren't doing it properly before but with little impact.

I've also made the changes per bug 555894 to a couple of places.

Comment 10

[Benjamin Smedberg]

Comment on attachment 517507 [details] [diff] [review]
patch rev 1

Ugh. I knew this rule, I wonder how I didn't catch it in review :-(

Comment 11

[Dave Townsend [:mossop]]

Landed on trunk: http://hg.mozilla.org/mozilla-central/rev/5f420b83c3fc
Need a litmus test for this since we can't do any automated tests in safe mode.

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Verified fixed with Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:5.0a2) Gecko/20110426 Firefox/5.0a2

The manual test is covered by the following Litmus test:
https://litmus.mozilla.org/show_test.cgi?id=15171

Comment 13

[Ginn Chen]

Any plan to land this for 4.0.x?
I saw it has blocking 2.0.x.
But why didn't we get this into 4.0.1?