Fix the last tab indentation

[char101]

Hi,

So it seems that the problem is with the SessionStore. By setting the 2nd parameter of _beginRemoveTabs, the tab is not stored in the SessionStore closed tab list. But I suspect somewhere Tree Style Tabs uses this information.

So as as solution which seems to work with my limited testing, I use the normal removeTab method and remove the tab from the recently closed tab list by using the (undocumented) forgetClosedTab method.

[NiklasGollenstede]

Unfortunately, there is a limit to the number of tabs you can restore, which means that gSessionStore.getClosedTabCount(gWindow) will never return a value greater than 10. Is there a reason why you added the lastClosedTabCount test, or was that just a precaution?

[NiklasGollenstede]

I have added a commit ( ab3387e) that removes the check, and also uploaded it as a beta version to AMO

[char101]

Not all closed tabs are added to the list: _shouldSaveTabState.

[NiklasGollenstede]

Ah, Ok.

Something like this should work then, shouldn't it?

const needToForget = gSessionStore._shouldSaveTabState(tab);
gBrowser.removeTab(tab);
needToForget && gSessionStore.forgetClosedTab(gBrowser.ownerGlobal, 0);

[char101]

Unfortunately the parameter is not the tab but an object constructed here, which is an implementation detail and not part of the API. That's why I didn't use it.

Ideally I think you could add a timestamp to the session data and then peek the closed tab list and compare the timestamp to verify that it is the correct tab.

[NiklasGollenstede]

Yeah, I missed that. But the timestamp thing is pretty ugly ... . I'll think about that, maybe I can come up with something better. Maybe there is a way to temporarily increase the value that browser.sessionstore.max_tabs_undo sets.

[char101]

Alternatively you could save/restore the whole closed tabs list

let SessionStoreNS = Components.utils.import('resource:///modules/sessionstore/SessionStore.jsm', {});

// save closed tabs list
let closedTabs = JSON.parse(gSessionStore.getClosedTabData(gBrowser.ownerGlobal);

// restore closed tabs list
SessionStoreNS.SessionStoreInternal._windows[gBrowser.ownerGlobal.__SSi]._closedTabs = closedTabs;

[char101]

By the way, the comment on forgetClosedTab says

// default to the most-recently closed tab
aIndex = aIndex || 0;

but here

saveClosedTabData(closedTabs, tabData) {
    // Find the index of the first tab in the list
    // of closed tabs that was closed before our tab.
    let index = closedTabs.findIndex(tab => {
      return tab.closedAt < tabData.closedAt;
    });

    // If we found no tab closed before our
    // tab then just append it to the list.
    if (index == -1) {
      index = closedTabs.length;
    }

the default position is at the end.

So I think the correct index is the last index not 0:

gSessionStore.forgetClosedTab(gBrowser.ownerGlobal, gSessionStore.getClosedTabCount() - 1);

[char101]

I agree that temporarily modifying the max_tabs_undo is better since it does not access the internal variables of SessionStoreInternal.

let prefBranch = Services.prefs.getBranch("browser.");
let maxTabsUndo = prefBranch.getIntPref('sessionstore.max_tabs_undo');
prefBranch.setIntPref('sessionstore.max_tabs_undo', maxTabsUndo + 1);
gBrowser.removeTab(tab);
// still need to check here that the tab is actually stored in the closed tabs list
prefBranch.setIntPref('sessionstore.max_tabs_undo', maxTabsUndo); // will remove the last entry

[NiklasGollenstede]

That looks good, I'll implement something along those lines tomorrow. But now it's 4am where I live, so I really need to go to bed. And thanks! (you were pretty quick funding that stuff)

[NiklasGollenstede]

Ok. Same thing again: I have added a commit (68b668b) that adds the maxTabsUndo and lastClosedTabCount logic, and also uploaded it as a beta version to AMO. To me it seems to be working, so it would be nice if you could test it with Tree Style Tabs in a real-world browser for a while.