No options for Safe-Mode

RESOLVED FIXED in seamonkey2.1b3

Status

SeaMonkey
Startup & Profiles
--
enhancement
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: Sascha Grage, Assigned: InvisibleSmiley)

Tracking

(Blocks: 1 bug)

Trunk
seamonkey2.1b3

Firefox Tracking Flags

(blocking-seamonkey2.1 needed, seamonkey2.1 wanted)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

7 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.1; WOW64; en-US; rv:1.9.3a6pre) Gecko/20100620 Minefield/3.7a6pre NOT Firefox/3.6
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.1; WOW64; en-US; rv:1.9.3a6pre) Gecko/20100620 Lightning/1.1a1pre SeaMonkey/2.1a2pre

Seamonkey has no options for the safe-mode.

Reproducible: Always

Steps to Reproduce:
1. Start Seamonkey with -safe-mode 
2.
3.
Actual Results:  
No dialog with options.

Expected Results:  
Dialog with options
http://www.firefox-browser.de/MediaWiki/images/d/db/AbgesicherterModus.png
(Assignee)

Updated

7 years ago
Status: UNCONFIRMED → NEW
Component: General → Startup & Profiles
Ever confirmed: true
QA Contact: general → profile-manager
Version: unspecified → Trunk

Updated

7 years ago
status-seamonkey2.1: --- → ?
Ok heres the intersting bits.

Using a debug build as my testbed (since trunk opt is building now): |seamonkey -safe-mode| && |seamonkey --safe-mode| spawned a no-restart SeaMonkey with the _SIDEBAR_ open. I closed it and re-ran, each time sidebar open.

|seamonkey| did spawn a restart and did not have sidebar open.

