593421 - Provide link to "Restore Previous Session" on Firefox Start

Status: VERIFIED FIXED

(Firefox :: General, defect)

Description

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Bug 588482 will enable restore on demand, but it's not the most visible feature, since the home tab isn't going to happen.

Dietrich and I talked about it & my idea was that we show a notification bar (or doorhanger) the first time you start Firefox with a session that could be restored (not one that is automatically restored).

Comment 1

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

I'd prefer if we had a link on Firefox Start that said:

"Restore previous session (N windows, M tabs)"

For people not using Firefox Start, I think an entry in the History menu is sufficient, like we do currently.

Adding Mano to CC, since he's the one that has done work on Firefox Start, and he can probably comment on the feasibility of adding this.

Comment 2

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

Great, Bugzilla ate my CC. :P

Mano:
> I'd prefer if we had a link on Firefox Start that said:
> 
> "Restore previous session (N windows, M tabs)"
> 
> For people not using Firefox Start, I think an entry in the History menu is
> sufficient, like we do currently.
> 
> Adding Mano to CC, since he's the one that has done work on Firefox Start, and
> he can probably comment on the feasibility of adding this.

Comment 3

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

Requesting blocking, as this is a "perceived data loss" situation if we don't make it very obvious how to restore the session once bug 592822 lands (currently blocking.

Comment 4

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Pretty sure Marco did about:home... so CCing him.

That said, I know Faaborg & I had talked about putting a "restore previous session" button on the Panorama page. I don't know where that bug went but it might just be better to make this a meta bug and have separate bugs for the different ways of exposing this.

