542122 - Add menu command to restart in safe mode

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Alex Faaborg [:faaborg] (Firefox UX)]

Attachment: Screen shot of Safe Mode shortcut directly on the user's start menu

I'm concerned about the "Mozilla Firefox (Safe Mode)" shortcut on the start menu.  While it is important for the user to have easy access to Safe Mode for the occasional trouble shooting and support issues, I believe it is introducing a number of significant usability problems:

-Having two shortcuts forces the user to decide which one they want to click on every time they launch Firefox from the start menu.  While this likely is a fast decision (perhaps just 100 or 200 ms in the ideal situation), it nonetheless places cognitive load on the user.

-Without access to external information, users may feel compelled to launch Firefox into safe mode because it is "safer" and they are concerned about online security.  The presence of a safe mode shortcut also indirectly implies that normal Firefox is not actually safe.

-(minor boundary case) With a fresh install of Windows 7, if Firefox is one of the first applications installed, "Mozilla Firefox (Safe Mode)" will sort higher than the main shortcut "Mozilla Firefox," resulting in it being placed directly on the user's start menu by itself [screen shot attached].

Possible changes:

-automatically launch into safe mode after Firefox has not been able to start up a certain number of times 
-assume that if the user is about to launch into safe mode, they are aware of what it is and and have access to a support document (so discoverability is not an issue) the remove the shortcut for safe mode, and replace with a more subtle interface, like holding down the alt key while launching Firefox.
-provide a command inside of Firefox to launch the application into safe mode (assuming that the

Comment 1

[Marco Bonardo [:mak]]

(In reply to comment #0)
> -automatically launch into safe mode after Firefox has not been able to start
> up a certain number of times 

this would not be a solution, 90% of the times we suggest to run safe mode to be sure reported bugs are not caused by add-ons.

> -assume that if the user is about to launch into safe mode, they are aware of
> what it is and and have access to a support document (so discoverability is not
> an issue) the remove the shortcut for safe mode, and replace with a more subtle
> interface, like holding down the alt key while launching Firefox.

ugh, having to hold down a button, or command line, or whatever won't help non-technical users, you can trust me. They need a simple one-click interface to run in safe mode.

Comment 2

[Marco Bonardo [:mak]]

(In reply to comment #0)
> -provide a command inside of Firefox to launch the application into safe mode

would not help if an add-on is actually causing the chrome to not come up at all or to be not working (even if in this case we could suggest an alternate more complex way).

Comment 3

[u88484]

How about changing the name to diagnostics or troubleshooting mode?

FWIW, there is an old bug about the name "safe mode" being misleading.  Probably a few more like bugs out there. Bug 259292

Bug 502958 and Bug 294260 are about starting in safe mode when startup dies

Bug 291534 about needing UI indication of being in safe mode

Comment 4

[Marco Bonardo [:mak]]

(In reply to comment #3)
> How about changing the name to diagnostics or troubleshooting mode?

i guess troubleshooting could be hard to translate in other locales (some locale already translates Safe Mode differently to avoid conflicts fwiw), Diagnostic Mode could work, but would not solve the fact it would sort higher in the menu.
I think if this is just going to rename the link, the focus for this link should be in the Diagnostic part, rather than on Mozilla Firefox, so it should most be like a phrase: "Having trouble with Firefox? (Diagnostic Mode)"

Comment 5

[Alex Faaborg [:faaborg] (Firefox UX)]

The problem I'm facing is that I can't seem to come up with a description for the Safe Mode shortcut that doesn't communicate to the user that Firefox sucks (or I guess it's a little more meta than that, Firefox could theoretically suck).  Language like "troubleshooting and diagnostic mode" conveys to everyone that Firefox is high maintenance, not a really solid product, for people who like to run diagnostics, etc.  This perhaps why we went with Safe Mode to start with, focusing on the positive instead of the negative.

For significant touch points like launching the app or the main window, discoverability needs to be proportional to usage.  In this UI it is assumed that the average user is going to launch Firefox into safe mode roughly 50% of the time that they launch Firefox from the start menu.

Comment 6

[Marco Bonardo [:mak]]

Holding down a key is probably the simplest solution but has drawbacks:
- not discoverable, user will have to ask support, while current link solution is more like try-this-and-see-what-happens.
- even if we notify the user on first start about it, i guess 50% of users won't read the tip, 40% will forget which was the key to hold down. Myself can't recall if the button for Windows safe mode is CTRL or F8 (well that has changed along versions iirc), or the same thing for BIOS SETUP (DEL, F1 or F2?).

Comment 7

[[:Cww]]

Why not do the following?

* Give an option to start it from the Help menu in Firefox.  (Restart Firefox in Diagnostic Mode) -- This will also prevent people not exiting fully and then not being able to use safe mode 
* If startup is unsuccessful (can we test for creation of browser chrome or something?) it will "It looks like Firefox isn't starting up fully, do you want to try loading it in Diagnostic mode?"
* AND offer a keyboard shortcut so if they do go to support, we can give good instructions. (To be fair, I think we usually go with the command-line option because that works on all 3 OSes)

Comment 8

[[:Cww]]

Wow, I'm sorry, I totally should have read all the above comments rather than the last one.  (Anyhow, my point is let's do all 3)

Comment 9

[Alex Limi (:limi) — Firefox UX Team]

I'm with Alex on this one, I'd argue for getting rid of the duplicate program menu item on Windows too.

As Cww points out, they usually do command line now instead, since it works the same on all OSes. If we could get the "hold down this key" (probably better to switch it to Shift instead of Alt, since Alt has menu functionality on Windows/Linux), that would be ideal. Trust me, it's easier to describe too — "hold down a key while starting Firefox" is easy to get versus command line, or even menu hunting.

I have seen people launch Firefox into "Safe Mode" because they want to be safe online. Two entries in the program menu on Windows is very confusing.

In sum, I believe we should fix the following issues:

* Remove the "(Safe Mode)" entry in the Windows programs menu
* Make the qualifier key for Safe Mode be consistent across platforms (which argues in favor for Shift as opposed to Option/Alt, since they mean menu on Windows)
* Give an option to "Start in Normal mode" instead of safe mode in the dialog that results from holding the key (its only option now is "Quit" or Safe Mode!)

Comment 10

[[:Cww]]

Shift doesn't always work as it serves as "select to range" if you have the actual program folder/app folder open. I think we will have to do some sort of per-OS magickey.  I do really like the "Start Firefox normally" option.

Also, I think in addition to this way of starting safe mode from outside Firefox, we definitely should have a way of starting safe mode from inside Firefox.  Usually we recommend safe mode when you are troubleshooting a problem and in those cases, you almost always have Firefox open already.

Comment 11

[LegNeato]

FWIW, on Mac OS X apps allow one to hold down the option key while launching the app to get different behavior. Try it with iTunes. Using shift on Mac OS X would definitely be non-standard. Looks like Apple replicates it on Windows by using shift (http://allthingsmarked.com/2006/09/13/howto-manage-multiple-libraries-in-itunes-7/)

Comment 12

[Jonathan Chang]

Internet Explorer's equivalent of "safe mode" is called "no addons," is buried within the Accessories -> System Tools menu, and has the helpful tooltip (from the Comment: field) "Start Internet Explorer without ActiveX controls or browser extensions."

Comment 13

[Carlos Alén Silva]

Mere user here.

I totally agree with the first suggestion in comment #7. Help > Restart Firefox
in Diagnostic Mode is a must.

Comment 14

[[not reading bugmail]]

I think its possible to tackle it with a number of new changes also or a mix of several ideas.

Opt-in Key:
_Shift_ has long been identifiable on Windows. 

Microsoft uses _shift_ to bypass startup routines/macros for Office Apps and to bypass Windows Startup programs.  They also use security notification bars in Office to communicate allowing un-trusted macros to run, so you have either trust once or enabled each time. 
 
Icons:
Veteran Windows users should know what is Safe Mode, newbies, no, I can agree it doesn't help to not know what is Safe Mode vs Secure or even PB Mode.  

Change Safe Mode icon to (No Add-ons Mode).  
Update the original to show  (Normal Mode).
Could add a third icon for  (Private Mode).  ;)

Add more Safe Mode Feedback:
I also like the idea to always show a dialog/notification in safe mode.  You could state something like the following every time it starts in safe mode:

"Safe Mode is for troubleshooting and diagnostics of the browser.  You are about to start Firefox without browser extensions or Add-ons."  Do you want to Continue?  blah, blah.. 

Help Menu:
Is _Troubleshooting_ Support menu item localized?

I also like the Help->Restart in Diagnostic mode, but certainly won't help users who crash on startup and cannot open the browser without first starting in safe mode.

Comment 15

[Alex Faaborg [:faaborg] (Firefox UX)]

Scoping this bug to three specific actions:

1) remove the "Firefox (Safe Mode)" shortcut
2) introduce a menu command in the help menu to relaunch in with add-ons disabled
3) add a keyboard modifier on launch to enter into safe mode

