Closed
Bug 884585
Opened 12 years ago
Closed 12 years ago
"Recently closed tabs" list should exclude about:newtab
Categories
(Firefox :: Session Restore, defect)
Firefox
Session Restore
Tracking
()
RESOLVED
FIXED
Firefox 24
People
(Reporter: sidrabbit, Assigned: sidrabbit)
References
Details
Attachments
(1 file, 3 obsolete files)
1.60 KB,
patch
|
ttaubert
:
review+
|
Details | Diff | Splinter Review |
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
Attachment #764476 -
Flags: review?
Attachment #764476 -
Attachment is obsolete: true
Attachment #764476 -
Flags: review?
Attachment #764481 -
Flags: review?
Comment 3•12 years ago
|
||
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+
Updated•12 years ago
|
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 4•12 years ago
|
||
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+
Comment 5•12 years ago
|
||
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 7•12 years ago
|
||
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+
Updated•12 years ago
|
Keywords: checkin-needed
Comment 9•12 years ago
|
||
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
Assignee | ||
Comment 10•12 years ago
|
||
Thanks for the tip. Is it OK now?
Attachment #765161 -
Attachment is obsolete: true
Attachment #765427 -
Flags: review?
Comment 11•12 years ago
|
||
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+
Comment 12•12 years ago
|
||
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
Comment 13•12 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 24
You need to log in
before you can comment on or make changes to this bug.
Description
•