(In reply to comment #1)
> "Restore previous session (N windows, M tabs)"

The current API doesn't expose the previous state directly (we could check sessionstore.canRestorePreviousSession and look at sessionstartup.state). We could get it but things get a little complicated with app tabs. Or we can just expose a new API the shows this data after sessionstore has processed the state.

Comment 5

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

>I know Faaborg & I had talked about putting a "restore previous
>session" button on the Panorama page.

that's bug 589665

mockup: https://bug589665.bugzilla.mozilla.org/attachment.cgi?id=497934

Comment 6

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

We removed this feature because we believed that it wasn't something that was commonly used by people, and bug 592822 aims to remove the question at shutdown. 

I think this is a nice-to-have, but not a blocker. If we don't get a solution for Firefox 4, we can address it with educational snippets, and figure out a full approach in a future release.

Comment 7

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

(In reply to comment #6)
> We removed this feature because we believed that it wasn't something that was
> commonly used by people, and bug 592822 aims to remove the question at
> shutdown. 

Not quite, The rationale was: The question is asked at the wrong point, and forces you to evaluate whether you want to get back to what you were previously doing on the way out instead of on the way in.

> I think this is a nice-to-have, but not a blocker. If we don't get a solution
> for Firefox 4, we can address it with educational snippets, and figure out a
> full approach in a future release.

I don't think we should dismiss it until we have more solid information from Marco or someone else familiar with how hard it would be to do.

Comment 8

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

Morphing the title of this bug, since I think we have a workable and low-risk approach now.

Things that were considered, but we *aren't* pursuing right now:

* Make about:home privileged code, so clicking a link can restore your session — unlikely to happen, since it's too risky and changes too much, and would need security review, etc.

* Show a text similar to Work Offline (ugh) on Firefox Start saying "You can restore your previous session from the History menu" — really not what we want, but is technically the simplest thing we could do.

* Keep what we do now, ask on exit — we'd really like to avoid this. With our focus on startup speed and not getting in the way of the user, it seems wrong to present all our users with a dialog when they are shutting down their browser, especially since we already made the session saving enabled by default.



Instead, we recommend the following, given the limited time left, and to manage risk:

* Make Firefox Start have a link to about:sessionrestore, and change the default text — right now, its default state is the "OMG you crashed" messaging. 

* Paul mentioned that he could make the crash recovery code pass in an attribute that would conditionally activate the crash text, but make it default to a simpler version with a message that is simply a "Restore Session" headline, and remove the "Start new session" button. 

* This will probably add a string, but be quite straightforward and self-contained.

* No privileged code is needed on the start page, and we'll get decent visibility and a known/tested implementation that we're already using.

* For bonus points: Add in the "Always Restore Session" checkbox in about:sessionrestore. Not a requirement for this to land, though.

Ran this by Beltzner earlier today, and he agreed that it's the best approach with the time we have left before shipping.

Comment 9

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Re-noming based on above.

Comment 10

[Drew Willcoxon :adw]

Taking because I need a blocker, but Paul, if you were already working on this, feel free to take back.

Comment 11

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

(In reply to comment #10)
> Taking because I need a blocker, but Paul, if you were already working on this,
> feel free to take back.

I did start (since I'd actually done a large part of this for an old patch (v0.3) in bug 588482). But if you want it, I can give you what I've done so far (or you can have a [hardblocker] ;) )

Comment 12

[Axel Hecht [:Pike]]

Can this land today? I don't want to have strings land after this week.

Comment 13

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

(In reply to comment #11)
> (In reply to comment #10)
> > Taking because I need a blocker, but Paul, if you were already working on this,
> > feel free to take back.
> 
> I did start (since I'd actually done a large part of this for an old patch
> (v0.3) in bug 588482). But if you want it, I can give you what I've done so far
> (or you can have a [hardblocker] ;) )

Actually, if you want to take the home page part of it, we can do a little captain planet "our powers combined" move. I haven't looked at that part yet.

(In reply to comment #12)
> Can this land today? I don't want to have strings land after this week.

The whole thing, no. The strings, maybe.

Comment 14

[Drew Willcoxon :adw]

(In reply to comment #13)
> Actually, if you want to take the home page part of it, we can do a little
> captain planet "our powers combined" move. I haven't looked at that part yet.

I haven't either, so unassigning for now.

Comment 15

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Attachment: Patch v0.1 (WIP)

This works, no new strings needed. I think reusing the string is OK since there aren't really space constraints and it's being used to convey the same meaning.

Axel, assuming this approach is good otherwise, does that sounds reasonable for string usage?

WIP because Limi wants to hide the button instead of disable it (should be just as easy).

Comment 16

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

Comment on attachment 505591 [details] [diff] [review]
Patch v0.1 (WIP)

discussed IRL:
-less generic name than "BrowserOnLoad"
-about:home URL regex check should match whole string
another nit:
-styling for about:home should be in aboutHome.css rather than inline

Comment 17

[Axel Hecht [:Pike]]

The concern I have is that the original string is in a menu, which may have made people choose a wording that is shorter than good in general. Which could be used on the startpage, then, I guess.

I've looked at http://mxr.mozilla.org/l10n-central/search?string=historyRestoreLastSession.l&find=browser.dtd%24&tree=l10n-central, and nothing there looks suspicious to me, the guy that speaks, well, two languages.

Should be good enough, if needed we can file a follow up bug for fx.next, I guess.

Comment 18

[Mihai Sucan [:msucan]]

Attachment: Patch v0.2 (WIP)

This is the updated patch, based on the work in bug 627301.

Comment 19

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

Comment on attachment 506506 [details] [diff] [review]
Patch v0.2 (WIP)

This doesn't seem to address the review comments in comment 16.

Comment 20

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

I'll address those. I was just expecting Mihai to attach diff to aboutHome.* and then I would followup.

Comment 21

[Mihai Sucan [:msucan]]

Gavin: sorry about that. I didn't notice requests you had.

Paul: yeah, it seemed more fitting to only update your patch to work with the styling of bug 627301 - less noise across the two bugs. There will be further styling work being done there. Thus keeping this patch minimal seemed reasonable. Please let me know if I should make further changes.

Comment 22

[Frédéric Buclin]

(In reply to comment #1)
> For people not using Firefox Start, I think an entry in the History menu is
> sufficient, like we do currently.

Why should it be sufficient? I just upgraded to beta10 and realized that all my tabs were gone, and got no confirmation on exit. When I restarted Firefox, I expected my tabs to be back, but no, nothing. At work, we have the default page set to the website of our company, so it's pretty common to not have the Firefox Start page by default. This means we would all miss the "Click here to restore your session" link. Are you assuming that people who changed their default page years ago are all involved enough with the Firefox development to know about bug 592822 and to know where to look at to restore their session? Or are you considering these users as unimportant?

Automatically saving the previous session is great, but not knowing how to restore this previous session makes this feature pretty useless. I had to come on IRC to know how to restore my session (and fortunately, I didn't close Firefox yet, else I guess the session with all the tabs that haven't been read yet and which are important for my day job would have been definitely lost). Is that how you plan to improve UX in Fx4? By making people discover new features the hard way?

Comment 23

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Attachment: Patch v0.3

Updated based on discussion with Gavin & IRL comment from Limi (hide the button, don't disable it).

This includes the minimal markup change to about:home (based on the patch in bug 627301). The CSS is still all over there, which I think is fine for now, but before that lands we'll want to make sure it gets taken out & added here.

Comment 24

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Based on Axel's comment & what we're going ahead with, this is no longer a [strings] bug.

Comment 25

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

Comment on attachment 507200 [details] [diff] [review]
Patch v0.3

Looks good, but will need to be rebase on top of bug 627301 once that lands.

Comment 26

[Mihai Sucan [:msucan]]

Attachment: Patch v0.3b (added styling)

Moved the button styling out of bug 627301.


Paul: please note that attachment 507200 [details] [diff] [review] has a rebase error in aboutHome.xhtml: #sessionRestoreContainer needs to be in #contentContainer, not in #snippetContainer. This issue is fixed in this patch.

Comment 27

[Stephen Horlander [:shorlander]]

Attachment: Restore Icon RTL

Comment 28

[Mihai Sucan [:msucan]]

Attachment: Patch v0.3c (added styling and RTL fixes)

Fixes for RTL users, based on review from Ehsan in the main bug 627301.

Comment 29

[(no longer active)]

Comment on attachment 507980 [details] [diff] [review]
Patch v0.3c (added styling and RTL fixes)

Amazing!

Comment 30

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

is there a specific reason we cannot mirror aboutHome-restore-icon.png instead of having a rtl version? is it much different?

Comment 31

[(no longer active)]

(In reply to comment #30)
> is there a specific reason we cannot mirror aboutHome-restore-icon.png instead
> of having a rtl version? is it much different?

It's a background picture...

But there's also the trick of double-flip, which can be used here.  Example:

data:text/html,<div style="-moz-transform:scaleX(-1);background:url(http://mozcom-cdn.mozilla.net/img/home/download-logo.png)"><div style="-moz-transform:scaleX(-1)">text</div></div>

r=me on any of them (although /me would prefer the latter).

Comment 32

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

I think we could do something with a :before pseudoelement,  as we did in other pages, that would also avoid having to change padding for each screen size since it would always appear before the text.

Comment 33

[Mihai Sucan [:msucan]]

(In reply to comment #32)
> I think we could do something with a :before pseudoelement,  as we did in other
> pages, that would also avoid having to change padding for each screen size
> since it would always appear before the text.

I did think of using :before, but afaik we'd have problems with the other backgrounds of the button. The icon wouldn't end-up "inside" the box-shadow of the main button, inside the gradients and borders, etc.

Additionally, flipping with -moz-transform wouldn't also flip the text of the element? Can I flip only the background-image?

Ehsan: that double flipping would work if we could switch away from the single-button element. I wanted to use a more divy approach, with more flexibility, but the browser.js code doesn't allow us to do that. We must have the button element clickable, nothing else.

Comment 34

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

(In reply to comment #33)
> (In reply to comment #32)
> The icon wouldn't end-up "inside" the box-shadow of
> the main button, inside the gradients and borders, etc.

This is untrue if the pseudoelement contains only the icon and is a #restorePreviousSession::before, the appearance is exactly identical but you can define the background in one single rule.

> Additionally, flipping with -moz-transform wouldn't also flip the text of the
> element? Can I flip only the background-image?

the before would contain only the icon, so it would work fine.

I did some experiment, using the pseudoelement to add a icon to the button is trivial, and works fine for rtl, the problem is that I've not found any way to make it resize WITH the button, looks like it is not aware of the size of the containing button, so I can't use height: 100%, nor inherit, nor -moz-calc. I've found no way to resize it automatically, setting the image as content makes it fixed size, setting it as background I can't give the pseudoelement a relative size.
So, I guess it's not feasible with ::before if we retain the resizeability :(

Comment 35

[Mihai Sucan [:msucan]]

(In reply to comment #34)
> (In reply to comment #33)
> > (In reply to comment #32)
> > The icon wouldn't end-up "inside" the box-shadow of
> > the main button, inside the gradients and borders, etc.
> 
> This is untrue if the pseudoelement contains only the icon and is a
> #restorePreviousSession::before, the appearance is exactly identical but you
> can define the background in one single rule.

Nice, I didn't know that.

> > Additionally, flipping with -moz-transform wouldn't also flip the text of the
> > element? Can I flip only the background-image?
> 
> the before would contain only the icon, so it would work fine.
> 
> I did some experiment, using the pseudoelement to add a icon to the button is
> trivial, and works fine for rtl, the problem is that I've not found any way to
> make it resize WITH the button, looks like it is not aware of the size of the
> containing button, so I can't use height: 100%, nor inherit, nor -moz-calc.
> I've found no way to resize it automatically, setting the image as content
> makes it fixed size, setting it as background I can't give the pseudoelement a
> relative size.
> So, I guess it's not feasible with ::before if we retain the resizeability :(

Thanks for testing ::before. I feared we'll bump into other issues. I tried several iterations of that button, played a lot with the code.

Comment 36

[Bill Gianopoulos [:WG9s]]

(In reply to comment #28)
> Created attachment 507980 [details] [diff] [review]
> Patch v0.3c (added styling and RTL fixes)

The RTL icon included here is 88x76 pixels, whereas the non-RTL icon is 78x66.

Is this intentional?

Comment 37

[Mihai Sucan [:msucan]]

(In reply to comment #36)
> (In reply to comment #28)
> > Created attachment 507980 [details] [diff] [review]
> > Patch v0.3c (added styling and RTL fixes)
> 
> The RTL icon included here is 88x76 pixels, whereas the non-RTL icon is 78x66.
> 
> Is this intentional?

Yes, that is intentional. The 10 additional pixels provide the padding on the right.

Comment 38

[Bill Gianopoulos [:WG9s]]

This looks fine under Linux.  The "Restore Previous Session" text looks way too small under Windows.

Comment 39

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

Comment on attachment 507980 [details] [diff] [review]
Patch v0.3c (added styling and RTL fixes)

Probably wouldn't hurt to throw an "i" on those about:home regexps so that about:Home works too.

Comment 40

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

(In reply to comment #39)
> Comment on attachment 507980 [details] [diff] [review]
> Patch v0.3c (added styling and RTL fixes)
> 
> Probably wouldn't hurt to throw an "i" on those about:home regexps so that
> about:Home works too.

We should file a follow-up bug that replaces the Firefox logo with Kanye West if you enter ABOUT:HOME

Comment 41

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Pushed http://hg.mozilla.org/mozilla-central/rev/abe3d7efd84f with the regex fix in comment 39. Bug 630405 is stopping about:Home from completely working anyway!

Comment 42

[Bill Gianopoulos [:WG9s]]

Perhaps it would be much simpler to make all about links be case sensitive and work in lower case only.

Comment 43

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

(In reply to comment #38)
> This looks fine under Linux.  The "Restore Previous Session" text looks way too
> small under Windows.

Can you file a followup and make sure shorlander and mihai are CC'ed? I know there are a few other tweaks being made to that page and we might as well get this done at the same time.

Comment 44

[Jim Jeffery not reading bug-mail 1/2/11]

Perhaps not the intention of this bug, but clicking the 'restore session' opens the restored session in a New Window, leaving the user with the 'start page' in one window, and the session in another.

Very annoying.

Comment 45

[Chris B.]

(In reply to comment #44)
> Perhaps not the intention of this bug, but clicking the 'restore session' opens
> the restored session in a New Window, leaving the user with the 'start page' in
> one window, and the session in another.
> 
> Very annoying.

It always does that, even if you use the Firefox menu to Restore Previous Session.  The previous session opens in a new window, because the previous session doesn't include the tabs you are currently viewing.

Comment 46

[eyal gruss]

(In reply to comment #44)
> Perhaps not the intention of this bug, but clicking the 'restore session' opens
> the restored session in a New Window, leaving the user with the 'start page' in
> one window, and the session in another.
> Very annoying.

Bug 627642

Comment 47

[Siddhartha Dugar [:sdrocking]]

(In reply to comment #44)
> Perhaps not the intention of this bug, but clicking the 'restore session' opens
> the restored session in a New Window, leaving the user with the 'start page' in
> one window, and the session in another.
WFM

Comment 48

[Ben]

Works ok on beta 11, text size seems ok for me.

Comment 49

[Antoine Turmel [:GeekShadow]]

I'm wondering if controls matters a lot on the restore icon (in case of RTL). On Ubuntu and Mac OS X these are on the left of the window ;)

Comment 50

[(no longer active)]

(In reply to comment #49)
> I'm wondering if controls matters a lot on the restore icon (in case of RTL).
> On Ubuntu and Mac OS X these are on the left of the window ;)

Well, yeah, given the fact that they're not really meaningful in LTR builds anyway, I guess it's not a big deal!

Comment 51

[Ben]

Why is the status above 'fixed' when people who've set a custom home page - surely the majority - get nothing?  As mentioned by various people, this effectively means LOST DATA.

This whole move is illogical: the only time the user is going to want to restore the session is immediately after restarting Firefox, so there's no clear benefit in having the History menu item.  You've expended many programming hours on a change that's going to infuriate huge numbers of loyal users.

What about privacy?  Users who previously closed Fx and selected 'No' not to save the session may believe it has not been saved.  Someone else using their profile could restore their session.

Someone who's managed to get the prompt on exit back can select 'No', but the session is saved anyway!

The old behaviour was the right one.  Sometimes the user wants to restore their session, and sometimes not.  On exit is the time they want to make this decision.  And if you MUST make the change, please tell ALL users with stored sessions how to restore them!!!

Comment 52

[u279076]

(In reply to comment #51)
> Why is the status above 'fixed' when people who've set a custom home page -
> surely the majority - get nothing?  As mentioned by various people, this
> effectively means LOST DATA.

Previous sessions are still accessible through History > Restore Previous Session
 
> This whole move is illogical: the only time the user is going to want to
> restore the session is immediately after restarting Firefox, so there's no
> clear benefit in having the History menu item.  

The whole point of this was to prevent loss of data. You can restore your session later or if you've forgotten to do so.

> What about privacy?  Users who previously closed Fx and selected 'No' not to
> save the session may believe it has not been saved.  Someone else using their
> profile could restore their session.

I guess this could be a concern before this feature was implemented too. Session Store has never asked for proof of ownership on a profile.  If someone sitting at your computer really wanted to snoop session data, they could still look through your cookies, history, and bookmarks. Privacy is always a valid concern, but I'm not sure how it can be improved without adversely affecting the user experience.

> Someone who's managed to get the prompt on exit back can select 'No', but the
> session is saved anyway!

The dialog does not exist anymore. Session data is always saved unless the pref is turned off in about:config.  The option in the Preferences dialog simply changes whether the session is restored at startup.

> The old behaviour was the right one.  Sometimes the user wants to restore 
> their session, and sometimes not. 

There was a design process and decision made to remove the Save and Quit dialog.  However, this bug is not what that covers.  Feel free to file a new bug to add that functionality back if it's a show-stopper for you.

> ...if you MUST make the change, please tell ALL users with stored
> sessions how to restore them!!!

This has been well communicated through the development process via the normal channels (newgroups, blogs, release notes, etc).  Any new features and major changes to Firefox will be communicated wider on the actual release.

Thank you for your passion for Firefox.

Comment 53

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

(In reply to comment #51)
> Why is the status above 'fixed' when people who've set a custom home page -
> surely the majority - get nothing?

This bug is only about adding a button on about:home, which is done. Please bring your concerns up in a different bug or on the mozilla.dev.apps.firefox newsgroup.

Comment 54

[Ben]

Thanks for your detailed reply, Anthony.  With respect, if this is all about safeguarding user data, not telling most users their data has been saved and how to restore it is something of a fatal flaw!

Most users do not read release notes, and may, possibly give the What's New page a 10 second glance.  And applications should be intuitive, anyway.

The old save dialog is not incompatible with the new Restore Session feature, and, as well as better privacy, has the added advantage that if a user accidentally clicks the button to close the last window they get a warning before the application exits.  So maybe I'll add a feature request for that.  But it'll then disappear into a sea of such requests.


Paul, your own title for this bug and first post do not make that clear.  It's your 'Backup Plan' in Bug 588482 that fails to cover all the bases.  But sure, if an end user adding a bug report is what it takes...

Comment 55

[u279076]

(In reply to comment #54)
> ...So maybe I'll add a feature request for that. 
> But it'll then disappear into a sea of such requests.

I strongly recommend filing a request for that.  Granted, there are a lot of bugs and feature requests; you should not let that prevent you from filing.  An unfiled bug will NEVER get fixed.  We do look at all feature requests and bugs; it's just that some are given higher priority than others.  Historically, "enhancement" bugs are given low priority (especially during the feature freeze phase of a release).

Comment 56

[Ben]

Filed as Bug 635231.  It is filed as 'Critical', which I believe is reasonable given the widespread effective loss of session data that will result if Firefox is shipped without addressing this.  I have also come up with a solution, included in the report, which may just leave everyone happy! :)

Comment 57

[u279076]

(In reply to comment #56)
> Filed as Bug 635231.  It is filed as 'Critical', which I believe is reasonable
> given the widespread effective loss of session data that will result if Firefox
> is shipped without addressing this.  I have also come up with a solution,
> included in the report, which may just leave everyone happy! :)

Thank you, Ben.