better safe mode UE

VERIFIED FIXED in mozilla1.8final

Status

()

Toolkit
Startup and Profile System
VERIFIED FIXED
13 years ago
9 years ago

People

(Reporter: mconnor, Assigned: mconnor)

Tracking

({fixed1.8})

unspecified
mozilla1.8final
fixed1.8
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.8b5 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 2 obsolete attachments)

(Assignee)

Description

13 years ago
brought up on yesterday's call, ongoing discussion with bsmedberg and beltzner.

This has two of four items implemented, the others are greyed out.  There's a
couple other non-critical tweaks here, but I'll work on those after branch.
(Assignee)

Comment 1

13 years ago
Created attachment 192482 [details] [diff] [review]
initial impl

String-complete, feature 50% implemented, bsmedberg and I each have one feature
to finish, but that can be done after branch as this doesn't involve any added
UI.
Attachment #192482 - Flags: approval1.8b4?
(Assignee)

Updated

13 years ago
Attachment #192482 - Flags: approval1.8b4?
(Assignee)

Updated

13 years ago
Flags: blocking1.8b4+
Whiteboard: [1.8 Branch ETA 8/11 Strings only are required for branch]
(Assignee)

Comment 2

13 years ago
Created attachment 192486 [details] [diff] [review]
more cleanup
Attachment #192482 - Attachment is obsolete: true
(Assignee)

Comment 3

13 years ago
Created attachment 192487 [details]
screenshot
(Assignee)

Updated

13 years ago
Attachment #192487 - Attachment is patch: false
Attachment #192487 - Attachment mime type: text/plain → image/png
(Assignee)

Comment 4

13 years ago
Created attachment 192489 [details] [diff] [review]
more cleanup

This adds a backup functionality to create bookmarks-backup.html if you choose
to reset bookmarks.  Prefs I'm not so worried about, but for bookmarks this
will be enhanced before beta to do the numerical incrementing a la dlmgr.

This has been tested heavily, and Beltzner has signed off on the strings.
Attachment #192486 - Attachment is obsolete: true
Attachment #192489 - Flags: approval1.8b4?

Comment 5

13 years ago
Comment on attachment 192489 [details] [diff] [review]
more cleanup

a=chase@mozilla.org
Attachment #192489 - Flags: approval1.8b4? → approval1.8b4+

Comment 6

13 years ago
See also my localstore patch in bug 303279

Comment 7

13 years ago
ok, I think there is a major misunderstanding here: when I read this dialog, I
assume that the checkboxes will work whether I click "Make Changes and Restart"
or "Continue in Safe Mode"... and that they *should* work that way. What reasons
do we have for making the "Continue in Safe Mode" button ignore the checkboxes?
Doesn't Safe Mode mean that all these things are always temporarily disabled?
That's what the strings were meant to reflect.

