mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-04-20 06:20:11 +00:00
fix: restore workflow tabs on browser restart (#10336)
## Problem Since PR #8520 (`feat(persistence): fix QuotaExceededError and cross-workspace draft leakage`), all workflow tabs are lost when the browser is closed and reopened. PR #8520 moved tab pointers (`ActivePath`, `OpenPaths`) from `localStorage` to `sessionStorage` for per-tab isolation. However, `sessionStorage` is cleared when the browser closes, so the open tab list is lost on restart. The draft data itself survives in `localStorage` — only the pointers to which tabs were open are lost. Reported in [Comfy-Org/ComfyUI#12984](https://github.com/Comfy-Org/ComfyUI/issues/12984). Confirmed via binary search: v1.40.9 (last good) → v1.40.10 (first bad). ## Changes Dual-write tab pointers to both storage layers: - **sessionStorage** (scoped by `clientId`) — used for in-session refresh, preserves per-tab isolation - **localStorage** (scoped by `workspaceId`) — fallback for browser restart when sessionStorage is empty Also adds: - `storageAvailable` guard on write functions for consistency with `writeIndex`/`writePayload` - `isValidPointer` validation on localStorage reads to reject stale or malformed data ## Benefits - Workflow tabs survive browser restart (restores V1 behavior) - Per-tab isolation is preserved for in-session use (sessionStorage is still preferred when available) ## Trade-offs - On browser restart, the restored tabs come from whichever browser tab wrote last to localStorage. If Tab A had workflows 1,2,3 and Tab B had 4,5 — the user gets whichever tab wrote most recently. This is the same limitation V1 had with `Comfy.OpenWorkflowsPaths` in localStorage. - Previously (post-#8520), opening a new browser tab would only restore the single most recent draft. With this fix, a new tab restores the full set of open tabs from the last session. This may be surprising for multi-tab users who expect a clean slate in new tabs. ## Test plan - [x] `pnpm typecheck` passes - [x] `pnpm lint` passes - [x] All 121 persistence tests pass - [x] Manual: open multiple workflow tabs → close browser → reopen → tabs restored - [x] Manual: open two browser tabs with different workflows → refresh each → correct tabs in each Fixes Comfy-Org/ComfyUI#12984 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10336-fix-restore-workflow-tabs-on-browser-restart-3296d73d365081b7a7d3e91427d08d17) by [Unito](https://www.unito.io) <!-- QA_REPORT_SECTION --> --- ## 🔍 Automated QA Report | | | |---|---| | **Status** | ✅ Complete | | **Report** | [sno-qa-10336.comfy-qa.pages.dev](https://sno-qa-10336.comfy-qa.pages.dev/) | | **CI Run** | [View workflow](https://github.com/Comfy-Org/ComfyUI_frontend/actions/runs/23373697656) | Before/after video recordings with **Behavior Changes** and **Timeline Comparison** tables.
This commit is contained in:
@@ -764,13 +764,13 @@ test.describe('Load workflow', { tag: '@screenshot' }, () => {
|
||||
)
|
||||
})
|
||||
|
||||
const generateUniqueFilename = (extension = '') =>
|
||||
`${Date.now().toString(36)}-${Math.random().toString(36).slice(2, 8)}${extension}`
|
||||
|
||||
test.describe('Restore all open workflows on reload', () => {
|
||||
let workflowA: string
|
||||
let workflowB: string
|
||||
|
||||
const generateUniqueFilename = (extension = '') =>
|
||||
`${Date.now().toString(36)}-${Math.random().toString(36).slice(2, 8)}${extension}`
|
||||
|
||||
test.beforeEach(async ({ comfyPage }) => {
|
||||
await comfyPage.settings.setSetting('Comfy.UseNewMenu', 'Top')
|
||||
|
||||
@@ -829,6 +829,82 @@ test.describe('Load workflow', { tag: '@screenshot' }, () => {
|
||||
})
|
||||
})
|
||||
|
||||
test.describe('Restore workflow tabs after browser restart', () => {
|
||||
let workflowA: string
|
||||
let workflowB: string
|
||||
|
||||
test.beforeEach(async ({ comfyPage }) => {
|
||||
await comfyPage.settings.setSetting('Comfy.UseNewMenu', 'Top')
|
||||
|
||||
workflowA = generateUniqueFilename()
|
||||
await comfyPage.menu.topbar.saveWorkflow(workflowA)
|
||||
workflowB = generateUniqueFilename()
|
||||
await comfyPage.menu.topbar.triggerTopbarCommand(['New'])
|
||||
await comfyPage.menu.topbar.saveWorkflow(workflowB)
|
||||
|
||||
// Wait for localStorage fallback pointers to be written
|
||||
await comfyPage.page.waitForFunction(() => {
|
||||
for (let i = 0; i < window.localStorage.length; i++) {
|
||||
const key = window.localStorage.key(i)
|
||||
if (key?.startsWith('Comfy.Workflow.LastOpenPaths:')) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
})
|
||||
|
||||
// Simulate browser restart: clear sessionStorage (lost on close)
|
||||
// but keep localStorage (survives browser restart)
|
||||
await comfyPage.page.evaluate(() => {
|
||||
sessionStorage.clear()
|
||||
})
|
||||
await comfyPage.setup({ clearStorage: false })
|
||||
})
|
||||
|
||||
test('Restores topbar workflow tabs after browser restart', async ({
|
||||
comfyPage
|
||||
}) => {
|
||||
await comfyPage.settings.setSetting(
|
||||
'Comfy.Workflow.WorkflowTabsPosition',
|
||||
'Topbar'
|
||||
)
|
||||
// Wait for both restored tabs to render (localStorage fallback is async)
|
||||
await expect(
|
||||
comfyPage.page.locator('.workflow-tabs .workflow-label', {
|
||||
hasText: workflowA
|
||||
})
|
||||
).toBeVisible()
|
||||
|
||||
const tabs = await comfyPage.menu.topbar.getTabNames()
|
||||
const activeWorkflowName = await comfyPage.menu.topbar.getActiveTabName()
|
||||
|
||||
expect(tabs).toEqual(expect.arrayContaining([workflowA, workflowB]))
|
||||
expect(tabs.indexOf(workflowA)).toBeLessThan(tabs.indexOf(workflowB))
|
||||
expect(activeWorkflowName).toEqual(workflowB)
|
||||
})
|
||||
|
||||
test('Restores sidebar workflows after browser restart', async ({
|
||||
comfyPage
|
||||
}) => {
|
||||
await comfyPage.settings.setSetting(
|
||||
'Comfy.Workflow.WorkflowTabsPosition',
|
||||
'Sidebar'
|
||||
)
|
||||
await comfyPage.menu.workflowsTab.open()
|
||||
const openWorkflows =
|
||||
await comfyPage.menu.workflowsTab.getOpenedWorkflowNames()
|
||||
const activeWorkflowName =
|
||||
await comfyPage.menu.workflowsTab.getActiveWorkflowName()
|
||||
expect(openWorkflows).toEqual(
|
||||
expect.arrayContaining([workflowA, workflowB])
|
||||
)
|
||||
expect(openWorkflows.indexOf(workflowA)).toBeLessThan(
|
||||
openWorkflows.indexOf(workflowB)
|
||||
)
|
||||
expect(activeWorkflowName).toEqual(workflowB)
|
||||
})
|
||||
})
|
||||
|
||||
test('Auto fit view after loading workflow', async ({ comfyPage }) => {
|
||||
await comfyPage.settings.setSetting(
|
||||
'Comfy.EnableWorkflowViewRestore',
|
||||
|
||||
@@ -250,61 +250,125 @@ function readSessionPointer<T extends { workspaceId: string }>(
|
||||
|
||||
/**
|
||||
* Reads the active path pointer from sessionStorage.
|
||||
* Falls back to workspace-based search when clientId changes after reload.
|
||||
* Falls back to workspace-based search when clientId changes after reload,
|
||||
* then to localStorage when sessionStorage is empty (browser restart).
|
||||
*/
|
||||
export function readActivePath(
|
||||
clientId: string,
|
||||
targetWorkspaceId?: string
|
||||
): ActivePathPointer | null {
|
||||
return readSessionPointer<ActivePathPointer>(
|
||||
StorageKeys.activePath(clientId),
|
||||
StorageKeys.prefixes.activePath,
|
||||
targetWorkspaceId
|
||||
return (
|
||||
readSessionPointer<ActivePathPointer>(
|
||||
StorageKeys.activePath(clientId),
|
||||
StorageKeys.prefixes.activePath,
|
||||
targetWorkspaceId
|
||||
) ??
|
||||
(targetWorkspaceId
|
||||
? readLocalPointer<ActivePathPointer>(
|
||||
StorageKeys.lastActivePath(targetWorkspaceId),
|
||||
isValidActivePathPointer
|
||||
)
|
||||
: null)
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Writes the active path pointer to sessionStorage.
|
||||
* Writes the active path pointer to both sessionStorage (tab-scoped)
|
||||
* and localStorage (survives browser restart).
|
||||
*/
|
||||
export function writeActivePath(
|
||||
clientId: string,
|
||||
pointer: ActivePathPointer
|
||||
): void {
|
||||
try {
|
||||
const key = StorageKeys.activePath(clientId)
|
||||
sessionStorage.setItem(key, JSON.stringify(pointer))
|
||||
} catch {
|
||||
// Best effort - ignore errors
|
||||
}
|
||||
const json = JSON.stringify(pointer)
|
||||
writeStorage(sessionStorage, StorageKeys.activePath(clientId), json)
|
||||
writeStorage(
|
||||
localStorage,
|
||||
StorageKeys.lastActivePath(pointer.workspaceId),
|
||||
json
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Reads the open paths pointer from sessionStorage.
|
||||
* Falls back to workspace-based search when clientId changes after reload.
|
||||
* Falls back to workspace-based search when clientId changes after reload,
|
||||
* then to localStorage when sessionStorage is empty (browser restart).
|
||||
*/
|
||||
export function readOpenPaths(
|
||||
clientId: string,
|
||||
targetWorkspaceId?: string
|
||||
): OpenPathsPointer | null {
|
||||
return readSessionPointer<OpenPathsPointer>(
|
||||
StorageKeys.openPaths(clientId),
|
||||
StorageKeys.prefixes.openPaths,
|
||||
targetWorkspaceId
|
||||
return (
|
||||
readSessionPointer<OpenPathsPointer>(
|
||||
StorageKeys.openPaths(clientId),
|
||||
StorageKeys.prefixes.openPaths,
|
||||
targetWorkspaceId
|
||||
) ??
|
||||
(targetWorkspaceId
|
||||
? readLocalPointer<OpenPathsPointer>(
|
||||
StorageKeys.lastOpenPaths(targetWorkspaceId),
|
||||
isValidOpenPathsPointer
|
||||
)
|
||||
: null)
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Writes the open paths pointer to sessionStorage.
|
||||
* Writes the open paths pointer to both sessionStorage (tab-scoped)
|
||||
* and localStorage (survives browser restart).
|
||||
*/
|
||||
export function writeOpenPaths(
|
||||
clientId: string,
|
||||
pointer: OpenPathsPointer
|
||||
): void {
|
||||
const json = JSON.stringify(pointer)
|
||||
writeStorage(sessionStorage, StorageKeys.openPaths(clientId), json)
|
||||
writeStorage(
|
||||
localStorage,
|
||||
StorageKeys.lastOpenPaths(pointer.workspaceId),
|
||||
json
|
||||
)
|
||||
}
|
||||
|
||||
function hasWorkspaceId(obj: Record<string, unknown>): boolean {
|
||||
return typeof obj.workspaceId === 'string'
|
||||
}
|
||||
|
||||
function isValidActivePathPointer(value: unknown): value is ActivePathPointer {
|
||||
if (typeof value !== 'object' || value === null) return false
|
||||
const obj = value as Record<string, unknown>
|
||||
return hasWorkspaceId(obj) && typeof obj.path === 'string'
|
||||
}
|
||||
|
||||
function isValidOpenPathsPointer(value: unknown): value is OpenPathsPointer {
|
||||
if (typeof value !== 'object' || value === null) return false
|
||||
const obj = value as Record<string, unknown>
|
||||
return (
|
||||
hasWorkspaceId(obj) &&
|
||||
Array.isArray(obj.paths) &&
|
||||
typeof obj.activeIndex === 'number'
|
||||
)
|
||||
}
|
||||
|
||||
function readLocalPointer<T>(
|
||||
key: string,
|
||||
validate: (value: unknown) => value is T
|
||||
): T | null {
|
||||
try {
|
||||
const key = StorageKeys.openPaths(clientId)
|
||||
sessionStorage.setItem(key, JSON.stringify(pointer))
|
||||
const json = localStorage.getItem(key)
|
||||
if (!json) return null
|
||||
const parsed = JSON.parse(json)
|
||||
return validate(parsed) ? parsed : null
|
||||
} catch {
|
||||
// Best effort - ignore errors
|
||||
return null
|
||||
}
|
||||
}
|
||||
|
||||
function writeStorage(storage: Storage, key: string, value: string): void {
|
||||
try {
|
||||
storage.setItem(key, value)
|
||||
} catch {
|
||||
// Best effort — silently degrade when storage is full or unavailable
|
||||
}
|
||||
}
|
||||
|
||||
@@ -317,7 +381,9 @@ export function clearAllV2Storage(): void {
|
||||
|
||||
const prefixes = [
|
||||
StorageKeys.prefixes.draftIndex,
|
||||
StorageKeys.prefixes.draftPayload
|
||||
StorageKeys.prefixes.draftPayload,
|
||||
StorageKeys.prefixes.lastActivePath,
|
||||
StorageKeys.prefixes.lastOpenPaths
|
||||
]
|
||||
|
||||
try {
|
||||
|
||||
@@ -72,6 +72,19 @@ export const StorageKeys = {
|
||||
return `Comfy.Workflow.OpenPaths:${clientId}`
|
||||
},
|
||||
|
||||
/**
|
||||
* localStorage copies of tab pointers for cross-session restore.
|
||||
* sessionStorage is per-tab (correct for in-session use) but lost
|
||||
* on browser restart; these keys preserve the last-written state.
|
||||
*/
|
||||
lastActivePath(workspaceId: string): string {
|
||||
return `Comfy.Workflow.LastActivePath:${workspaceId}`
|
||||
},
|
||||
|
||||
lastOpenPaths(workspaceId: string): string {
|
||||
return `Comfy.Workflow.LastOpenPaths:${workspaceId}`
|
||||
},
|
||||
|
||||
/**
|
||||
* Prefix patterns for cleanup operations.
|
||||
*/
|
||||
@@ -79,6 +92,8 @@ export const StorageKeys = {
|
||||
draftIndex: 'Comfy.Workflow.DraftIndex.v2:',
|
||||
draftPayload: 'Comfy.Workflow.Draft.v2:',
|
||||
activePath: 'Comfy.Workflow.ActivePath:',
|
||||
openPaths: 'Comfy.Workflow.OpenPaths:'
|
||||
openPaths: 'Comfy.Workflow.OpenPaths:',
|
||||
lastActivePath: 'Comfy.Workflow.LastActivePath:',
|
||||
lastOpenPaths: 'Comfy.Workflow.LastOpenPaths:'
|
||||
}
|
||||
} as const
|
||||
|
||||
Reference in New Issue
Block a user