Closed Bug 884585 Opened 8 years ago Closed 7 years ago

"Recently closed tabs" list should exclude about:newtab

Categories

(Firefox :: Session Restore, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 24

People

(Reporter: sidrabbit, Assigned: sidrabbit)

References

Details

Attachments

(1 file, 3 obsolete files)

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: 7 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.