I'm requesting blocking final on this since having a shortcut on Windows that implies Firefox often breaks undermines user confidence in it being a high quality product, even if they never need to select the Safe Mode shortcut. (and additionally there is some room for confusion over which option is more "secure").

Comment 16

[(mostly gone) XtC4UaLL [:xtc4uall]]

(In reply to comment #15)
> Scoping this bug to three specific actions:
> 2) introduce a menu command in the help menu to relaunch in with add-ons
> disabled
That is Bug 417272.

Comment 17

[Alex Faaborg [:faaborg] (Firefox UX)]

Limi proposes grouping the command to disable all add-ons inside of the About Window (along with a link to troubleshooting information).  I'm fine with that as well.  Switching the three actions to:

1) Remove the "Firefox (Safe Mode)" shortcut
2) Introduce a command in the About window to relaunch in with add-ons
disabled
3) Add a keyboard modifier on launch to enter into safe mode

Comment 18

[Dietrich Ayala (:dietrich)]

Blocking+ to resolve the original problem in the attached screenshot. Whatever implementation we end up with, that must be confirmed fixed.

Comment 19

[Benjamin Smedberg]

We already launch Firefox into safe mode if you hold down the option key on Mac. On Windows there really isn't a good key to bind: both control and alt are very commonly held down for other things, and I suspect that accidentally launching into safe mode would be even more confusing.

Comment 20

[Alex Limi (:limi) — Firefox UX Team]