The task flow may be clearer if we made it so ...
  1. User invokes safe mode from shortcut
  2. Safe mode UI dialog opens (but browser doesn't) 
  3. IF user choses restart, then apply changes & restart in normal mode
  4. IF user choses proceed, then proceed into safe mode with temporary changes

Ideally, we'd have the [Continue in Safe Mode] button *above* the checkboxes,
making it clearer that this is an either/or proposition. I've been informed that
this is rocket science, though, and I should stop trying to make dialogs that
aren't standard 2 or 3 button ones. My creative style is seriously cramped :)

bsmedberg, what are the uses cases someone might have for entering Safe Mode but
leaving all of those boxes unchecked?

Other random thoughts:

 - the [Make Changes and Restart] button should only be enabled if at least
   one of the items is checked

 - while label changes are a pain, perhaps adding "Or, you can continue to work
in Safe Mode with all of these items temporarily disabled" would clear things up.

Updated

12 years ago
Blocks: 286589
(Assignee)

Updated

12 years ago
Blocks: 303279
Whiteboard: [1.8 Branch ETA 8/11 Strings only are required for branch] → [initial patch landed][remaining work ETA 8/19]

Comment 9

12 years ago
(In reply to comment #8)

> Ideally, we'd have the [Continue in Safe Mode] button *above* the checkboxes,
> making it clearer that this is an either/or proposition. I've been informed that
> this is rocket science, though, and I should stop trying to make dialogs that
> aren't standard 2 or 3 button ones. My creative style is seriously cramped :)
> 

I have to agree that from attachment 192487 [details] it is unclear whether Continuing
applies the changes temporarily, doesn't apply any changes, or will apply the
changes when the app is restarted.
(Assignee)

Comment 10

12 years ago
Not an RC blocker, leaving this on my list for now.  Should be safe to leave
this until after I get back, since this is waiting on a bsmedberg bug, and for
vlad to land the improved bookmarks backup code.
Whiteboard: [initial patch landed][remaining work ETA 8/19] → [initial patch landed][remaining work ETA 09/08]

Updated

12 years ago
Flags: blocking1.8b5+ → blocking1.8b5-

Comment 11

12 years ago
Asa, we at least need to fix the problem where the safe-mode window pops up
every time a new window is opened.
Flags: blocking1.8b5- → blocking1.8b5?
(Assignee)

Comment 12

12 years ago
Created attachment 197383 [details] [diff] [review]
localstore cleanup

implements deletion of localstore.rdf to reset toolbars/other customizations.

Also fixes the dialog-per-window issue, this should now open before the first
browser window.  I've been testing this in various combinations all weekend,
should work ok.

Need to talk to bsmedberg/Rob Strong about the safety of implemention disable
all in nsIExtensionManager.  This is probably a trunk improvement, but if we
can come up with a safe patch that sits beside other code, we should be able to
make it happen.  If we don't do this, I'll hide instead of disable the
checkbox.
Attachment #197383 - Flags: review?(bugs.mano)
Shouldn't the "Make changes and restart button" be disabled if no checkboxes are
checked?
(Assignee)

Comment 14

12 years ago
Yeah, I'll get that in part2, whatever form it takes.

Comment 15

12 years ago
Comment on attachment 197383 [details] [diff] [review]
localstore cleanup

Mike, why use a pref for this? We should be showing the safe-mode dialog from
an app-startup handler, not from browser window code at all.
Attachment #197383 - Flags: review?(bugs.mano) → review-
(Assignee)

Updated

12 years ago
Depends on: 310076
(Assignee)

Updated

12 years ago
Flags: blocking1.8b5? → blocking1.8b5+

Comment 16

12 years ago
If you come up with a very safe fix in the next couple of days, please request
approval for the patch and we'll evaluate.
Flags: blocking1.8b5+ → blocking1.8b5-

Comment 17

12 years ago
Asa, unless we're going to take the supplementary fix we should back out the
original patch. As it stands the behavior is simply unacceptable.
Flags: blocking1.8b5- → blocking1.8b5?
(Assignee)

Updated

12 years ago
Flags: blocking1.8b5? → blocking1.8b5+

Updated

12 years ago
Whiteboard: [initial patch landed][remaining work ETA 09/08] → [initial patch landed]

Comment 18

12 years ago
Mike, is part 2 coming soon?
(Assignee)

Comment 19

12 years ago
Asa, the new patch hasn't been tested on branch yet since bug 310076 hasn't yet
landed there, but its ready pending testing with bsmedberg's fix on branch.  Not
at risk, it'll make it in tonight assuming Benjamin lands soon.

Comment 20

12 years ago
bug 310076's simple patch is in. we've got 6 hours so hustle :-)
(Assignee)

Comment 21

12 years ago
Created attachment 198403 [details] [diff] [review]
the long-awaited part 2
Attachment #198403 - Flags: review?(bugs.mano)
Attachment #198403 - Flags: approval1.8b5?

Comment 22

12 years ago
Comment on attachment 198403 [details] [diff] [review]
the long-awaited part 2

r=me
Attachment #198403 - Flags: review?(bugs.mano) → review+

Comment 23

12 years ago
Comment on attachment 198403 [details] [diff] [review]
the long-awaited part 2

plussing for the branch after talking this over with mconnor.
Attachment #198403 - Flags: approval1.8b5? → approval1.8b5+
Depends on: 311014
(Assignee)

Comment 24

12 years ago
fixed branch and trunk, spun off bug 311015 for the trunk work on addon disabling.
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Keywords: fixed1.8
Resolution: --- → FIXED
Mike, when I don't check any of that options, close Firefox when it was started
and restart again, than I only get three checkboxes. The one for "Disable all
themes and extensions" will never be displayed as often as I restart Firefox. Is
it by design or do we have a issue left?

I'm using Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a1) Gecko/20051004
Firefox/1.6a1
(Assignee)

Comment 26

12 years ago
I switched that to hidden since it won't be implemented for 1.5.  I filed bug
311015 on supporting that.
Blocks: 311086
Blocks: 311158
We have some new entities here which aren't known by localizers, right?
Keywords: late-l10n
Whiteboard: [initial patch landed]
No, the strings were checked in before the l10n freeze.
Keywords: late-l10n
Depends on: 311496
Status: RESOLVED → VERIFIED
Target Milestone: --- → Firefox1.5

Comment 29

12 years ago
*** Bug 303282 has been marked as a duplicate of this bug. ***

Comment 30

12 years ago
I can see it's a FIXED bug but guys, how about PLUGINS? They also cause a lot of issues.
Product: Firefox → Toolkit
You need to log in before you can comment on or make changes to this bug.