Restore previously-active tab when closing a browser tab

When closing a browser tab, announce the MRU page selection
before guest teardown to prevent fallback to registration order.
Reorder closeBrowserTab() before destroyWorkspaceWebviews()
consistently across all close-tab handlers.
This commit is contained in:
Jinjing
2026-08-24 15:00:15 -07:00
parent 89545ce475
commit f9b5691e50
5 changed files with 58 additions and 8 deletions
+5 -3
View File
@@ -1777,8 +1777,9 @@ function Terminal(): React.JSX.Element | null {
const hasUnifiedEntry = Object.values(state.unifiedTabsByWorktree).some((tabs) =>
tabs.some((tab) => tab.contentType === 'browser' && tab.entityId === tabId)
)
destroyWorkspaceWebviews(state.browserPagesByWorkspace, tabId)
closeBrowserTab(tabId)
// closeBrowserTab announces the MRU target before guest teardown can trigger bridge fallback.
destroyWorkspaceWebviews(state.browserPagesByWorkspace, tabId)
if (!hasUnifiedEntry && state.activeWorktreeId === owningWorktreeId) {
const worktreeFile = state.openFiles.find((file) => file.worktreeId === owningWorktreeId)
if (worktreeFile) {
@@ -1796,8 +1797,9 @@ function Terminal(): React.JSX.Element | null {
}
return
}
destroyWorkspaceWebviews(state.browserPagesByWorkspace, tabId)
closeBrowserTab(tabId)
// closeBrowserTab announces the MRU target before guest teardown can trigger bridge fallback.
destroyWorkspaceWebviews(state.browserPagesByWorkspace, tabId)
},
[closeBrowserTab, setActiveFile, setActiveTab, setActiveTabType, setActiveWorktree]
)
@@ -1868,8 +1870,8 @@ function Terminal(): React.JSX.Element | null {
} else if (
(state.browserTabsByWorktree[activeWorktreeId] ?? []).some((tab) => tab.id === id)
) {
destroyWorkspaceWebviews(state.browserPagesByWorkspace, id)
closeBrowserTab(id)
destroyWorkspaceWebviews(state.browserPagesByWorkspace, id)
} else if (unifiedTab?.contentType === 'simulator') {
// Why: simulator tabs live only in the unified-tab store, so the
// entity-store checks above never match them.
@@ -896,8 +896,8 @@ export function FloatingTerminalPanel({
if (item.contentType === 'terminal') {
closeTab(item.entityId, { reason: 'cleanup' })
} else if (item.contentType === 'browser') {
destroyWorkspaceWebviews(state.browserPagesByWorkspace, item.entityId)
closeBrowserTab(item.entityId)
destroyWorkspaceWebviews(state.browserPagesByWorkspace, item.entityId)
} else if (item.contentType === 'simulator') {
closeUnifiedTab(item.id)
} else {
@@ -952,8 +952,8 @@ export function FloatingTerminalPanel({
onClose: () => {
const latest = useAppStore.getState()
if (item.contentType === 'browser') {
destroyWorkspaceWebviews(latest.browserPagesByWorkspace, item.entityId)
closeBrowserTab(item.entityId)
destroyWorkspaceWebviews(latest.browserPagesByWorkspace, item.entityId)
} else if (item.contentType === 'simulator') {
closeUnifiedTab(item.id)
} else {
@@ -99,8 +99,8 @@ export function useTabGroupTabCloseCommands({
reason: 'user'
})
}
destroyWorkspaceWebviews(browserState.browserPagesByWorkspace, item.entityId)
closeBrowserTab(item.entityId)
destroyWorkspaceWebviews(browserState.browserPagesByWorkspace, item.entityId)
closeUnifiedTab(item.id)
} else if (item.contentType === 'simulator') {
closeUnifiedTab(item.id)
@@ -159,8 +159,8 @@ export function useTabGroupTabCloseCommands({
reason: 'user'
})
}
destroyWorkspaceWebviews(browserState.browserPagesByWorkspace, item.entityId)
closeBrowserTab(item.entityId)
destroyWorkspaceWebviews(browserState.browserPagesByWorkspace, item.entityId)
closeUnifiedTab(item.id)
} else if (item.contentType === 'terminal') {
closeTab(item.entityId)
@@ -105,6 +105,21 @@ function makeAnnotation(pageId: string, id = 'annotation-1'): BrowserPageAnnotat
}
describe('createBrowserSlice annotations', () => {
it('announces the store-selected browser page before its guest is destroyed', () => {
const store = createTestStore()
const previous = store.getState().createBrowserTab('wt-1', 'https://previous.example.com')
const closing = store.getState().createBrowserTab('wt-1', 'https://closing.example.com')
store.getState().setActiveBrowserTab(previous.id)
store.getState().setActiveBrowserTab(closing.id)
mockApi.browser.notifyActiveTabChanged.mockClear()
store.getState().closeBrowserTab(closing.id)
expect(mockApi.browser.notifyActiveTabChanged).toHaveBeenCalledWith({
browserPageId: previous.activePageId
})
})
it('keeps a requested canonical page identity distinct from its workspace', () => {
const store = createTestStore()
const tab = store.getState().createBrowserTab('wt-1', 'about:blank', {
+34 -1
View File
@@ -805,6 +805,7 @@ export const createBrowserSlice: StateCreator<AppState, [], [], BrowserSlice> =
},
closeBrowserTab: (tabId) => {
let remotePagesToClose: { worktreeId: string; handle: RemoteBrowserPageHandle }[] = []
let activeBrowserWorktreeIdToNotify: string | null = null
set((s) => {
let owningWorktreeId: string | null = null
let closedWorkspace: BrowserWorkspace | null = null
@@ -865,6 +866,9 @@ export const createBrowserSlice: StateCreator<AppState, [], [], BrowserSlice> =
const isActiveTabInOwningWorktree =
s.activeWorktreeId === owningWorktreeId && s.activeBrowserTabId === tabId
if (isActiveTabInOwningWorktree) {
activeBrowserWorktreeIdToNotify = owningWorktreeId
}
const nextActiveTabTypeByWorktree = { ...s.activeTabTypeByWorktree }
let nextActiveTabType = s.activeTabType
if (remainingBrowserTabs.length === 0) {
@@ -946,6 +950,34 @@ export const createBrowserSlice: StateCreator<AppState, [], [], BrowserSlice> =
get().closeUnifiedTab(workspaceItem.id)
}
}
// Why: announce the MRU page before guest teardown so bridge fallback cannot choose registration order.
if (activeBrowserWorktreeIdToNotify) {
const state = get()
const activeWorkspaceId = state.activeBrowserTabIdByWorktree[activeBrowserWorktreeIdToNotify]
const activeWorkspace = activeWorkspaceId
? findWorkspace(state.browserTabsByWorktree, activeWorkspaceId)
: null
const activePage = activeWorkspace?.activePageId
? (state.browserPagesByWorkspace[activeWorkspace.id] ?? []).find(
(page) => page.id === activeWorkspace.activePageId
)
: undefined
if (
activeWorkspace?.activePageId &&
isLocalBrowserPageOwner(
state,
activeBrowserWorktreeIdToNotify,
activePage?.browserRuntimeEnvironmentId
) &&
typeof window !== 'undefined' &&
window.api?.browser
) {
window.api.browser
.notifyActiveTabChanged({ browserPageId: activeWorkspace.activePageId })
.catch(() => {})
}
}
},
shutdownWorktreeBrowsers: async (worktreeId) => {
@@ -953,8 +985,9 @@ export const createBrowserSlice: StateCreator<AppState, [], [], BrowserSlice> =
// Why: snapshot before the loop — closeBrowserTab empties the array, so set() below couldn't recompute hadBrowserTabs.
const hadBrowserTabs = workspaces.length > 0
for (const workspace of workspaces) {
destroyWorkspaceWebviews(get().browserPagesByWorkspace, workspace.id)
const browserPagesByWorkspace = get().browserPagesByWorkspace
get().closeBrowserTab(workspace.id)
destroyWorkspaceWebviews(browserPagesByWorkspace, workspace.id)
}
set((s) => {
const nextBrowserTabsByWorktree = { ...s.browserTabsByWorktree }