(In reply to comment #19)
> We already launch Firefox into safe mode if you hold down the option key on
> Mac. On Windows there really isn't a good key to bind: both control and alt are
> very commonly held down for other things, and I suspect that accidentally
> launching into safe mode would be even more confusing.

I don't think that should be a reason to not do it. We can fix this by messaging ("Firefox was started in safe mode because you held down shift when starting it").

I believe the Shift key is the most natural modifier to use on Windows for this kind of behavior — but Faaborg might have other suggestions.

In any case, there should be similar ways to enter safe mode across platforms. I don't believe Windows users are more error-prone than Mac users wrt. holding down a qualifier key when starting to activate safe mode.

Comment 21

[[not reading bugmail]]

I think that is good idea Limi.  

My experience is that most people don't even know about qualifier key on Windows, but those that do, make sure other people know what its for.  I believe in this case to be like the platform, and not be something strange or new, people will appreciate.  I don't think we need to worry about accidental use cases as long as we have a dialog and a way to continue to startup in normal mode or continue to start in safe mode.  That should cover it.

Comment 22

[Alex Faaborg [:faaborg] (Firefox UX)]

Mockup of the menu item command placed in the help menu: https://bug583386.bugzilla.mozilla.org/attachment.cgi?id=467305

I believe we should add this in addition to the keyboard command, since it is mouse driven, and a bit easier to explain (assuming the user has Firefox running).

Comment 23

[[not reading bugmail]]

(In reply to comment #22)
> Mockup of the menu item command placed in the help menu:
> https://bug583386.bugzilla.mozilla.org/attachment.cgi?id=467305
> 
> I believe we should add this in addition to the keyboard command, since it is
> mouse driven, and a bit easier to explain (assuming the user has Firefox
> running).

Yeah, agreed, very nice indeed.

Comment 24

[Benjamin Smedberg]

I am primarily worried about users who will have held down the shortcut as part of another operation: e.g. quicksilver-like launch keys for power users are common, and I personally have hooked them up with Ctrl-Shift.

Comment 25

[Alex Faaborg [:faaborg] (Firefox UX)]

Depending on when the check occurs during startup, the user should have plenty of time to uplick any modifiers.  This behavior is standard on OS X, and on windows the odds of the user hitting a modifier key on launch are very small (using Enso?)  Either way the problem we are trying to address currently effects 100% of Windows users.

Comment 26

[Alex Faaborg [:faaborg] (Firefox UX)]

This is pretty big product perception issue.  At one point VLC installed with two shortcuts, one to launch VLC, and the other to launch VLC with DirectX disabled.  I've actually never seen VLC crash on Windows, but from viewing the two shortcuts I have irrationally (and perhaps permanently) formed the cover-of-book conclusion that it is not a high quality product. This is all because they gave me a top level option to indicate "please stop being so broken"

Comment 27

[:Gavin Sharp [email: [e-mail]]]

The key check needs to happen very early in the startup process.

(On Mac, the Alt behavior really bothers me, to the point where I've considered disabling the relevant code in my development tree. But I use Alt+A for window switching in irssi, so I'm more susceptible to that particular problem than the average user.)

Comment 28

[Alex Limi (:limi) — Firefox UX Team]

(In reply to comment #27)
> (On Mac, the Alt behavior really bothers me, to the point where I've considered
> disabling the relevant code in my development tree. But I use Alt+A for window
> switching in irssi, so I'm more susceptible to that particular problem than the
> average user.)

Yeah, I think you're a special case — FWIW, OS X has a pretty well-established custom of holding down the Option (aka. "Alt") key to get to special modes when starting applications, and indeed the computer itself.

Comment 29

[Mike Beltzner [:beltzner, not reading bugmail]]

Moving to beta6+ at this point; needs an owner, any volunteers? I think the right way forward is the menuItem, would prefer to make it Alt just for consistency.

Comment 30

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Attachment: v0

Adds the menu item to the app menu. Adds the localization string for the menuitem.

I have no idea what oncommand should be set to. I don't even know if there's an existing function for "restart in safe mode". So I left that blank.

It's a start, at least, right?

Comment 31

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

This does have a string, so if it is planned for Fx4, someone should add the [strings] tag to the whiteboard.

Comment 32

[Axel Hecht [:Pike]]

Looks like "Restart with Add-ons Disabled..." needs a language zar. Ellipsis should be unicode glyph as elsewhere in the file, 'disabled' lowercase, not sure about the rest of the wording.

Comment 33

[Alex Faaborg [:faaborg] (Firefox UX)]

How about "disable all add-ons" we can mention the restart in a dialog box confirming the action (which we will need to have either way)

Comment 34

[Dietrich Ayala (:dietrich)]

Thanks for getting this started Wes. I'm assigning over to Ddahl to take it the rest of the way, w/ the keyboard shortcut and whatever other changes need to happen at startup to check for it.

Comment 35

[David Dahl :ddahl]

According to mossop we can:

1. set an env var to start in safe mode from firefox front-end via a "restartInSafeMode" function we add to browser.js, then call Application.restart()

see http://mxr.mozilla.org/mozilla-central/source/extensions/pref/autoconfig/src/prefcalls.js#236

2. check for said env var in toolkit/xre/nsAppRunner.cpp 

see http://mxr.mozilla.org/mozilla-central/source/toolkit/xre/nsAppRunner.cpp#2992

3. if safe mode is "true" you just set gSafeMode like in:

see http://mxr.mozilla.org/mozilla-central/source/toolkit/xre/nsAppRunner.cpp#3047

4. unset the env var in nsAppRunner.cpp as well.

Comment 36

[Mike Beltzner [:beltzner, not reading bugmail]]

(In reply to comment #33)
> How about "disable all add-ons" we can mention the restart in a dialog box
> confirming the action (which we will need to have either way)

It's not that permanent, though. Cheng: are we concerned about renaming this from "Safe Mode" which has a bunch of documentation (some that we control, others on fan sites) already using it?

Alex: can you live with "Restart in Safe Mode" or "Enter Safe Mode" much like "Enter Private Browsing Mode"?

Comment 37

[David Dahl :ddahl]

Attachment: v 0.1 safe-mode-tweaks

Just getting this spun up to see how much work it is. For some reason the menu item does not appear at all. I must not be building correctly. I tried  make -C toolkit && make -C toolkit/library/ && make -C browser

and then cd browser && make chrome

Comment 38

[Alex Faaborg [:faaborg] (Firefox UX)]

>Alex: can you live with "Restart in Safe Mode"

I'm worried that safe has no context for the user, and kind of sounds like it might be better than private browsing mode, or more secure than normal Firefox.  We can change our own documentation, and I'm not sure we should avoid a more descriptive term based on the assumption that fan sites won't update their content eventually.

Comment 39

[Dietrich Ayala (:dietrich)]

Comment on attachment 472776 [details] [diff] [review]
v 0.1 safe-mode-tweaks


>+  if (PR_GetEnv("MOZ_SAFE_MODE_RESTART") == "1") {
>+    gSafeMode = PR_TRUE;
>+    // unset the env variable
>+    PR_SetEnv("MOZ_SAFE_MODE_RESTART=0");
>+  }
>+  
>   ar = CheckArg("safe-mode", PR_TRUE);
>   if (ar == ARG_BAD) {
>     PR_fprintf(PR_STDERR, "Error: argument -safe-mode is invalid when argument -osint is specified\n");
>     return 1;
>   } else if (ar == ARG_FOUND) {
>     gSafeMode = PR_TRUE;
>   }
> 

should the safemode-related code that follows be in an else block?

could be that you've rebuilt, but need to blow away XUL.mfasl in your profile for the browser.xul changes to show?

Comment 40

[David Dahl :ddahl]

(In reply to comment #39)
> Comment on attachment 472776 [details] [diff] [review]
> v 0.1 safe-mode-tweaks
> 
> 
> >+  if (PR_GetEnv("MOZ_SAFE_MODE_RESTART") == "1") {
> >+    gSafeMode = PR_TRUE;
> >+    // unset the env variable
> >+    PR_SetEnv("MOZ_SAFE_MODE_RESTART=0");
> >+  }
> >+  
> >   ar = CheckArg("safe-mode", PR_TRUE);
> >   if (ar == ARG_BAD) {
> >     PR_fprintf(PR_STDERR, "Error: argument -safe-mode is invalid when argument -osint is specified\n");
> >     return 1;
> >   } else if (ar == ARG_FOUND) {
> >     gSafeMode = PR_TRUE;
> >   }
> > 
> 
> should the safemode-related code that follows be in an else block?
yeah, perhaps so.

> 
> could be that you've rebuilt, but need to blow away XUL.mfasl in your profile
> for the browser.xul changes to show?

just did a clobber, and used purgecaches and is ok now.

Comment 41

[David Dahl :ddahl]

Attachment: v 0.2 safe-mode-tweaks

Should this be windows only? won't it work cross-platform?

Comment 42

[Robert Strong (they/them - no direct email)]

Comment on attachment 473339 [details] [diff] [review]
v 0.2 safe-mode-tweaks

There must be a better way than adding the same string in 2 places though I'm not sure what the best way would be.

... should be unicode …

for example,
http://mxr.mozilla.org/mozilla-central/source/browser/locales/en-US/installer/custom.properties#96


+    // unset the env variable
+    PR_SetEnv("MOZ_SAFE_MODE_RESTART=0");
to unset this should be
PR_SetEnv("MOZ_SAFE_MODE_RESTART=");

I don't think you want the if else... CheckArg removes the argument by default and the if else prevents safe-mode from being removed.

You should probably get bsmedberg to look at this as well

Comment 43

[David Dahl :ddahl]

Attachment: v 0.3 safe-mode-tweaks

I ifdef'd the UI bits to keep this windows only. Is that correct? It seems like this code will work on all platforms.

Comment 44

[David Dahl :ddahl]

Attachment: v 0.3 safe-mode-tweaks

forgot the unicode ellipsis

Comment 45

[Dietrich Ayala (:dietrich)]

Comment on attachment 473747 [details] [diff] [review]
v 0.3 safe-mode-tweaks

r+ on the browser bits. could clean up a little by moving to a command (calling the func directly in only one place then), but that's not required.

per rs's suggestion, should get bsmedberg review on the apprunner changes.

Comment 46

[Jesper Kristensen]

When you remove the Safe Mode shortcut, will you also move the main Firefox shortcut out of the sub folder?

All Programs
- Mozilla Firefox

is much nicer than

All Programs
- Mozilla Firefox
  - Mozilla Firefox

Can you also make sure you clean up the start menu? On my previous computer I had a whole bunch of Safe Mode shortcuts because I had installed Firefox in two different languages and one of them had at some time changed the localization of the Safe Mode shortcut. So I had:

All Programs
 - Mozilla Firefox
   - Mozilla Firefox
   - Mozilla Firefox (Safe Mode)
   - Mozilla Firefox (Fejlsikret tilstand)
   - Mozilla Firefox (Sikker tilstand)

(on my current computer I only have a lot of shortcuts from betas, which doesn't seem to clean up after themselves)

Comment 47

[Alex Faaborg [:faaborg] (Firefox UX)]

>I ifdef'd the UI bits to keep this windows only. Is that correct? It seems like
>this code will work on all platforms.

I think we should have this on all platforms, but it is particularly important for us to have it on windows (due to the start menu shortcuts).

>When you remove the Safe Mode shortcut, will you also move the main Firefox
>shortcut out of the sub folder?

Yeah, placing inside of a folder doesn't make sense for a single shortcut.

Comment 48

[David Dahl :ddahl]

rs: I assume the shortcut is created at install time, correct?

Comment 49

[David Dahl :ddahl]

(In reply to comment #48)
> rs: I assume the shortcut is created at install time, correct?

And, is removing it a separate bug + patch?

Comment 50

[David Dahl :ddahl]

Attachment: v 0.4 removed ifdefs making it x-platform

Comment 51

[David Dahl :ddahl]

Tested on linux, works well.

Comment 52

[David Dahl :ddahl]

Just want to double check a few things:

1. on update should we also remove any existing safe mode shortcut?

2. if so, should we also remove the folder that both application icons are in under Start -> All Programs -> Firefox? - as this folder was only added when we needed it to contain both shortcut icons.

Note:

Spoke with bsmedberg about using the env var approach. This is OK since we do not have the infrastructure to handle the flags approach.

Comment 53

[[:Cww]]

(In reply to comment #36)
Sorry was in a workweek, catching up on bugs.

Ideally, say something like "Restart with add-ons disabled (in safe mode)".  In any case, when you start, it's still going to say "Safe mode" right? so all documentation calling it safe mode is not badbad.  (Are we actually redoing all the UI around safe mode, the dialog boxes etc?)

We have a major 3 week cram period in our schedule at the start of Q4 to update every page of documentation affected by Firefox 4 changes so whatever the outcome, we'll document and do outreach.

Comment 54

[Axel Hecht [:Pike]]

I think I'm missing at least one accesskey in the latest patch, fwiw.

Comment 55

[Mike Beltzner [:beltzner, not reading bugmail]]

Is this going to make it? We need to make a call. I'd rather we fix this than not, but I don't think I'd hold the entire release on it.

Comment 56

[David Dahl :ddahl]

(In reply to comment #55)
> Is this going to make it? We need to make a call. I'd rather we fix this than
> not, but I don't think I'd hold the entire release on it.

This patch is written and works. It may need one more accesskey (i can fix that quickly), and I was waiting to hear from faaborg about the questions I posted in comment 52. If we need to remove the folder and previously created shortcuts, then there is a trifle of nsis scripting that rs said he would help me with (I cannot imagine that taking more than an hour).

Comment 57

[Alex Faaborg [:faaborg] (Firefox UX)]

>1. on update should we also remove any existing safe mode shortcut?

yes

>2. if so, should we also remove the folder that both application icons are in
>under Start -> All Programs -> Firefox? - as this folder was only added when we
>needed it to contain both shortcut icons.

yes

Comment 58

[David Dahl :ddahl]

Attachment: v 0.5 Added Accesskeys

added accesskeys, next up the nsis tweaks. this, however, is an independent patch.

Comment 59

[David Dahl :ddahl]

Comment on attachment 476914 [details] [diff] [review]
v 0.5 Added Accesskeys

http://img830.imageshack.us/img830/5844/minefieldstartpageminefd.png

http://img830.imageshack.us/img830/5317/minefieldstartpageminef.png

Menuitem screenshots with this patch applied

Comment 60

[David Dahl :ddahl]

Attachment: v 0.1 nsis patch

I'm pretty sure I am missing something here

Comment 61

[Robert Strong (they/them - no direct email)]

Comment on attachment 476926 [details] [diff] [review]
v 0.1 nsis patch

This is missing more than I can say in a comment ;)

I'll take a look at what's missing in a bit

Comment 62

[David Dahl :ddahl]

(In reply to comment #61)
> Comment on attachment 476926 [details] [diff] [review]
> v 0.1 nsis patch
> 
> This is missing more than I can say in a comment ;)
> 
> I'll take a look at what's missing in a bit

Yeah, I just am not sure where some of the objects come from:)

Comment 63

[Robert Strong (they/them - no direct email)]

Here is something to get you started

The startmenu directory select page for the advanced installation will need to be removed since we will no longer support folders. See the section in installer.nsi that starts with:
; Start Menu Folder Page Configuration

along with the related functions

On install / update the safe mode and normal shortcuts will need to be removed from the startmenu directory and the directory removed if it is now empty in the same manner as is done during uninstall.

Comment 64

[David Dahl :ddahl]

Attachment: v 0.2 nisis patch

I feel a little hesitant in working with nsis, especially since these are installer bits.

Comment 65

[Robert Strong (they/them - no direct email)]

Comment on attachment 477147 [details] [diff] [review]
v 0.2 nisis patch

># HG changeset patch
># Parent ebcb166a9e628426e05d5fdcb8ed167d4d383567
>
>diff --git a/browser/installer/windows/nsis/installer.nsi b/browser/installer/windows/nsis/installer.nsi
>--- a/browser/installer/windows/nsis/installer.nsi
>+++ b/browser/installer/windows/nsis/installer.nsi
>...
>@@ -267,22 +261,16 @@ Section "-Application" APP_IDX
>   ${LogUninstall} "File: \active-update.xml"
>   ${LogUninstall} "File: \install.log"
>   ${LogUninstall} "File: \install_status.log"
>   ${LogUninstall} "File: \install_wizard.log"
>   ${LogUninstall} "File: \updates.xml"
> 
>   ClearErrors
> 
>-  ; Default for creating Start Menu folder and shortcuts
>-  ; (1 = create, 0 = don't create)
>-  ${If} $AddStartMenuSC == ""
>-    StrCpy $AddStartMenuSC "1"
>-  ${EndIf}
We still need to default to creating a start menu shortcut and allow the option not to for the advanced install case.

>   ; Default for creating Quick Launch shortcut (1 = create, 0 = don't create)
>   ${If} $AddQuickLaunchSC == ""
>     StrCpy $AddQuickLaunchSC "1"
>   ${EndIf}
> 
>   ; Default for creating Desktop shortcut (1 = create, 0 = don't create)
>   ${If} $AddDesktopSC == ""
>     StrCpy $AddDesktopSC "1"
>@@ -379,37 +367,35 @@ Section "-Application" APP_IDX
>   ${LogHeader} "Adding Shortcuts"
> 
>   !insertmacro MUI_STARTMENU_WRITE_BEGIN Application
> 
>   ; Always add the relative path to the application's Start Menu directory and
>   ; the application's shortcuts to the shortcuts log ini file. The
>   ; DeleteShortcuts macro will do the right thing on uninstall if they don't
>   ; exist.
>-  ${LogSMProgramsDirRelPath} "$StartMenuDir"
>   ${LogSMProgramsShortcut} "${BrandFullName}.lnk"
>-  ${LogSMProgramsShortcut} "${BrandFullName} ($(SAFE_MODE)).lnk"
>   ${LogQuickLaunchShortcut} "${BrandFullName}.lnk"
>   ${LogDesktopShortcut} "${BrandFullName}.lnk"
>+  ${LogSMProgramsDirRelPath} "$StartMenuDir"
Don't move ${LogSMProgramsDirRelPath}
Also, you removed the code that sets $StartMenuDir so this won't work. The code that removes the shortcuts might need to be reworked since we no longer have a directory.

>   ; UAC only allows elevating to an Admin account so there is no need to add
>   ; the Start Menu or Desktop shortcuts from the original unelevated process
>   ; since this will either add it for the user if unelevated or All Users if
>   ; elevated.
>   ${If} $AddStartMenuSC == 1
>-    ${Unless} ${FileExists} "$SMPROGRAMS\$StartMenuDir"
>-      CreateDirectory "$SMPROGRAMS\$StartMenuDir"
>-      ${LogMsg} "Added Start Menu Directory: $SMPROGRAMS\$StartMenuDir"
>-    ${EndUnless}
>-    CreateShortCut "$SMPROGRAMS\$StartMenuDir\${BrandFullName}.lnk" "$INSTDIR\${FileMainEXE}" "" "$INSTDIR\${FileMainEXE}" 0
>-    ApplicationID::Set "$SMPROGRAMS\$StartMenuDir\${BrandFullName}.lnk" "${AppUserModelID}"
>-    ${LogMsg} "Added Shortcut: $SMPROGRAMS\$StartMenuDir\${BrandFullName}.lnk"
>-    CreateShortCut "$SMPROGRAMS\$StartMenuDir\${BrandFullName} ($(SAFE_MODE)).lnk" "$INSTDIR\${FileMainEXE}" "-safe-mode" "$INSTDIR\${FileMainEXE}" 0
>-    ApplicationID::Set "$SMPROGRAMS\$StartMenuDir\${BrandFullName} ($(SAFE_MODE)).lnk" "${AppUserModelID}"
>-    ${LogMsg} "Added Shortcut: $SMPROGRAMS\$StartMenuDir\${BrandFullName} ($(SAFE_MODE)).lnk"
>+    CreateShortCut "$SMPROGRAMS\${BrandFullName}.lnk" "$INSTDIR\${FileMainEXE}" "" "$INSTDIR\${FileMainEXE}" 0
>+    ApplicationID::Set "$SMPROGRAMS\${BrandFullName}.lnk" "${AppUserModelID}"
>+    ${LogMsg} "Added Shortcut: $SMPROGRAMS\${BrandFullName}.lnk"
>+  ${EndIf}
>+
>+  ; Remove unneeded start menu directory
>+  ${If} ${FileExists} "$SMPROGRAMS\$StartMenuDir"
>+    RmDir "$SMPROGRAMS\$StartMenuDir"
>+    ${LogMsg} "Removed Start Menu Directory: $SMPROGRAMS\$StartMenuDir"
>   ${EndIf}
You need to remove the shortvuts in our startmenu directory first. Do this in a new macro in shared.nsh so post update can use the same code. Also, you can't use $StartMenuDir since you removed the code that sets its value. Instead use the similar code as the uninstaller.

If you would like me to do this I can do so but it might be a day before I am able to due to other work

Comment 66

[David Dahl :ddahl]

Comment on attachment 476914 [details] [diff] [review]
v 0.5 Added Accesskeys

Rob: can you look this over one more time so we can land it independent of the nsis patch? It had r+, but I added strings and removed the windows-only approach. I will push to try so it can be tested on Mac too.

Comment 67

[Robert Strong (they/them - no direct email)]

Comment on attachment 476914 [details] [diff] [review]
v 0.5 Added Accesskeys

I just did an interdiff review between 0.3 and 0.5

diff --git a/browser/base/content/baseMenuOverlay.xul b/browser/base/content/baseMenuOverlay.xul
--- a/browser/base/content/baseMenuOverlay.xul
+++ b/browser/base/content/baseMenuOverlay.xul
@@ -103,16 +103,21 @@
                   label="&helpReleaseNotes.label;"
                   oncommand="openReleaseNotes()"
                   onclick="checkForMiddleClick(this, event);"/>
         <menuitem id="feedbackPage"
                   accesskey="&helpFeedbackPage.accesskey;"
                   label="&helpFeedbackPage.label;"
                   oncommand="openFeedbackPage()"
                   onclick="checkForMiddleClick(this, event);"/>
+        <menuitem id="helpMenuSafeMode"
+		  accesskey="&helpMenuSafeMode.accesskey;"
tabs!

+                  label="&helpMenuSafeMode.label;"
+                  oncommand="safeModeRestart();"/>
+        <menuseparator/>
         <menuseparator id="updateSeparator"/>

diff --git a/browser/base/content/browser.xul b/browser/base/content/browser.xul
--- a/browser/base/content/browser.xul
+++ b/browser/base/content/browser.xul
@@ -30,16 +30,17 @@
 #   Pierre Chanial <[e-mail]>
 #   Dean Tessman <[e-mail]>
 #   Johnathan Nightingale <[e-mail]>
 #   Dão Gottwald <[e-mail]>
 #   Ehsan Akhgari <[e-mail]>
 #   Robert Strong <[e-mail]>
 #   Rob Campbell <[e-mail]>
 #   Patrick Walton <[e-mail]>
+#   David Dahl <[e-mail]>
 #
 # Alternatively, the contents of this file may be used under the terms of
 # either the GNU General Public License Version 2 or later (the "GPL"), or
 # the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
 # in which case the provisions of the GPL or the LGPL are applicable instead
 # of those above. If you wish to allow use of your version of this file only
 # under the terms of either the GPL or the LGPL, and not to allow others to
 # use your version of this file under the terms of the MPL, indicate your
@@ -757,16 +758,21 @@
                             label="&helpMenu.label;"
                             oncommand="openHelpLink('firefox-help')"
                             onclick="checkForMiddleClick(this, event);"/>
                   <menuitem id="appmenu_gettingStarted"
                             label="&appMenuGettingStarted.label;"
                             oncommand="gBrowser.loadOneTab('http://www.mozilla.com/firefox/central/', {inBackground: false});"
                             onclick="checkForMiddleClick(this, event);"/>
                   <menuseparator/>
+                  <menuitem id="appmenu_safeMode"
+			    accesskey="&appMenuSafeMode.accesskey;"
tabs!

+<!ENTITY helpMenuSafeMode.label   "Restart with Add-ons Disabled (Safe Mode)…">
+<!ENTITY helpMenuSafeMode.accesskey "R">
You might as well go with helpSafeMode for consistency with the other strings which will allow you to align these like the rest.
I didn't read the bug comments to find out whether you got ui-r+ for the strings... I trust that you have.

r=me for the changes between 0.3 and 0.5

Comment 68

[Alex Faaborg [:faaborg] (Firefox UX)]

Comment on attachment 476914 [details] [diff] [review]
v 0.5 Added Accesskeys

Remove the (Save Mode) in the string, it makes it too long and isn't needed to describe the functionality.  Support documents can be updated.

Comment 69

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 476914 [details] [diff] [review]
v 0.5 Added Accesskeys

The menu item label is a bit long for Mac: http://grab.by/6uFL

You should also get rid of the quotes in the properties file, they show up in the dialog: http://grab.by/6uFQ

Comment 70

[Alex Limi (:limi) — Firefox UX Team]

Comment on attachment 476914 [details] [diff] [review]
v 0.5 Added Accesskeys

Agreed, we're trying to get away from implying that there's anything "safer" about using the browser like that, which is part of the original issue.

ui-r+ with the "Safe Mode" parenthetical references removed.

Comment 71

[[not reading bugmail]]

FWIW, IE uses Add-ons Disabled terminology.

Comment 72

[David Dahl :ddahl]

Attachment: v 0.6 Safe Mode patch for checkin

Fixed faaborg's and rs' review comments

Comment 73

[David Dahl :ddahl]

Attachment: v 0.7 Safe Mode patch for checkin [rebased] Patch 1 of 2

Rebased to current trunk, pushing to try

Comment 74

[David Dahl :ddahl]

Patch 1 (that has strings and is everything but the installer stuff) landed: http://hg.mozilla.org/mozilla-central/rev/084a262560eb

Comment 75

[[e-mail]]

Comment on attachment 477382 [details] [diff] [review]
v 0.7 Safe Mode patch for checkin [rebased] Patch 1 of 2

>-/* duplicateTabIn duplicates tab in a place specified by the parameter |where|.
>+// Prompt user to restart the browser in safe mode 
>+function safeModeRestart()
>+{
...
>+}/* duplicateTabIn duplicates tab in a place specified by the parameter |where|.
I don't think you meant to do this to the poor comment...

Comment 76

[pal-moz]

after click "Restart with Add-ons Disabled",
there are 2 steps.
1) http://img191.imageshack.us/img191/343/ss1po.jpg
2) http://img710.imageshack.us/img710/3408/ss2pg.jpg

I think safe mode will start automatically.
but not automatically.

so "Restart with Add-ons Disabled" should be "Start Safe Mode" (like "Start Private Browsing") ?

Comment 77

[David Dahl :ddahl]

(In reply to comment #76)
> after click "Restart with Add-ons Disabled",
> there are 2 steps.
> 1) http://img191.imageshack.us/img191/343/ss1po.jpg
> 2) http://img710.imageshack.us/img710/3408/ss2pg.jpg
> 
> I think safe mode will start automatically.
> but not automatically.

Please see the previous comments by the UI/UX team.

Comment 78

[David Dahl :ddahl]

(In reply to comment #75)

> I don't think you meant to do this to the poor comment...

I really boned it. I'll file a followup bug.

Comment 79

[David Dahl :ddahl]

Opened a spin off for the nsis patch: bug 598779

Comment 80

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #75)
> >+}/* duplicateTabIn duplicates tab in a place specified by the parameter |where|.
> I don't think you meant to do this to the poor comment...

http://hg.mozilla.org/mozilla-central/rev/043dbdb238c4

Comment 81

[Marcia Knous [:marcia]]

https://litmus.mozilla.org/show_test.cgi?id=10031 was updated to reflect the menu change, but there needs to be be additional test cases added for this feature.

Comment 82

[(no longer active)]

So, this bug was supposed to replace the safe mode shortcut with a menu command *and* a keyboard modifier.

As far as I can tell, the latter did not happen.  Therefore this bug is not fully fixed.

Comment 83

[Robert Strong (they/them - no direct email)]

Ehsan, the removal of the shortcuts will happen in bug 598779

Comment 84

[Mike Beltzner [:beltzner, not reading bugmail]]

Ehsan is correct; there *must* be a way for users to launch Firefox in Safe Mode from the desktop. Otherwise we can't diagnose startup crashes.

If we can't do that, we'll need to re-add the additional Windows Application shortcut for the desktop.

Comment 85

[Mike Beltzner [:beltzner, not reading bugmail]]

Resummarizing; we'll deal with the shortcuts and such in bug 598779

Comment 86

[Robert Strong (they/them - no direct email)]

(In reply to comment #84)
> Ehsan is correct; there *must* be a way for users to launch Firefox in Safe
> Mode from the desktop. Otherwise we can't diagnose startup crashes.
Not what was said in comment #82 but I am fine with not removing the shortcuts for this exact reason and am fine with wontfixing bug 598779 if the shortcuts won't be removed

Comment 87

[tonda kavalec]

what about replace safe-mode with:
default setting -mode
no extensions -mode
solving problems -mode

your decision about removing the -save-mode item from the menu is WRONG as well as the "arguments". Users have the normal link on the desktop, in quickstart menu, in the root of Startmenu, using Firefox aften if not every day.

Comment 88

[[not reading bugmail]]

Last I checked we added Restart with Add-ons Disabled... in the help menu to deal this as well as holding down the shift key to start in safe mode.  Both of which were requirements before removing the shortcut on the windows start menu.

Comment 89

[Tomer Cohen :tomer]

How's about adding a 'restart in safe mode' button to about:support as suggested by bug 547623?

Comment 90

[Alice Wyman]

(In reply to comment #76)
> after click "Restart with Add-ons Disabled",
> there are 2 steps.
> 1) http://img191.imageshack.us/img191/343/ss1po.jpg
> 2) http://img710.imageshack.us/img710/3408/ss2pg.jpg
> 
> I think safe mode will start automatically.
> but not automatically.
> 
> so "Restart with Add-ons Disabled" should be "Start Safe Mode" (like "Start
> Private Browsing") ?

I agree. 

When you select ""Restart with Add-ons Disabled..." from the Help menu, you see a dialog box, "Are you sure you want to disable all add-ons and restart?".  If you confirm,  Firefox restarts but before it displays a browser window, you presented with the "Firefox Safe Mode" window containing a series of troubleshooting options.  You would have to check the option to "Disable all add-ons" and then select "Make changes and Restart") in order to restart Firefox with all add-ons (including plugins) disabled.

The menu option should be "Restart in Firefox Safe Mode" or something similar, if that's what happens.  and not "Restart with Add-ons Disabled...".  The confirmation dialog should also refer to "Firefox Safe Mode".

Comment 91

[Swarnava Sengupta (:Swarnava)]

Removing user-doc-needed keyword!

https://support.mozilla.org/en-US/kb/troubleshoot-firefox-issues-using-safe-mode

Comment 92

[Fahmida Noor]

I have reproduced this bug with Firefox Nightly 3.7a1pre on Windows 7(64-bit).

Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a1pre) Gecko/20100125 Minefield/3.7a1pre

Verified as fixed with Latest Firefox Nightly 46.0a1 (Build ID : 20160114030246)

Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:46.0) Gecko/20100101 Firefox/46.0

[bugday-20160113]