Closed Bug 597637 Opened 10 years ago Closed 10 years ago

Trying to restore final tab in closed tab list throws an exception

Categories

(SeaMonkey :: Tabbed Browser, defect)

x86
Windows XP
defect
Not set
major

Tracking

(Not tracked)

RESOLVED FIXED
seamonkey2.1b2

People

(Reporter: morac, Assigned: neil)

Details

(Keywords: regression)

Attachments

(1 file)

In the 20100915 trunk load, after closing a bunch of tabs, all of them can be restored except for the last one which throws the following exception:

Error: uncaught exception: [Exception... "'Illegal value' when calling method: [nsISessionStore::undoCloseTab]"  nsresult: "0x80070057 (NS_ERROR_ILLEGAL_VALUE)"  location: "JS frame :: chrome://navigator/content/tabbrowser.xml :: undoCloseTab :: line 1550"  data: no]

From what I can tell, the closed tab list is someone getting cleared before final closed tab can be restored.  I'm fairly certain this used to work.  I'm not sure when this broke since I haven't run the SeaMonkey trunk for a while.  I upgraded directly from the 20100722021157 (2.1a3pre) load to the 2010915012425 (2.1b1pre) load so the problem was introduced some time in that time frame (I believe).

Steps to reproduce:
1. Open a bunch of "Home" tabs by middle clicking the home button.
2. Press CTRL-W a few times (I did it three times).
3. Press CTRL-SHIFT-T the same number of times (again I did 3).

Expected results:
1. All closed should be restored.

Actual Results:
1. All tabs restored except the first tab closed (last attempted to restore) which throws an exception.

Error: uncaught exception: [Exception... "'Illegal value' when calling method: [nsISessionStore::undoCloseTab]"  nsresult: "0x80070057 (NS_ERROR_ILLEGAL_VALUE)"  location: "JS frame :: chrome://navigator/content/tabbrowser.xml :: undoCloseTab :: line 1550"  data: no]
blocking-seamonkey2.1: --- → ?
Keywords: regression
morac, I can promise to look into this if you can get me at least a 48 hour regression window, do you think you'll have time for that?
I'll try and narrow the window down tomorrow.
Hmm, now, "tomorrow" has passed by a bit, any news there?
I had issues reproducing this on my work PC which is where I tried to narrow the range down.  I then got side tracked on a bunch of other issue with SessionStore being fairly broken in Minefield.
Okay I figured out why I couldn't reproduce the problem.  I left out a step in my steps to reproduce.  Between steps 2 and 3, restore session data.  My guess based on that is that the closed tab list in SessionStore is getting out of sync with what's displayed in the browser.  I'll do some digging into this and see if I can see what's going wrong.
Okay, it doesn't always happen on a session restore and it can also happen without a session restore.  Using a session restore where the session doesn't have any closed tabs is the easiest way to trigger it though.  For example closing a bunch of tabs and then pasting the following into the error console:

Components.classes["@mozilla.org/suite/sessionstore;1"].getService(Components.interfaces.nsISessionStore).setBrowserState('{"windows":[{"tabs":[{"entries":[{"url":"about:blank"}],"index":1}],"selected":1}],"selectedWindow":0}');

At this point the _windows[aWindow.__SSi]._closedTabs variable is [], but the Recently Closed Tab list still shows tabs in it.  The _closedTabs variable gets set in Sessionstore:restoreWindow().

Now at this point the _closedTabs variable for the window is [], but the savedBrowsers variable for the window (in tabbrowser.xml) still has the closed tabs.  This is reflected in both the SessionStore:getClosedTabCount and SessionStore:getClosedTabData functions:

Components.classes["@mozilla.org/suite/sessionstore;1"].getService(Components.interfaces.nsISessionStore).getClosedTabCount(Components.classes['@mozilla.org/appshell/window-mediator;1'].getService(Components.interfaces.nsIWindowMediator).getMostRecentWindow("navigator:browser"))

Components.classes["@mozilla.org/suite/sessionstore;1"].getService(Components.interfaces.nsISessionStore).getClosedTabData(Components.classes['@mozilla.org/appshell/window-mediator;1'].getService(Components.interfaces.nsIWindowMediator).getMostRecentWindow("navigator:browser"))


The problem is that the SessionStore:undoCloseTab function will thrown an exception if the index is out of range of _closedTabs , which is always the case since _closedTabs is empty.

    var closedTabs = this._windows[aWindow.__SSi]._closedTabs;
    if (!(aIndex in closedTabs))
      throw (Components.returnCode = Components.results.NS_ERROR_INVALID_ARG);


Basically this will happen anytime that the _closedTabs array length in SessionStore does not match the savedBrowsers array length in tabbrowser.xml.  Also if the items in the array don't match restoring closed tabs can result in unexpected results.


Either savedBrowsers needs to be updated when the _closedTabs value is restored or undoClosedTabs needs to look in the returned data of getClosedTabsData to find the closed tab to restore.
You can also trigger this by setting the session restore undo depth to less than the tabbrowser undo depth.
Attached patch Proposed patchSplinter Review
* Added helper function to combine the list of sessionstore and browser tabs
  (based on getClosedTabData, which makes the diff look a little odd)
* Switched getClosedTabCount/Data to use the helper function
* undo/forgetClosedTab now use the helper function, and separately remove the
  tab from the sessionstore list.
Assignee: nobody → neil
Status: NEW → ASSIGNED
Attachment #481465 - Flags: review?(misak.bugzilla)
Attachment #481465 - Flags: review?(misak.bugzilla) → review+
Pushed changeset bfd1320b39ac to comm-central.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.1b2
blocking-seamonkey2.1: ? → ---
You need to log in before you can comment on or make changes to this bug.