From cf0ea617dfa0cd16111e082bcaf8158ac7ec2a33 Mon Sep 17 00:00:00 2001 From: Jeremy Swinarton Date: Wed, 8 Jan 2025 22:33:13 +0000 Subject: [PATCH] Bug 1939599: Tab groups can be restored by Ctrl+Shift+T r=dao,sthompson,sessionstore-reviewers Differential Revision: https://phabricator.services.mozilla.com/D233102 --- browser/base/content/browser.js | 36 ++++++++++ browser/base/content/browser.js.globals | 1 + .../sessionstore/SessionStore.sys.mjs | 38 ++++++++--- ...r_restoreLastClosedTabOrWindowOrSession.js | 68 +++++++++++++++++++ 4 files changed, 132 insertions(+), 11 deletions(-) diff --git a/browser/base/content/browser.js b/browser/base/content/browser.js index 6145489a21fa..868dbdf050be 100644 --- a/browser/base/content/browser.js +++ b/browser/base/content/browser.js @@ -5977,6 +5977,10 @@ function restoreLastClosedTabOrWindowOrSession() { undoCloseTab(); break; } + case SessionStore.LAST_ACTION_CLOSED_TAB_GROUP: { + undoCloseTabGroup(lastActionTaken.closedId); + break; + } case SessionStore.LAST_ACTION_CLOSED_WINDOW: { undoCloseWindow(); break; @@ -6051,6 +6055,38 @@ function undoCloseTab(aIndex, sourceWindowSSId) { return tab; } +/** + * Re-open a closed tab group. + * @param {string} tabGroupId + * The tab group's unique ID. + * @returns {MozTabbrowserTabGroup} the reopened group. + */ +function undoCloseTabGroup(tabGroupId) { + let targetWindow = window; + + // wallpaper patch to prevent an unnecessary blank tab (bug 343895) + let blankTabToRemove = null; + if ( + targetWindow.gBrowser.visibleTabs.length == 1 && + targetWindow.gBrowser.selectedTab.isEmpty + ) { + blankTabToRemove = targetWindow.gBrowser.selectedTab; + } + + let group; + try { + group = SessionStore.undoCloseTabGroup(window, tabGroupId, targetWindow); + } catch (err) { + group = SessionStore.openSavedTabGroup(tabGroupId, targetWindow); + } + + if (group && blankTabToRemove) { + targetWindow.gBrowser.removeTab(blankTabToRemove); + } + + return group; +} + /** * Re-open a closed window. * @param aIndex diff --git a/browser/base/content/browser.js.globals b/browser/base/content/browser.js.globals index 8b6558612aaa..39053b685b24 100644 --- a/browser/base/content/browser.js.globals +++ b/browser/base/content/browser.js.globals @@ -90,6 +90,7 @@ "AddKeywordForSearchField", "restoreLastClosedTabOrWindowOrSession", "undoCloseTab", + "undoCloseTabGroup", "undoCloseWindow", "ReportFalseDeceptiveSite", "ReportSiteIssue", diff --git a/browser/components/sessionstore/SessionStore.sys.mjs b/browser/components/sessionstore/SessionStore.sys.mjs index 6a7896440c65..9532cdf9843d 100644 --- a/browser/components/sessionstore/SessionStore.sys.mjs +++ b/browser/components/sessionstore/SessionStore.sys.mjs @@ -237,6 +237,10 @@ export var SessionStore = { return SessionStoreInternal._LAST_ACTION_CLOSED_TAB; }, + get LAST_ACTION_CLOSED_TAB_GROUP() { + return SessionStoreInternal._LAST_ACTION_CLOSED_TAB_GROUP; + }, + get LAST_ACTION_CLOSED_WINDOW() { return SessionStoreInternal._LAST_ACTION_CLOSED_WINDOW; }, @@ -1047,6 +1051,8 @@ var SessionStoreInternal = { _LAST_ACTION_CLOSED_TAB: "tab", + _LAST_ACTION_CLOSED_TAB_GROUP: "tab-group", + _LAST_ACTION_CLOSED_WINDOW: "window", _log: null, @@ -3046,6 +3052,15 @@ var SessionStoreInternal = { win, tabGroup ) { + // don't update our internal state if we don't have to + if (this._max_tabs_undo == 0) { + return; + } + + // Record the closed action right away to make sure it's also being + // recorded for groups that are saved and closed + this._addClosedAction(this._LAST_ACTION_CLOSED_TAB_GROUP, tabGroup.id); + if (this.getSavedTabGroup(tabGroup.id)) { // If a tab group is being removed from the tab strip but it's already // saved, then this is a "save and close" action; the saved tab group @@ -3054,24 +3069,17 @@ var SessionStoreInternal = { return; } - // don't update our internal state if we don't have to - if (this._max_tabs_undo == 0) { - return; - } - let closedGroups = this._windows[win.__SSi].closedGroups; - let tabGroupState = lazy.TabGroupState.closed(tabGroup, win.__SSi); tabGroupState.tabs = this._collectClosedTabsForTabGroup(tabGroup.tabs, win); // TODO(jswinarton) it's unclear if updating lastClosedTabGroupCount is // necessary when restoring tab groups — it largely depends on how we - // decide to do the restore. If we add a _LAST_ACTION_CLOSED_TAB_GROUP - // item to the closed actions list, this may not be necessary. + // decide to do the restore. // To address in bug1915174 this._windows[win.__SSi]._lastClosedTabGroupCount = tabGroupState.tabs.length; - closedGroups.push(tabGroupState); + closedGroups.unshift(tabGroupState); this._closedObjectsChanged = true; }, @@ -3312,8 +3320,16 @@ var SessionStoreInternal = { * The tabData to be inserted. * @param closedTabs (array) * The list of closed tabs for a window. + * @param saveAction (boolean|null) + * Whether or not to add an action to the closed actions stack on save. + * True by default if the tab was not closed as part of a tab group; false otherwise. */ - saveClosedTabData(winData, closedTabs, tabData, saveAction = true) { + saveClosedTabData(winData, closedTabs, tabData, saveAction = null) { + if (saveAction === null) { + // By default, a closed action for this tab, but only if it wasn't closed as part of a tab group. + saveAction = closedTabs === winData._closedTabs; + } + // Find the index of the first tab in the list // of closed tabs that was closed before our tab. let index = closedTabs.findIndex(tab => { @@ -4128,7 +4144,7 @@ var SessionStoreInternal = { if ( groupIdx < closedGroups.length && - (tabIdx >= closedTabs.length || group?.closedAt < tab?.closedAt) + (tabIdx >= closedTabs.length || group?.closedAt > tab?.closedAt) ) { group.tabs.forEach((groupTab, idx) => { groupTab._originalStateIndex = idx; diff --git a/browser/components/sessionstore/test/browser_restoreLastClosedTabOrWindowOrSession.js b/browser/components/sessionstore/test/browser_restoreLastClosedTabOrWindowOrSession.js index 10551238f50f..72e8fce490d1 100644 --- a/browser/components/sessionstore/test/browser_restoreLastClosedTabOrWindowOrSession.js +++ b/browser/components/sessionstore/test/browser_restoreLastClosedTabOrWindowOrSession.js @@ -173,6 +173,74 @@ add_task(async function test_undo_last_action() { "Window still has two open tabs" ); + // create a tab group, delete it, and re-open it + const groupedTab = BrowserTestUtils.addTab( + window.gBrowser, + "https://example.com" + ); + await BrowserTestUtils.browserLoaded(groupedTab.linkedBrowser); + const tabGroup = window.gBrowser.addTabGroup([groupedTab]); + Assert.equal( + window.gBrowser.tabs.length, + 3, + "Window has three tabs after creating tab group" + ); + + await TabStateFlusher.flushWindow(window); + let removePromise = BrowserTestUtils.waitForEvent( + tabGroup, + "TabGroupRemoved" + ); + window.gBrowser.removeTabGroup(tabGroup); + await removePromise; + + restoreLastClosedTabOrWindowOrSession(); + Assert.equal( + window.gBrowser.tabs.length, + 3, + "Window has three tabs after restoring tab group" + ); + Assert.equal( + window.gBrowser.tabGroups.length, + 1, + "Tab group exists on the tab strip after restore" + ); + + // Save and close the tab group, and then re-open it + const groupedTabToSave = BrowserTestUtils.addTab( + window.gBrowser, + "https://example.com" + ); + await BrowserTestUtils.browserLoaded(groupedTabToSave.linkedBrowser); + const tabGroupToSave = window.gBrowser.addTabGroup([groupedTabToSave]); + Assert.equal( + window.gBrowser.tabs.length, + 4, + "Window has four tabs after creating tab group" + ); + + await TabStateFlusher.flushWindow(window); + tabGroupToSave.save(); + removePromise = BrowserTestUtils.waitForEvent( + tabGroupToSave, + "TabGroupRemoved" + ); + window.gBrowser.removeTabGroup(tabGroupToSave); + await removePromise; + + await TabStateFlusher.flushWindow(window); + restoreLastClosedTabOrWindowOrSession(); + Assert.equal( + window.gBrowser.tabs.length, + 4, + "Window has four tabs after restoring tab group" + ); + Assert.equal( + window.gBrowser.tabGroups.length, + 2, + "Tab group exists on the tab strip after restore" + ); + gBrowser.removeAllTabsBut(gBrowser.tabs[0]); });