I also did get the set-as-default prompt each time (I don't have my trunk builds as default).
blocking-seamonkey2.1: --- → ?
status-seamonkey2.1: ? → wanted

Comment 2

7 years ago
Not blocking a3 on it but we should try to get it into one of the betas.
blocking-seamonkey2.1: ? → b1+

Comment 3

7 years ago
Currently that dialog can't disable all addons for firefox trunk. At least for me.

Updated

7 years ago
blocking-seamonkey2.1: b1+ → needed
Depends on: 207763

Updated

7 years ago
No longer depends on: 207763
(Assignee)

Comment 4

7 years ago
TB porting FF's dialog in bug 641781.
(Assignee)

Updated

7 years ago
Assignee: nobody → jh
Status: NEW → ASSIGNED
(Assignee)

Comment 5

7 years ago
Created attachment 523624 [details] [diff] [review]
patch

Neil, if review takes some time, please give at least a general direction whether the strings are acceptable so I can create an extra patch to make the string freeze.
Attachment #523624 - Flags: review?(neil)
Attachment #523624 - Flags: feedback?(philip.chee)

Comment 6

7 years ago
Some brief comments:
> +  // Remove the pref-overrides dir, if it exists.
> +  try {

We don't have Bug 364297 Change default home page search and default search engine for Fx 2.0 series.

> +<!ENTITY resetToolbars.label          "Reset toolbars and controls">

Not sure what "controls" are but deleting local store deletes all persistent attributes. I don't know how to word it in a user facing way.
(Assignee)

Comment 7

7 years ago
(In reply to comment #6)
> We don't have Bug 364297 Change default home page search and default search
> engine for Fx 2.0 series.

Ah, I see. Still maybe we should leave it in here so this part is ready should we ever use it. But I'll leave that decision to Neil.

> > +<!ENTITY resetToolbars.label          "Reset toolbars and controls">
> 
> Not sure what "controls" are but deleting local store deletes all persistent
> attributes. I don't know how to word it in a user facing way.

FWIW I'd associate "controls" with toolbar items, but in that case it would be pretty redundant here.

AFAIK local store is also the place where window sizes are remembered, right? So maybe "Reset toolbars and size settings"?

Comment 8

7 years ago
> Ah, I see. Still maybe we should leave it in here so this part is ready should
> we ever use it. But I'll leave that decision to Neil.

I suggested over IRC that we add a comment along the lines of:
//XXX This code currently doesn't do anything unless the rest of Bug 364297 is ported.
To prevent some developer from "optimizing" this code away later on.

> AFAIK local store is also the place where window sizes are remembered, right?
[I think "controls" here mean the window controls so window size and position.]

> So maybe "Reset toolbars and size settings"?
Any and every attribute that is persisted. (XUL persist= attribute and JS document.persist(elementID, attributeName). e.g. "Reset toolbars and settings other than those stored in user preferences"

Comment 9

7 years ago
Comment on attachment 523624 [details] [diff] [review]
patch

>+   content/communicator/safeMode.js                                 (safeMode.js)
>+   content/communicator/safeMode.xul                                (safeMode.xul)
Top-level files have their own section.

>+  document.getElementById("tasks")
>+          .addEventListener("CheckboxStateChange", UpdateOKButtonState, false);
<vbox id="tasks" oncommand="UpdateOKButtonState();">

>+function UpdateOKButtonState() {
>+  document.documentElement.getButton("accept").disabled = 
>+    !document.getElementById("resetUserPrefs").checked &&
>+    !document.getElementById("deleteBookmarks").checked &&
>+    !document.getElementById("resetToolbars").checked &&
>+    !document.getElementById("disableAddons").checked &&
>+    !document.getElementById("restoreSearch").checked;
[I wonder whether !getElementsByAttribute("checked", "true").item(0) works]

>+<!ENTITY % platformCommunicatorDTD SYSTEM "chrome://communicator-platform/locale/platformCommunicatorOverlay.dtd">
Which entity are we using from here?

>+            buttonlabelextra1="&continueButton.label;"
It would be nice if this button could have initial focus.

>+            ondialogcancel="onCancel();"
Cancel normally tries to close the dialog, so we need this to return false.
[Although I'm not sure whether it would be neater for the caller to restart.]

>   _onProfileStartup: function()
>   {
>+    // check if we're in safe mode
>+    if (Services.appinfo.inSafeMode) {
>+      Services.ww.openWindow(null, "chrome://communicator/content/safeMode.xul", 
>+                             "_blank", "chrome,centerscreen,modal,resizable=no", null);
[I assume this is early enough.]
>+    }
>+
>     this._updatePrefs();
>     migrateMailnews(); // mailnewsMigrator.js
> 
>     Sanitizer.checkAndSanitize();
So, the problem here is that even if you want to restart, profile startup still gets to finish. One fix might be to use a different notification, such as profile-after-change.

Comment 10

7 years ago
(In reply to comment #8)

> > So maybe "Reset toolbars and size settings"?
> Any and every attribute that is persisted. (XUL persist= attribute and JS
> document.persist(elementID, attributeName). e.g. "Reset toolbars and settings
> other than those stored in user preferences"

I think it would be very difficult to come up with something that would fit into a single line - I guess "Reset toolbars and persisted attributes" is still not very user-friendly.
Maybe we just accept nothing will be user-friendly and just keep it fairly short and explain it in help pages.
(Assignee)

Comment 11

7 years ago
I'll concentrate on l10n issues for now.

(In reply to comment #9)
> >+<!ENTITY % platformCommunicatorDTD SYSTEM "chrome://communicator-platform/locale/platformCommunicatorOverlay.dtd">
> Which entity are we using from here?

quitApplicationCmd.label

(In reply to comment #10)
> I think it would be very difficult to come up with something that would fit
> into a single line

I think we don't have to take care of every single case this affects but may instead concentrate on the common/most visible things this affects. IMO that's toolbars (including their contents) and window sizes, but I'm not a localstore expert so maybe there's more.

> I guess "Reset toolbars and persisted attributes" is still not very
> user-friendly.

Right.

> Maybe we just accept nothing will be user-friendly and just keep it fairly
> short and explain it in help pages.

Agreed, but keep in mind that Help cannot/should not be called this early, and that it's hard to predict when this particular topic would be written (given the nature of our development environment).
Reset toolbars, window sizes and other miscellaneous settings?
(Assignee)

Comment 13

7 years ago
(In reply to comment #12)
> Reset toolbars, window sizes and other miscellaneous settings?

Apart from being a bit long it's also, a little too vague. A bit scary, actually, like "may affect anything, even things you didn't know existed". I think we should really look at this from a user's perspective rather than trying to be exact or all-encompassing (I think we'd fail on both anyway).
(Assignee)

Comment 14

7 years ago
Created attachment 523833 [details] [diff] [review]
patch v2 [Checkin: comment 18]

(In reply to comment #9)
> [I wonder whether !getElementsByAttribute("checked", "true").item(0) works]

It does.

> >+            ondialogcancel="onCancel();"
> Cancel normally tries to close the dialog, so we need this to return false.
> [Although I'm not sure whether it would be neater for the caller to restart.]

I don't see any difference in behavior (tried Esc and the window's close button), in both cases the application closes (i.e. startup is interrupted).

> >   _onProfileStartup: function()
> >   {
> >+    // check if we're in safe mode
> >+    if (Services.appinfo.inSafeMode) {
> >+      Services.ww.openWindow(null, "chrome://communicator/content/safeMode.xul", 
> >+                             "_blank", "chrome,centerscreen,modal,resizable=no", null);
> [I assume this is early enough.]

For showing the dialog before any other window opens? Then Yes.

> So, the problem here is that even if you want to restart, profile startup
> still gets to finish. One fix might be to use a different notification, such 
> as profile-after-change.

Unfortunately I don't know how to check whether that makes any difference, but I tried to apply what you suggested (and of course checked that the parts I could observe still work).
Attachment #523624 - Attachment is obsolete: true
Attachment #523833 - Flags: review?(neil)
Attachment #523624 - Flags: review?(neil)
Attachment #523624 - Flags: feedback?(philip.chee)
(In reply to comment #14)
> > >+            ondialogcancel="onCancel();"
> > Cancel normally tries to close the dialog, so we need this to return false.
> > [Although I'm not sure whether it would be neater for the caller to restart.]
> I don't see any difference in behavior (tried Esc and the window's close
> button), in both cases the application closes (i.e. startup is interrupted).
Just for consistency, since "accept" prevents the window from closing (except as part of the restart process).
Comment on attachment 523833 [details] [diff] [review]
patch v2 [Checkin: comment 18]

>+    !document.documentElement.getElementsByAttribute("checked", "true").item(0);
[IIRC document has getElementsByAttribute, or you could use the vbox]

>         break;
>+      case "profile-after-change":
>+        this._onProfileAfterChange();
>       case "final-ui-startup":
:-(

>+    Services.obs.addObserver(this, "profile-after-change", false);
[I wonder whether it's better to use the category method]
Comment on attachment 523833 [details] [diff] [review]
patch v2 [Checkin: comment 18]

r=me with changes as discussed on IRC.
Attachment #523833 - Flags: review?(neil) → review+
(Assignee)

Comment 18

7 years ago
Comment on attachment 523833 [details] [diff] [review]
patch v2 [Checkin: comment 18]

http://hg.mozilla.org/comm-central/rev/9d9a25c4364d
with comment 16 points 1+2 addressed
Attachment #523833 - Attachment description: patch v2 → patch v2 [Checkin: comment 18]
(Assignee)

Updated

7 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.1b3
(Assignee)

Updated

7 years ago
Blocks: 646897
(In reply to comment #16)
>>+    Services.obs.addObserver(this, "profile-after-change", false);
>[I wonder whether it's better to use the category method]
FYI, this works as follows:
1. Remove the explicit calls to add/removeObserver
2. Add an extra line to SuiteCommon.manifest
This article targets extensions but it demonstrates the idea.
https://developer.mozilla.org/en/XPCOM/XPCOM_changes_in_Gecko_2.0#What_you_need_to_change
(Note that as an app component we still want app-startup registration.)
You need to log in before you can comment on or make changes to this bug.