"Recently closed tabs" list should exclude about:newtab

RESOLVED FIXED in Firefox 24

Status

()

defect
RESOLVED FIXED
6 years ago
Last year

People

(Reporter: sidrabbit, Assigned: sidrabbit)

Tracking

Trunk
Firefox 24
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:24.0) Gecko/20130618 Firefox/24.0 (Nightly/Aurora)
Build ID: 20130618031335

Steps to reproduce:

1. Start Nightly
2. Open New Tab
3. Close the New Tab
4. History > Recently Closed Tabs


Actual results:

"Recently closed tabs" listed New Tab (which is about:newtab by default)


Expected results:

Should not
Blocks: 581937
Component: Untriaged → Session Restore
Attachment #764476 - Flags: review?
Attachment #764476 - Attachment is obsolete: true
Attachment #764476 - Flags: review?
Attachment #764481 - Flags: review?
Comment on attachment 764481 [details] [diff] [review]
Treat about:newtab just like about:blank

Thanks for the patch!

This looks fine to me, but I'd like to run it by Tim first.  Tim, you're working on SessionStore now, right?

>diff --git a/browser/components/sessionstore/src/SessionStore.jsm b/browser/components/sessionstore/src/SessionStore.jsm
>--- a/browser/components/sessionstore/src/SessionStore.jsm
>+++ b/browser/components/sessionstore/src/SessionStore.jsm
>@@ -4052,18 +4052,19 @@ let SessionStoreInternal = {
>    * @returns boolean
>    */
>   _shouldSaveTabState: function ssi_shouldSaveTabState(aTabState) {
>     // If the tab has only the transient about:blank history entry, no other
>     // session history, and no userTypedValue, then we don't actually want to
>     // store this tab's data.

Could you please update this comment now that about:newtab is excluded too?  For example, "If the tab has only a transient about: history entry, ..."
Attachment #764481 - Flags: review?(ttaubert)
Attachment #764481 - Flags: review?
Attachment #764481 - Flags: feedback+
Assignee: nobody → sidrabbit
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
OS: Windows 7 → All
Hardware: x86_64 → All
Summary: "Recently closed tabs" list keep about:newtab tabs → "Recently closed tabs" list should exclude about:newtab
Comment on attachment 764481 [details] [diff] [review]
Treat about:newtab just like about:blank

Review of attachment 764481 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me, thanks!
Attachment #764481 - Flags: review?(ttaubert) → review+
I forgot to say, r=me with Drew's comment addressed, of course. Thanks!
Attachment #764481 - Attachment is obsolete: true
Attachment #765161 - Flags: review?(ttaubert)
Comment on attachment 765161 [details] [diff] [review]
Treat about:newtab just like about:blank v2

Review of attachment 765161 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for updating it.  I'll carry over Tim's r+, r=ttaubert.
Attachment #765161 - Flags: review?(ttaubert) → review+
Keywords: checkin-needed
Thank you all!
Sid, can you please format your patch so that we can check it in? It currently lacks the necessary header information, like the patch author and patch name. Here's a great blog post that describes how to prepare your patch for "checkin-needed":

http://blog.bonardo.net/2010/06/22/so-youre-about-to-use-checkin-needed
Keywords: checkin-needed
Thanks for the tip. Is it OK now?
Attachment #765161 - Attachment is obsolete: true
Attachment #765427 - Flags: review?
Comment on attachment 765427 [details] [diff] [review]
Treat about:newtab just like about:blank v2

Review of attachment 765427 [details] [diff] [review]:
-----------------------------------------------------------------

Yes, this looks great. Thank you!
Attachment #765427 - Flags: review? → review+
I landed this on the fx-team branch, it will be in Nightly by roughly tomorrow:

https://hg.mozilla.org/integration/fx-team/rev/80cd0ed2e33c
https://hg.mozilla.org/mozilla-central/rev/80cd0ed2e33c
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 24
Depends on: 918276
You need to log in before you can comment on or make changes to this bug.