Open Bug 477322 Opened 11 years ago Updated 6 years ago

Way too easy to hit the wrong button on about:sessionrestore

Categories

(Firefox :: Session Restore, defect, major)

defect
Not set
major

Tracking

()

REOPENED
Tracking Status
blocking2.0 --- -

People

(Reporter: Waldo, Unassigned)

References

(Depends on 1 open bug)

Details

(Keywords: dataloss, Whiteboard: [strings][not-ready-for-cedar])

Attachments

(1 file, 4 obsolete files)

Nothing other than the text of the buttons distinguishes them, so if you skim and misread you hose yourself.  Possible ideas for improvement:

* make the "Restore session" button larger
* make the "Start new" button not a button (e.g. a small-size link like cert errors show)
* align one button to left, one to right so spatial memory can be more helpful
* hide the "Start new" behind a disclosure widget, a la cert error again

The previous dialog also had this basic problem as well, although of course the layout that caused the problem was rather different.
Flags: blocking-firefox3.1?
No dataloss issue: you can always either navigate Back or enter about:sessionrestore again after accidentally clicking "Start New Session".
Keywords: dataloss
Version: unspecified → Trunk
That might be true, but I think it's far from obvious.
Not a blocker.

> * make the "Restore session" button larger
> * align one button to left, one to right so spatial memory can be more helpful

These are good ideas. I think I prefer the latter, really, and it seems pretty easy to do. I don't see the need to make one button larger than the other.

> * make the "Start new" button not a button (e.g. a small-size link like cert
> errors show)
> * hide the "Start new" behind a disclosure widget, a la cert error again

I don't think either of these are appropriate. Especially since in the all-too-possible case that the user is experiencing a recurring crash (remember: they now only get here when there have been two crashes in a row) starting a new session might be an extremely reasonable option. The analogy to the certificate error doesn't really fit, as in that case we're trying to avoid a "whatever" response.

(removing ui-wanted, please consider the above your response!)
Flags: blocking-firefox3.1? → blocking-firefox3.1-
Keywords: uiwanted
> * make the "Restore session" button larger
> * align one button to left, one to right so spatial memory can be more helpful

