Add menu command to restart in safe mode

RESOLVED FIXED in Firefox 4.0b7

Status

()

defect
RESOLVED FIXED
10 years ago
4 years ago

People

(Reporter: faaborg, Assigned: ddahl)

Tracking

unspecified
Firefox 4.0b7
x86
Windows 7
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite ?
in-litmus ?

Firefox Tracking Flags

(blocking2.0 beta7+)

Details

(Whiteboard: [strings] [strings patch landed] [functionality landed])

Attachments

(2 attachments, 10 obsolete attachments)

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
(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.
(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

10 years ago
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
(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)"
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.
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

10 years ago
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

10 years ago
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)
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

10 years ago
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

9 years ago
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

9 years ago
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."
Mere user here.

I totally agree with the first suggestion in comment #7. Help > Restart Firefox
in Diagnostic Mode is a must.
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.
Summary: Safe mode start menu item may cause more problems than it solves → Replace safe mode shortcut with keyboard modifier and menu command
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").
blocking2.0: --- → ?
(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.
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
Blocking+ to resolve the original problem in the attached screenshot. Whatever implementation we end up with, that must be confirmed fixed.
blocking2.0: ? → beta5+
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.
(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.
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.
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).
(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.
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.
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.
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"
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.)
(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.
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.
blocking2.0: beta5+ → beta6+
Posted patch v0 (obsolete) — Splinter Review
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?
Attachment #471414 - Flags: feedback?
This does have a string, so if it is planned for Fx4, someone should add the [strings] tag to the whiteboard.
Whiteboard: [strings]
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.
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)
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.
Assignee: nobody → david
Assignee

Updated

9 years ago
Assignee: david → ddahl
Assignee

Comment 35

9 years ago
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.
(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"?
Assignee

Comment 37

9 years ago
Posted patch v 0.1 safe-mode-tweaks (obsolete) — Splinter Review
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
Attachment #471414 - Attachment is obsolete: true
Attachment #472776 - Flags: feedback?(dietrich)
Attachment #471414 - Flags: feedback?
>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 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?
Attachment #472776 - Flags: feedback?(dietrich) → feedback+
Assignee

Comment 40

9 years ago
(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.
Assignee

Comment 41

9 years ago
Posted patch v 0.2 safe-mode-tweaks (obsolete) — Splinter Review
Should this be windows only? won't it work cross-platform?
Attachment #472776 - Attachment is obsolete: true
Attachment #473339 - Flags: feedback?(robert.bugzilla)
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
Attachment #473339 - Flags: feedback?(robert.bugzilla) → feedback+
Whiteboard: [strings] → [strings][has WIP patch]
Assignee

Comment 43

9 years ago
Posted patch v 0.3 safe-mode-tweaks (obsolete) — Splinter Review
I ifdef'd the UI bits to keep this windows only. Is that correct? It seems like this code will work on all platforms.
Attachment #473339 - Attachment is obsolete: true
Attachment #473742 - Flags: review?(dietrich)
Assignee

Comment 44

9 years ago
Posted patch v 0.3 safe-mode-tweaks (obsolete) — Splinter Review
forgot the unicode ellipsis
Attachment #473742 - Attachment is obsolete: true
Attachment #473747 - Flags: review?(dietrich)
Attachment #473742 - Flags: review?(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.
Attachment #473747 - Flags: review?(dietrich) → review+
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)
Assignee

Updated

9 years ago
Attachment #473747 - Flags: review?(benjamin)
>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.
Assignee

Comment 48

9 years ago
rs: I assume the shortcut is created at install time, correct?
Assignee

Comment 49

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

And, is removing it a separate bug + patch?
Assignee

Comment 50

9 years ago
Attachment #473747 - Attachment is obsolete: true
Attachment #474812 - Flags: review?(benjamin)
Attachment #473747 - Flags: review?(benjamin)
Assignee

Comment 51

9 years ago
Tested on linux, works well.
Assignee

Comment 52

9 years ago
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.
Assignee

Updated

9 years ago
Attachment #474812 - Flags: review?(benjamin)

Comment 53

9 years ago
(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.
I think I'm missing at least one accesskey in the latest patch, fwiw.
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.
Assignee

Comment 56

9 years ago
(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).
>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
Assignee

Comment 58

9 years ago
Posted patch v 0.5 Added Accesskeys (obsolete) — Splinter Review
added accesskeys, next up the nsis tweaks. this, however, is an independent patch.
Assignee

Comment 60

9 years ago
Posted patch v 0.1 nsis patch (obsolete) — Splinter Review
I'm pretty sure I am missing something here
Attachment #476926 - Flags: feedback?(robert.bugzilla)
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
Attachment #476926 - Flags: feedback?(robert.bugzilla) → feedback-
Assignee

Comment 62

9 years ago
(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:)
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.
Assignee

Comment 64

9 years ago
I feel a little hesitant in working with nsis, especially since these are installer bits.
Attachment #476926 - Attachment is obsolete: true
Attachment #477147 - Flags: review?(robert.bugzilla)
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
Attachment #477147 - Flags: review?(robert.bugzilla) → review-
Assignee

Comment 66

9 years ago
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.
Attachment #476914 - Flags: review?(robert.bugzilla)
Assignee

Updated

9 years ago
Attachment #474812 - Attachment is obsolete: true
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 <chanial@noos.fr>
 #   Dean Tessman <dean_tessman@hotmail.com>
 #   Johnathan Nightingale <johnath@mozilla.com>
 #   Dão Gottwald <dao@mozilla.com>
 #   Ehsan Akhgari <ehsan.akhgari@gmail.com>
 #   Robert Strong <robert.bugzilla@gmail.com>
 #   Rob Campbell <rcampbell@mozilla.com>
 #   Patrick Walton <pcwalton@mozilla.com>
+#   David Dahl <ddahl@mozilla.com>
 #
 # 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
Attachment #476914 - Flags: review?(robert.bugzilla) → review+
Assignee

Updated

9 years ago
Whiteboard: [strings][has WIP patch] → [strings] [eta 9-22-2010]
Assignee

Updated

9 years ago
Whiteboard: [strings] [eta 9-22-2010] → [strings] [eta 9-21-2010]
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.
Attachment #476914 - Flags: ui-review?(faaborg) → ui-review-
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 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.
Attachment #476914 - Flags: ui-review- → ui-review+
FWIW, IE uses Add-ons Disabled terminology.
Assignee

Comment 72

9 years ago
Fixed faaborg's and rs' review comments
Attachment #423442 - Attachment is obsolete: true
Attachment #476914 - Attachment is obsolete: true
Attachment #477377 - Flags: review+
Assignee

Comment 73

9 years ago
Rebased to current trunk, pushing to try
Attachment #477377 - Attachment is obsolete: true
Attachment #477382 - Flags: review+
Assignee

Updated

9 years ago
Attachment #477382 - Attachment description: v 0.7 Safe Mode patch for checkin [rebased] → v 0.7 Safe Mode patch for checkin [rebased] Patch 1 of 2
Assignee

Comment 74

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

Updated

9 years ago
Whiteboard: [strings] [eta 9-21-2010] → [strings] [strings patch landed] [functionality landed] [need nsis patch only]
Assignee

Updated

9 years ago
Flags: in-testsuite?
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

9 years ago
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") ?
Assignee

Comment 77

9 years ago
(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.
Assignee

Comment 78

9 years ago
(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.
Assignee

Comment 79

9 years ago
Opened a spin off for the nsis patch: bug 598779
Assignee

Updated

9 years ago
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [strings] [strings patch landed] [functionality landed] [need nsis patch only] → [strings] [strings patch landed] [functionality landed]
Target Milestone: --- → Firefox 4.0b7
(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
Flags: in-litmus?
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

9 years ago
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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Ehsan, the removal of the shortcuts will happen in bug 598779
Depends on: 598779
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.
Resummarizing; we'll deal with the shortcuts and such in bug 598779
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
Summary: Replace safe mode shortcut with keyboard modifier and menu command → Add menu command to restart in safe mode
(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
Depends on: 616836

Comment 87

9 years ago
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.
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.
How's about adding a 'restart in safe mode' button to about:support as suggested by bug 547623?

Comment 90

9 years ago
(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".

Updated

8 years ago
Depends on: 640677

Comment 92

4 years ago
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]
You need to log in before you can comment on or make changes to this bug.