355253 - History > Recently Closed Tabs doesn't remember first closed tab.

Status: RESOLVED DUPLICATE of bug 369151

(Firefox :: Bookmarks & History, defect)

Description

[Pablo Greco]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1) Gecko/20061003 BonEcho/2.0
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1) Gecko/20061003 BonEcho/2.0

Session Saver doesn't save first closed tab if "show my windows and tabs from last time" is active

Reproducible: Always

Steps to Reproduce:
1) activate "show my windows and tabs ...."
2) activate "always show tab bar"
3) close all tabs so FF can open with only a blank tab.
4) close and reopen firefox.
5) open anything from bookmarks (didn't test writing url) in current (only) tab
6) close tab.
Actual Results:  
Tab data go into "recently closed tabs".

Expected Results:  
Closed tab should be listed in "recently closed tabs"

Nothing appears on error console

Comment 1

[Steve England [:stevee]]

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8.1) Gecko/20061003 BonEcho/2.0 ID:2006100303

I see this too. Can't see a dupe, so --> NEW
Also punting over to History rather than Tabbed Browsing.

Comment 2

[tabmix.onemen]

this is happen to all blank tab that open by session store.

there is bug in function onTabClose in nsSessionStore.js
its only save the data if (tabState && tabState.entries[0].url != "about:blank") but if the session open blank tabs then entries[0].url is "about:blank"

Comment 3

[tabmix.onemen]

or better 

not to call _deserializeHistoryEntry from restoreHistory
if tabData.entries.length == 1 and tabData.entries[0].url == "about:blank"

Comment 4

[Dietrich Ayala (:dietrich)]

Attachment: fix

As comment #2 alludes, we don't store closed-tab data for about:blank tabs.

This patch modifies that to "don't store closed-tab data for about:blank tabs with no history".

Comment 5

[tabmix.onemen]

Attachment: propose fix

combined  comment #2 and  comment #3

Comment 6

[Simon Bünzli]

Comment on attachment 241109 [details] [diff] [review]
propose fix

This patch goes down the TMP way which we really don't want to follow: with this patch, opening three clean tabs and restarting the browser will make these tabs disappear.

Extensions are free to prune blank tabs before they're restored, but we shouldn't second guess what to do with them by default.

Oh, and Dietrich: Told you so (bug 342169 comment #2)... ;-)

Comment 7

[tabmix.onemen]

(In reply to comment #6)
> (From update of attachment 241109 [details] [diff] [review] [edit])
> .....with
> this patch, opening three clean tabs and restarting the browser will make these
> tabs disappear....

Simon, that not true. all tabs including blank tabs are restored. !

Comment 8

[Simon Bünzli]

Comment on attachment 241109 [details] [diff] [review]
propose fix

Indeed - seems that I misread your patch, sorry.

The second change only omits the "Loading..." ui-polish which isn't needed for blank tabs anyway; and the third change doesn't add an entry which shouldn't be there in the first place.

So IMO, these changes are even well worth taking.

Comment 9

[Asaf Romano (gone)]

So, i don't get it. Say there's an about:blank tab without history (but with say, modified content), how does this patch make sure they're restored?

Comment 10

[Simon Bünzli]

(In reply to comment #9)
It doesn't. But that's nothing to worry about as long as SessionStore isn't able to correctly restore pages with a dynamically modified DOM (i.e. in the case you cite you would only get a blank page anyway).

Also note that we continue to restore blank tabs after crashes or when resuming the session. This is just about not listing useless tabs in the Recently Closed Tabs submenu.

Comment 11

[Asaf Romano (gone)]

Comment on attachment 241109 [details] [diff] [review]
propose fix

OK, r=mano

Comment 12

[Asaf Romano (gone)]

Comment on attachment 241109 [details] [diff] [review]
propose fix

>Index: browser/components/sessionstore/src/nsSessionStore.js
>===================================================================

>+   // add history entry except for about:blank tabs that have no history
>+   if (tabData.entries.length > 1 || tabData.entries[0].url != "about:blank")
>+     for (var i = 0; i < tabData.entries.length; i++) {
>+        history.addEntry(this._deserializeHistoryEntry(tabData.entries[i], idMap), true);
>+     }

"entries"

Comment 13

[u81239]

Probably related:

Testcase: go to Google, search for ‘test’ (or whatever) and open the say, first 15 search results. Then ctrl-tab to the first result, and close the tabs sequentially by using alt-F4 (the next tab is selected each time). Then try to look undo the closing of the last tab. It’s not in the list. And the tab you closed FIRST is on top of the list, while it should have dropped off the list.

Using Firefox 2.0.0.3.


~Grauw

Comment 14

[u81239]

Sorry, of course I meant by using ctrl-f4, not alt-f4.

Comment 15

[charlamage]

 Just as I thought! This problem wasn't any of my add-on's fault! Anyways, another problem that is cause by this bug is the "Restore Session" option (accessable when you restart your system/close it though the Task Manager (Windows)) shows 1 loading tab and 1(or more) blank tabs, with no way to restore your session.

Comment 16

[charlamage]

(In reply to comment #0)
> User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1)
> Gecko/20061003 BonEcho/2.0
> Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1)
> Gecko/20061003 BonEcho/2.0
> 
> Session Saver doesn't save first closed tab if "show my windows and tabs from
> last time" is active
> 
> Reproducible: Always
> 
> Steps to Reproduce:
> 1) activate "show my windows and tabs ...."
> 2) activate "always show tab bar"
> 3) close all tabs so FF can open with only a blank tab.
> 4) close and reopen firefox.
> 5) open anything from bookmarks (didn't test writing url) in current (only) tab
> 6) close tab.
> Actual Results:  
> Tab data go into "recently closed tabs".
> 
> Expected Results:  
> Closed tab should be listed in "recently closed tabs"
> 
> Nothing appears on error console
> 

All session saving add-ons are effected by this bug.

Comment 17

[Simon Bünzli]

(In reply to comment #13)
> Probably related:

Sounds like a different bug (although WORKSFORME).

(In reply to comment #15)
> another problem that is cause by this bug is the "Restore Session" option
> shows 1 loading tab and 1(or more) blank tabs

Sounds like a different bug as well (although WORKSFORME as well).

The issue as described in comment #0 has been fixed in bug 369151 for Firefox 2.0.0.2.