Probably the most effective way to differentiate the buttons is the same way it is done in dialog boxes, with one of them being visually presented as the default choice.  Changing the size or position both seem sort of strange, and feel like inventing new solutions to an otherwise addressed problem.
(In reply to comment #4)
> with one of them being visually presented as the default choice.

Which already happens, doesn't it?
(In reply to comment #5)
> (In reply to comment #4)
> > with one of them being visually presented as the default choice.
> 
> Which already happens, doesn't it?

Right. "Restore" is the default choice.

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2a1pre) Gecko/20090528 Minefield/3.6a1pre ID:20090528031402
Assignee: nobody → michaelkohler
Status: NEW → ASSIGNED
Attached patch WIP (obsolete) — Splinter Review
* this doesn't work yet. The right button is not aligned right (but is not because of |#buttons button:nth-of-type(2)|)... Any hint?
* is this going toward the right direction?
Comment on attachment 380234 [details] [diff] [review]
WIP

>+  -moz-margin-end: 0px;

s/px/em/
Another thought, if we move the buttons apart they lose their mapping as being a choice between two options, and instead present themselves as two independent tools.  Sp this has the overall effect of making it hard for the user to realize they are even being asked a question.  This is compounded by the fact that we have morphed an otherwise modal dialog box into an informational page.

I think the default control styling is the most effective way to solve the UI problem.
(In reply to comment #9)
> I think the default control styling is the most effective way to solve the UI
> problem.

What do you mean with "default control styling"?
On OS X it the default button is dark blue (and in other apps that support widget animation) pulsing.  On Vista the default button gets a light blue glow.  XP just adds a blue line around the edge of the default button (with Luna), etc.
This means that #errorTryAgain needs default="true" and for platforms other than OS X, this styling will have to be dynamically removed, whenever a different button gets the focus - as we already do for dialogs:
http://hg.mozilla.org/mozilla-central/annotate/48ffab521d3f/toolkit/content/widgets/dialog.xml#l409
Attached patch v1 (obsolete) — Splinter Review
>-  
>+

This removes two unnecessary spaces.
Attachment #380234 - Attachment is obsolete: true
Attachment #381941 - Flags: review?(zeniko)
Attached patch v1.1 : comment edited (obsolete) — Splinter Review
I guess the comment for the event listener is better like that.
Comment on attachment 381941 [details] [diff] [review]
v1

>+  // focus event listener for platforms other than OS X to make sure the styling is
>+  // removed dynamically if another button gets the focus
>+#ifndef XP_MACOSX

Please drop the bit about what platforms it applies to (that's obvious) and move the comment inside the #ifndef (it's not needed on OS X).

>+  document.getElementById("errorTryAgain").addEventListener("focus", function() {
>+    var defaultButton = document.getElementById("errorTryAgain");
>+    if(defaultButton)
>+      defaultButton.setAttribute("default", this.originalTarget == defaultButton);
>+  }, false);

This doesn't work. Please include a passing test to ensure it does.

The spec is: Mark the button as default, unless another button is selected (and we're not on OS X).
Attachment #381941 - Flags: review?(zeniko) → review-
So you're removing default="true" from the Restore button when the other button has focus, because Enter won't activate the Restore button in that case. Correct? But Enter won't active that button whenever it loses focus, regardless of whether focus goes to the second button.
(In reply to comment #16)

That's right, that's what I'm gonna do now. Thanks.

Simon (is it okay to call you Simon here on bmo?): do I need to "connect" Enter with the Restore button or is it okay to just remove default="true" when #errorCancel has focus?
I'm not sure you understood comment 16. I think there's a problem with Simon's specification.
Simon: could you please explain what is to do exactly?
(In reply to comment #16)
> So you're removing default="true" from the Restore button when the other button
> has focus, because Enter won't activate the Restore button in that case.

No, we're removing default="true" in this case because otherwise two buttons would *look* active. Changing the behavior of the Enter key is a related but different bug.
(In reply to comment #20)
> No, we're removing default="true" in this case because otherwise two buttons
> would *look* active.

That would be on Windows, where default="true" just gets the focused appearance. That's not the case on Linux.

> Changing the behavior of the Enter key is a related but
> different bug.

Adding default="true" without adjusting the behavior of the Enter key would /introduce/ a bug.
(In reply to comment #20)
> (In reply to comment #16)
> > So you're removing default="true" from the Restore button when the other button
> > has focus, because Enter won't activate the Restore button in that case.
> 
> No, we're removing default="true" in this case because otherwise two buttons
> would *look* active. Changing the behavior of the Enter key is a related but
> different bug.

Okay, so I'll write a patch that removes default="true" when the other button has focus. And for the Enter bug I'll file an other bug report which I'll fix after this one.
Enter is consumed in the tree, which probably shouldn't be changed. So you might not be able to solve that issue, and default="true" might be the wrong approach for this bug.
(In reply to comment #23)
> Enter is consumed in the tree, which probably shouldn't be changed. So you
> might not be able to solve that issue, and default="true" might be the wrong
> approach for this bug.

what about making sure that Enter just calls restoreSession() if nothing in the tree is focused?

any other possible propose how to fix this bug?
(In reply to comment #24)
> what about making sure that Enter just calls restoreSession() if nothing in the
> tree is focused?

As soon as the tree has focus, the selected row will have focus as well, and there's always a selected row. The interesting question is whether it's an expandable container (where Enter will be consumed) or a tab (where Enter won't be consumed).

> any other possible propose how to fix this bug?

see comment 3
(In reply to comment #25) 
> > any other possible propose how to fix this bug?
> 
> see comment 3

see comment 9 which is an argument against different alignment.
What is the best way to go?
Whiteboard: [needs decision what to do]
I would suggest something different: I recently saw a Safari Session Restore window (some plugin) which had the buttons "Cancel" and "OK", aligned to the right. It actually is a dialog box popping up before the browser is started, but after thinking about it I think this would also be a good way for Firefox, even when session restore is not a modal dialog box, along with replacing the heading "Well, this is embarrassing." by "Restore Session" (as it also is in the page title).

Why?
 - When the user reads the headline "Restore session", the meaning of the options "OK" and "Cancel" is obvious
 - Even if "Start New Session" and "Restore" say more explicitly what is done, "Start New Session" reads like an "OK" message which I think is one of the reasons I am clicking it unintentionally
 - There is a default layout for "OK" and "Cancel" buttons that every user is familiar with
 - The heading "Well, this is embarassing." might be true but is not what the user needs to read when first seeing this page and quickly deciding which button to click. The sentence could be included in the text paragraph below.
Alex, do you have any further thoughts on this?
We are preparing a lot of UI work that leverages a single action button (new notification system [1], content page to temporarily disable third party plugins, etc.).  So I think keeping about:session restore internally consistent with a single action button is the best overall approach.

[1] http://people.mozilla.com/~faaborg/files/20090821-notification/newNotification-i1.png
(In reply to comment #31)

So how should that button look exactly? (wording)

I thought of something similar to this:

----------------------
| Restore         | v | <---- Button with dropdown arrow
--------------------------
| Restore                 | <---- Menuitem if clicked on arrow
---------------------------
| Start new Session       | <---- Menuitem if clicked on arrow
---------------------------

When clicking on the button (not the arrow), Firefox should restore the session, right?
Whiteboard: [needs decision what to do]
.. or actually it would be better to display "Start new session" as first item in the dropdown menu to fix this issue.

These kind of buttons are not implemented yet, right? What's the bugnumber of this UI enhancement?
button type="menu" with some styling should do, no? Or maybe there needs to be a new widget created to express this separation of button and menu [button|v] ?
(In reply to comment #33)
> .. or actually it would be better to display "Start new session" as first item
> in the dropdown menu to fix this issue.

Err, no. Than it would be way too easy to lose all your tabs.

If that single button with "Restore" as the default option doesn't fix this bug, then maybe it's not the right solution.
(In reply to comment  
> > in the dropdown menu to fix this issue.
> 
> Err, no. Than it would be way too easy to lose all your tabs.
> 
> If that single button with "Restore" as the default option doesn't fix this
> bug, then maybe it's not the right solution.

I disagree. If theuser wants to restore  the session he just clicks on the button without using the dropdown menu. If he wants to start a new session it's better to not need to move the mouse over the restore menuitem.
I was assuming that the option on top of the list is also the default option.
I'm fine with us not using a split button (bug 509642) in this particular case, but if we want to, we should have it change its state to "Start New Session" in the event that the user unchecks everything.  Also, I'm pretty sure these buttons are only supposed to expose additional actions in the drop down (so not including the main action).
Depends on: 509642
Worse than "Well, this is embarassing" is the text "Firefox is having trouble recovering you windows and tabs..."  It's inappropriate for those people who prefer to have browser.sessionstore.max_resumed_crashes set to 0.
We don't design for about:config customizations, the whole point of about:config is giving some technical users extra control without going through all of the work and boundary cases involved in creating an actual pref in the UI.
This bug has gone off into the weeds. Let's pull it back. I suggest filing a new bug for modifying this dialog so that it uses a compound button, since that's predicated on other parts of our UI using those types of buttons.

This bug is about the fact that the button text and placements are confusing users. To solve that issue, I would suggest:

 - changing the text of "Start New Session" to "Close" (probably needs a new bug so we don't get held up on string freeze)
 - moving the "Restore" button so that it is always beneath the "Restore" column

The close button should close the session restore window and replace it with the user's home page.

This will break slightly with the OSX consistency of always putting the "passive" option on the far right (ie: Cancel / OK ) but has the benefit of being cleaner in terms of information hierarchy as there is strong alignment with the options for restoring.

(In reply to comment #39)
> Worse than "Well, this is embarassing" is the text "Firefox is having trouble
> recovering you windows and tabs..."  It's inappropriate for those people who
> prefer to have browser.sessionstore.max_resumed_crashes set to 0.

File a separate bug for that, if you wish. Suggest new strings. While I agree with Alex that it's not a priority, it also shouldn't be hard for someone interested to come up with a patch, and the community who're adjusting those prefs are the type who like writing patches, too.
^ I have filed bug 523137 and bug 523140 for this.
Going back to comment #9, if we use the correct default button styling (default="true") the user should have an easier time finding the control to restore their session.  In any case where we have two buttons (dialogs, content area, etc.) the expected path should be styled as the default choice.
(In reply to comment #43)
> Going back to comment #9, if we use the correct default button styling

Yes, you've mentioned. Is there a bug on supporting this in our toolkit? By all means, let's do that as well.

I don't think that alone will solve this issue, though. At a glance, it's far too simple to mistake "Start New Session" with "get back to what I was doing." I think we should make it clear that the in-content page is about restoring your session, and that your alternative is to close that page at which point you have declined to restore any part of that session.
Right I'm not really disagreeing, just chiming in so that we don't forget that small fix as well (I was assuming we just didn't bother to set a default button, we need to check if the issue is that it doesn't render as default while in content).
Please feel free to take this bug.
Assignee: michaelkohler → nobody
Status: ASSIGNED → NEW
Duplicate of this bug: 527610
Depends on: 527610
One more suggestion. Add a menu item under History for something like "Recently Closed Sessions". If this isn't a data loss issue, then implementing that should be easy, right?
Duplicate of this bug: 530022
The Launchpad user wanted to mention Fitts's Law with regard to this as well.  Didn't see it mentioned yet.
Adding my 2 cents:

Solution:
Restore button should be on the left, not the right.

Reason:
Just ran down the list of my open tabs, dis-selected the ones I did not want, flowed straight down and hit the button that was along the same vertical line of activity: Start New Session.
I think it would be appropriate to put a note in the crasher page to alert people that if they hit start new session by accident, they can simply hit the back button if they change their mind.
Attached image An idea... (obsolete) —
So I was thinking about this the other day. I got to thinking how there are a lot of forms on the web that have a "default" action as a button, and then the less desirable option as a link. Often this shows up on edit forms - "[Save] or _Cancel_". I know it's really common with 37signals products (and in turn ruby on rails based sites since it's in the default forms).

So I decided to mock something like that up here. I know it's standard practice for us to use buttons as action items, but since this is a chrome-in-content page, I thought it would be worth exploring treating it a bit more like content design-wise.

Thoughts?
Attachment #449078 - Attachment is patch: false
Attachment #449078 - Attachment mime type: text/plain → image/png
Seems reasonable to me.  I'd suggest, by analogy to the various bad-page bits we have, that the link should be opposite-aligned (or more spatially separated, somehow) from the button to further distinguish the two:

http://www.mozilla.com/firefox/its-a-trap.html
http://www.mozilla.com/firefox/its-an-attack.html

(I wonder whether this comment's bugmail will show up as dangerous in Thunderbird due to these links?)
It seems like a hack to me to make an action item like "Start New Session" a link, but I actually think it's a great idea, because a link would imply to the user that they can use the back button if they clicked on it by mistake.
Comment on attachment 449078 [details]
An idea...

Worth also noting that this approach would be anti-os-specific ordering. Currently on OSX, "Restore" is to the right of "Start New Session", but we wouldn't want to do that with one of them being a text link instead.

An additional benefit: links imply that the action is undoable. Buttons have the opposite implication. Since "Start New Session" is actually undoable, this might help give users who don't realize that now a little hint.
This feels a bit too much like web design (which makes sense for a 37signals product) as opposed to browser chrome.  OSs already have a solution to this problem, styling the default button (usually blue).
Speaking only for myself, I would find the OS styling to be insufficient to prevent me from making this mistake.  I find this manner of OS styling to be helpful when interacting with interfaces using the keyboard, but in the failure case here I'm using a mouse.
Nominating blocking1.9.3.

This bug is important. Firefox has a reputation for being crashy. Against that, we've sunk a ton of effort into Session Restore so we can recover as gracefully as can be expected. This single, tiny, easy-to-fix UI gaffe not only wipes out all that hard work, it piles frustration on top of frustration in the user's experience.

It won't take a lot of effort to fix this, either. Several good ideas have been suggested.
blocking2.0: --- → ?
Effort?  About 2 minutes of any competent coder's time to switch the behavior of the buttons.  It's the 95 committees debating the bug that have stalled this fix. And oh so many others.
Yes, this bug shouldn't be at 62 comments.  All of our new notification work is based on a single button for "yes", and the user ignoring and moving on for "eh" (where "eh" means probably no, but I can go back and say yes later if I feel like it).  There are a lot of reasons why we think this is better, no forced choice, supports undo, etc.  And I think all of those ideas apply here as well.

Single "Restore Session" button: restores the session

Closing the tab, opening a new tab, navigating this tab somewhere else, etc: starts a new session
> Closing the tab

We don't allow this.
right, because it is the first and only tab.  Either way, the user has a number of controls to indicate that they want to move on, including new tab, location bar, search bar, bookmarks widget, etc.
Attachment #381941 - Attachment is obsolete: true
Attachment #381943 - Attachment is obsolete: true
Attachment #449078 - Attachment is obsolete: true
an additional tweak based on feedback by zpao: we should have the first tab be the user's home page (or set of home page tabs), and the second active tab be about:session restore.  This way the message does have a close button on the tab (since there is more than one tab) and there is even one more target for the user to indicate that they aren't interested in restoring right now.

We will end up doing this implicitly when we load the home tab and other application tabs, so we could just land this knowing that those other targets are going to be available later. (or we could add the home page now, really either way).
If arbitrary pages are opened after a crash, it partially defeats the point of not automatically opening the entire session. The home page(s) could be the reason why the browser just crashed.
I agree with the previous comment, also keep in mind that users might have multiple home pages.
yeah good point.  Either way later when we get a locally hosted home tab we can be pretty sure that our own code isn't causing the crash (or if it is it's a problem bigger than session restore).
Thanks for pointing me to this bug, Paul. Sorry I didn't see it earlier but I'm new and don't know how to use Bugzilla very well.

It seems to me that the technically correct arrangement of buttons is already in place, but we're still having problems with:

a) people clicking the wrong button
b) communicating you can go back if you pressed the wrong button

I propose this: http://imgur.com/mP3Qf.png

The message appears only when you select "Start New Session". It simultaneously informs the user that you can go back to that page whenever you want and gives them a way to do so quickly if they didn't mean to start a new session.
I'd prefer removing the "Start New Session" button entirely, but that might confuse the user a bit if they didn't know they could still type in a web address in the location bar.
Not breaking or preventing major functionality, and not a regression, so not blocking on this.

However, the fix is very low risk, would definitely take it once UI-R+'d and reviewed.

Alex, if this patch is approach you agree with (sounds like it from comment 62) can you UI-R+ it?
blocking2.0: ? → -
Attachment #451682 - Flags: review?(paul)
Comment on attachment 451682 [details] [diff] [review]
remove "Start New Session" button

I'm assuming the mozmill stuff is fine. Maybe it would be good for Anthony or Henrik to take a quick look.
Attachment #451682 - Flags: review?(paul) → review+
Attachment #451682 - Flags: approval2.0?
Comment on attachment 451682 [details] [diff] [review]
remove "Start New Session" button

Requesting additional review from Henrik per Zpao's comment.
Attachment #451682 - Flags: review?(hskupin)
Comment on attachment 451682 [details] [diff] [review]
remove "Start New Session" button

Thanks Paul for the patch. It looks fine so far. But can you also create the patch against our mozmill-tests repository? The copy we have on mozilla-central is a bit outdated and needs a new sync in the next days once mozmill-1.4.2 has been released. All the current work happens on http://hg.mozilla.org/qa/mozmill-tests/. Thanks!
Attachment #451682 - Flags: review?(hskupin) → review+
Assignee: nobody → gavin.sharp
Status: NEW → ASSIGNED
Attachment #451682 - Flags: approval2.0? → approval2.0+
Whiteboard: [strings]
http://hg.mozilla.org/mozilla-central/rev/860c490eb849
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0b4
FTR, this was pretty confusing and surprising to me when I saw the change today; not sure if some of that will be resolved in zpao's follow-up work or not.

Alex: I'm assuming your comment 62 meant to imply that the recommended action here is always to restore the session, so that's the Very Obvious Path. As Dao said in comment 63, the user can't actually close this tab in the usual case of restoring-from-crash, so it's actually now pretty hard to get to a new, blank session.
>As Dao said in comment 63, the user can't actually close this tab in the usual >case of restoring-from-crash, so it's actually now pretty hard to get to a new, >blank session.

If we don't get a Home Tab landed, then we should probably go back to treating this tab like a modal dialog box.  Ideally though the user should have a variety of targets to move forward (home tab, new tab, tab close button).
In that case, I think the easiest thing to do is to back out this change until some of those other dependencies have landed.
I was going to suggest the same. Backed out:
http://hg.mozilla.org/mozilla-central/rev/0868c30effb1
http://hg.mozilla.org/mozilla-central/rev/9599efaaa3d8
Assignee: gavin.sharp → nobody
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: Firefox 4.0b4 → ---
Attachment #451682 - Flags: approval2.0+
Whiteboard: [strings] → [strings][not-ready-for-cedar]
I use FireFox on Mac & Windows.  Because the buttons are back to front compared to each other I am constantly pressing the wrong button.  Especially since FF has been crashing so often lately.  Anything would be better than the default buttons.
Perhaps it’s time to attempt removing the Start New Session button again, or replacing it with a link? Both are good ideas. Both would have saved the 10 or so Panorama groups, each with lots of tabs, that I have now lost, apparently irreversibly.

("Back" no longer has any effect on any of the tabs I still have; "about:sessionrestore" is blank.)

This UI issue + Panorama = major data loss.
Severity: normal → major
Keywords: dataloss
See also bug 639992 for making it less catastrophic if you hit the wrong button (and know where to look).
You need to log in before you can comment on or make changes to this bug.