Closed
Bug 884585
Opened 11 years ago
Closed 11 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•11 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•11 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•11 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•11 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•11 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•11 years ago
|
Keywords: checkin-needed
Comment 9•11 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•11 years ago
|
||
Thanks for the tip. Is it OK now?
Attachment #765161 -
Attachment is obsolete: true
Attachment #765427 -
Flags: review?
Comment 11•11 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•11 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•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/80cd0ed2e33c
Status: ASSIGNED → RESOLVED
Closed: 11 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
•