Provide link to "Restore Previous Session" on Firefox Start

VERIFIED FIXED in Firefox 4.0b11

Status

()

VERIFIED FIXED
9 years ago
5 years ago

People

(Reporter: zpao, Assigned: zpao)

Tracking

(Blocks: 1 bug)

Trunk
Firefox 4.0b11
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking2.0 betaN+)

Details

(Whiteboard: [softblocker][fx4-fixed-bugday][about-home])

Attachments

(3 attachments, 3 obsolete attachments)

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).
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.
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.
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.
blocking2.0: --- → ?
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.
>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
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.
blocking2.0: ? → -
(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.
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.
Summary: Discoverability of "Restore Previous Session" is suboptimal → Provide link to "Restore Previous Session" on Firefox Start
Whiteboard: [strings]
Re-noming based on above.
blocking2.0: - → ?
blocking2.0: ? → betaN+
Whiteboard: [strings] → [strings][softblocker]
Taking because I need a blocker, but Paul, if you were already working on this, feel free to take back.
Assignee: nobody → adw
Status: NEW → ASSIGNED
(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] ;) )
Can this land today? I don't want to have strings land after this week.
(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.
(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.
Assignee: adw → nobody
Status: ASSIGNED → NEW
Assignee: nobody → paul
Posted patch Patch v0.1 (WIP) (obsolete) — Splinter Review
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).
Attachment #505591 - Flags: feedback?(gavin.sharp)
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
Attachment #505591 - Flags: feedback?(gavin.sharp) → feedback+
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.
Posted patch Patch v0.2 (WIP) (obsolete) — Splinter Review
This is the updated patch, based on the work in bug 627301.
Attachment #506506 - Flags: review?(gavin.sharp)
Comment on attachment 506506 [details] [diff] [review]
Patch v0.2 (WIP)

This doesn't seem to address the review comments in comment 16.
Attachment #506506 - Flags: review?(gavin.sharp) → review-
I'll address those. I was just expecting Mihai to attach diff to aboutHome.* and then I would followup.
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.
(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?
Posted patch Patch v0.3Splinter Review
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.
Attachment #505591 - Attachment is obsolete: true
Attachment #506506 - Attachment is obsolete: true
Attachment #507200 - Flags: review?(gavin.sharp)
Based on Axel's comment & what we're going ahead with, this is no longer a [strings] bug.
Whiteboard: [strings][softblocker] → [softblocker]
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.
Attachment #507200 - Flags: review?(gavin.sharp) → feedback+
Posted patch Patch v0.3b (added styling) (obsolete) — Splinter Review
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.
Fixes for RTL users, based on review from Ehsan in the main bug 627301.
Attachment #507955 - Attachment is obsolete: true
Attachment #507980 - Flags: review?(ehsan)

Comment 29

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

Amazing!
Attachment #507980 - Flags: review?(ehsan) → review+
is there a specific reason we cannot mirror aboutHome-restore-icon.png instead of having a rtl version? is it much different?

Comment 31

8 years ago
(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).
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.
(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.
(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 :(
(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.
(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?
(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.

Updated

8 years ago
Blocks: 592822
This looks fine under Linux.  The "Restore Previous Session" text looks way too small under Windows.
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.
Attachment #507980 - Flags: review?(gavin.sharp) → review+
(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
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!
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0b11
Perhaps it would be much simpler to make all about links be case sensitive and work in lower case only.
(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.
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

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

Updated

8 years ago
Blocks: 630568

Comment 48

8 years ago
Works ok on beta 11, text size seems ok for me.
Status: RESOLVED → VERIFIED
Whiteboard: [softblocker] → [softblocker][fx4-fixed-bugday]
No longer blocks: 467530
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 ;)
Whiteboard: [softblocker][fx4-fixed-bugday] → [softblocker][fx4-fixed-bugday][about-home]

Comment 50

8 years ago
(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

8 years ago
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!!!
(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.
(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

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

8 years ago
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! :)
(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.

Updated

8 years ago
Blocks: 561152
You need to log in before you can comment on or make changes to this bug.