From a94574d37951547307b322cdf5596df2f23cb23a Mon Sep 17 00:00:00 2001 From: Christian Byrne Date: Mon, 23 Feb 2026 21:28:16 -0800 Subject: [PATCH] fix: open image in new tab on cloud fetches as blob to avoid GCS auto-download (#9122) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Fix "Open Image" on cloud opening a new tab that auto-downloads the asset instead of displaying it inline. ## Changes - **What**: Add `openFileInNewTab()` to `downloadUtil.ts` that fetches cross-origin URLs as blobs before opening in a new tab, avoiding GCS `Content-Disposition: attachment` redirects. Opens the blank tab synchronously to preserve user-gesture activation (avoiding popup blockers), then navigates to a blob URL once the fetch completes. Blob URLs are revoked after 60s or immediately if the tab was closed. Update both call sites (`useImageMenuOptions` and `litegraphService`) to use the new function. ## Review Focus - The synchronous `window.open('', '_blank')` before the async fetch is intentional to preserve user-gesture context and avoid popup blockers. - Blob URL revocation strategy: 60s timeout for successful opens, immediate revoke if tab was closed, tab closed on fetch failure. - Shared `fetchAsBlob()` helper is also used by the existing `downloadViaBlobFetch`. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9122-fix-open-image-in-new-tab-on-cloud-fetches-as-blob-to-avoid-GCS-auto-download-3106d73d365081a3bfa6eb7d77fde99f) by [Unito](https://www.unito.io) --- src/base/common/downloadUtil.test.ts | 129 +++++++++++++++++-- src/base/common/downloadUtil.ts | 64 ++++++++- src/composables/graph/useImageMenuOptions.ts | 4 +- src/locales/en/main.json | 1 + src/services/litegraphService.ts | 4 +- 5 files changed, 182 insertions(+), 20 deletions(-) diff --git a/src/base/common/downloadUtil.test.ts b/src/base/common/downloadUtil.test.ts index fc5c32a86d..e601d870c9 100644 --- a/src/base/common/downloadUtil.test.ts +++ b/src/base/common/downloadUtil.test.ts @@ -2,17 +2,28 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { downloadFile, - extractFilenameFromContentDisposition + extractFilenameFromContentDisposition, + openFileInNewTab } from '@/base/common/downloadUtil' -let mockIsCloud = false +const { mockIsCloud } = vi.hoisted(() => ({ + mockIsCloud: { value: false } +})) vi.mock('@/platform/distribution/types', () => ({ get isCloud() { - return mockIsCloud + return mockIsCloud.value } })) +vi.mock('@/i18n', () => ({ + t: (key: string) => key +})) + +vi.mock('@/platform/updates/common/toastStore', () => ({ + useToastStore: vi.fn(() => ({ addAlert: vi.fn() })) +})) + // Global stubs const createObjectURLSpy = vi .spyOn(URL, 'createObjectURL') @@ -26,7 +37,7 @@ describe('downloadUtil', () => { let fetchMock: ReturnType beforeEach(() => { - mockIsCloud = false + mockIsCloud.value = false fetchMock = vi.fn() vi.stubGlobal('fetch', fetchMock) createObjectURLSpy.mockClear().mockReturnValue('blob:mock-url') @@ -154,7 +165,7 @@ describe('downloadUtil', () => { }) it('streams downloads via blob when running in cloud', async () => { - mockIsCloud = true + mockIsCloud.value = true const testUrl = 'https://storage.googleapis.com/bucket/file.bin' const blob = new Blob(['test']) const blobFn = vi.fn().mockResolvedValue(blob) @@ -173,6 +184,7 @@ describe('downloadUtil', () => { expect(fetchMock).toHaveBeenCalledWith(testUrl) const fetchPromise = fetchMock.mock.results[0].value as Promise await fetchPromise + await Promise.resolve() // let fetchAsBlob return const blobPromise = blobFn.mock.results[0].value as Promise await blobPromise await Promise.resolve() @@ -183,7 +195,7 @@ describe('downloadUtil', () => { }) it('logs an error when cloud fetch fails', async () => { - mockIsCloud = true + mockIsCloud.value = true const testUrl = 'https://storage.googleapis.com/bucket/missing.bin' const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) fetchMock.mockResolvedValue({ @@ -197,14 +209,15 @@ describe('downloadUtil', () => { expect(fetchMock).toHaveBeenCalledWith(testUrl) const fetchPromise = fetchMock.mock.results[0].value as Promise await fetchPromise - await Promise.resolve() + await Promise.resolve() // let fetchAsBlob throw + await Promise.resolve() // let .catch handler run expect(consoleSpy).toHaveBeenCalled() expect(createObjectURLSpy).not.toHaveBeenCalled() consoleSpy.mockRestore() }) it('uses filename from Content-Disposition header in cloud mode', async () => { - mockIsCloud = true + mockIsCloud.value = true const testUrl = 'https://storage.googleapis.com/bucket/abc123.png' const blob = new Blob(['test']) const blobFn = vi.fn().mockResolvedValue(blob) @@ -223,6 +236,7 @@ describe('downloadUtil', () => { expect(fetchMock).toHaveBeenCalledWith(testUrl) const fetchPromise = fetchMock.mock.results[0].value as Promise await fetchPromise + await Promise.resolve() // let fetchAsBlob return const blobPromise = blobFn.mock.results[0].value as Promise await blobPromise await Promise.resolve() @@ -231,7 +245,7 @@ describe('downloadUtil', () => { }) it('uses RFC 5987 filename from Content-Disposition header', async () => { - mockIsCloud = true + mockIsCloud.value = true const testUrl = 'https://storage.googleapis.com/bucket/abc123.png' const blob = new Blob(['test']) const blobFn = vi.fn().mockResolvedValue(blob) @@ -253,6 +267,7 @@ describe('downloadUtil', () => { const fetchPromise = fetchMock.mock.results[0].value as Promise await fetchPromise + await Promise.resolve() // let fetchAsBlob return const blobPromise = blobFn.mock.results[0].value as Promise await blobPromise await Promise.resolve() @@ -260,7 +275,7 @@ describe('downloadUtil', () => { }) it('falls back to provided filename when Content-Disposition is missing', async () => { - mockIsCloud = true + mockIsCloud.value = true const testUrl = 'https://storage.googleapis.com/bucket/abc123.png' const blob = new Blob(['test']) const blobFn = vi.fn().mockResolvedValue(blob) @@ -278,6 +293,7 @@ describe('downloadUtil', () => { const fetchPromise = fetchMock.mock.results[0].value as Promise await fetchPromise + await Promise.resolve() // let fetchAsBlob return const blobPromise = blobFn.mock.results[0].value as Promise await blobPromise await Promise.resolve() @@ -285,6 +301,99 @@ describe('downloadUtil', () => { }) }) + describe('openFileInNewTab', () => { + let windowOpenSpy: ReturnType + + beforeEach(() => { + vi.useFakeTimers() + windowOpenSpy = vi.spyOn(window, 'open').mockImplementation(() => null) + }) + + afterEach(() => { + vi.useRealTimers() + }) + + it('opens URL directly when not in cloud mode', async () => { + mockIsCloud.value = false + const testUrl = 'https://example.com/image.png' + + await openFileInNewTab(testUrl) + + expect(windowOpenSpy).toHaveBeenCalledWith(testUrl, '_blank') + expect(fetchMock).not.toHaveBeenCalled() + }) + + it('opens blank tab synchronously then navigates to blob URL in cloud mode', async () => { + mockIsCloud.value = true + const testUrl = 'https://storage.googleapis.com/bucket/image.png' + const blob = new Blob(['test'], { type: 'image/png' }) + const mockTab = { location: { href: '' }, closed: false, close: vi.fn() } + windowOpenSpy.mockReturnValue(mockTab as unknown as Window) + fetchMock.mockResolvedValue({ + ok: true, + blob: vi.fn().mockResolvedValue(blob) + } as unknown as Response) + + await openFileInNewTab(testUrl) + + expect(windowOpenSpy).toHaveBeenCalledWith('', '_blank') + expect(fetchMock).toHaveBeenCalledWith(testUrl) + expect(createObjectURLSpy).toHaveBeenCalledWith(blob) + expect(mockTab.location.href).toBe('blob:mock-url') + }) + + it('revokes blob URL after timeout in cloud mode', async () => { + mockIsCloud.value = true + const blob = new Blob(['test'], { type: 'image/png' }) + const mockTab = { location: { href: '' }, closed: false, close: vi.fn() } + windowOpenSpy.mockReturnValue(mockTab as unknown as Window) + fetchMock.mockResolvedValue({ + ok: true, + blob: vi.fn().mockResolvedValue(blob) + } as unknown as Response) + + await openFileInNewTab('https://example.com/image.png') + + expect(revokeObjectURLSpy).not.toHaveBeenCalled() + vi.advanceTimersByTime(60_000) + expect(revokeObjectURLSpy).toHaveBeenCalledWith('blob:mock-url') + }) + + it('closes blank tab and logs error when cloud fetch fails', async () => { + mockIsCloud.value = true + const testUrl = 'https://storage.googleapis.com/bucket/missing.png' + const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) + const mockTab = { location: { href: '' }, closed: false, close: vi.fn() } + windowOpenSpy.mockReturnValue(mockTab as unknown as Window) + fetchMock.mockResolvedValue({ + ok: false, + status: 404 + } as unknown as Response) + + await openFileInNewTab(testUrl) + + expect(mockTab.close).toHaveBeenCalled() + expect(consoleSpy).toHaveBeenCalled() + consoleSpy.mockRestore() + }) + + it('revokes blob URL immediately if tab was closed by user', async () => { + mockIsCloud.value = true + const blob = new Blob(['test'], { type: 'image/png' }) + const mockTab = { location: { href: '' }, closed: true, close: vi.fn() } + windowOpenSpy.mockReturnValue(mockTab as unknown as Window) + fetchMock.mockResolvedValue({ + ok: true, + blob: vi.fn().mockResolvedValue(blob) + } as unknown as Response) + + await openFileInNewTab('https://example.com/image.png') + + expect(revokeObjectURLSpy).toHaveBeenCalledWith('blob:mock-url') + expect(mockTab.location.href).toBe('') + }) + }) + describe('extractFilenameFromContentDisposition', () => { it('returns null for null header', () => { expect(extractFilenameFromContentDisposition(null)).toBeNull() diff --git a/src/base/common/downloadUtil.ts b/src/base/common/downloadUtil.ts index 78bb1ca565..78c208c776 100644 --- a/src/base/common/downloadUtil.ts +++ b/src/base/common/downloadUtil.ts @@ -1,7 +1,9 @@ /** * Utility functions for downloading files */ +import { t } from '@/i18n' import { isCloud } from '@/platform/distribution/types' +import { useToastStore } from '@/platform/updates/common/toastStore' // Constants const DEFAULT_DOWNLOAD_FILENAME = 'download.png' @@ -112,14 +114,23 @@ export function extractFilenameFromContentDisposition( return null } -const downloadViaBlobFetch = async ( +/** + * Fetch a URL and return its body as a Blob. + * Shared by download and open-in-new-tab cloud paths. + */ +async function fetchAsBlob(url: string): Promise { + const response = await fetch(url) + if (!response.ok) { + throw new Error(`Failed to fetch ${url}: ${response.status}`) + } + return response +} + +async function downloadViaBlobFetch( href: string, fallbackFilename: string -): Promise => { - const response = await fetch(href) - if (!response.ok) { - throw new Error(`Failed to fetch ${href}: ${response.status}`) - } +): Promise { + const response = await fetchAsBlob(href) // Try to get filename from Content-Disposition header (set by backend) const contentDisposition = response.headers.get('Content-Disposition') @@ -129,3 +140,44 @@ const downloadViaBlobFetch = async ( const blob = await response.blob() downloadBlob(headerFilename ?? fallbackFilename, blob) } + +/** + * Open a file URL in a new browser tab. + * On cloud, fetches the resource as a blob first to avoid GCS redirects + * that would trigger an auto-download instead of displaying the file. + * + * Opens the tab synchronously to preserve the user-gesture context + * (browsers block window.open after an await), then navigates it to + * the blob URL once the fetch completes. + */ +export async function openFileInNewTab(url: string): Promise { + if (!isCloud) { + window.open(url, '_blank') + return + } + + // Open immediately to preserve user-gesture activation. + const tab = window.open('', '_blank') + + try { + const response = await fetchAsBlob(url) + const blob = await response.blob() + const blobUrl = URL.createObjectURL(blob) + + if (tab && !tab.closed) { + tab.location.href = blobUrl + // Revoke after the tab has had time to load the blob. + setTimeout(() => URL.revokeObjectURL(blobUrl), 60_000) + } else { + URL.revokeObjectURL(blobUrl) + } + } catch (error) { + tab?.close() + console.error('Failed to open image:', error) + useToastStore().addAlert( + t('toastMessages.errorOpenImage', { + error: error instanceof Error ? error.message : String(error) + }) + ) + } +} diff --git a/src/composables/graph/useImageMenuOptions.ts b/src/composables/graph/useImageMenuOptions.ts index 0eddf3d00e..a343563773 100644 --- a/src/composables/graph/useImageMenuOptions.ts +++ b/src/composables/graph/useImageMenuOptions.ts @@ -1,6 +1,6 @@ import { useI18n } from 'vue-i18n' -import { downloadFile } from '@/base/common/downloadUtil' +import { downloadFile, openFileInNewTab } from '@/base/common/downloadUtil' import type { LGraphNode } from '@/lib/litegraph/src/LGraphNode' import { useCommandStore } from '@/stores/commandStore' @@ -23,7 +23,7 @@ export function useImageMenuOptions() { if (!img) return const url = new URL(img.src) url.searchParams.delete('preview') - window.open(url.toString(), '_blank') + void openFileInNewTab(url.toString()) } const copyImage = async (node: LGraphNode) => { diff --git a/src/locales/en/main.json b/src/locales/en/main.json index b365db5a3e..aa494011b0 100644 --- a/src/locales/en/main.json +++ b/src/locales/en/main.json @@ -1910,6 +1910,7 @@ "nodeDefinitionsUpdated": "Node definitions updated", "errorSaveSetting": "Error saving setting {id}: {err}", "errorCopyImage": "Error copying image: {error}", + "errorOpenImage": "Error opening image: {error}", "noTemplatesToExport": "No templates to export", "failedToFetchLogs": "Failed to fetch server logs", "migrateToLitegraphReroute": "Reroute nodes will be removed in future versions. Click to migrate to litegraph-native reroute.", diff --git a/src/services/litegraphService.ts b/src/services/litegraphService.ts index 1a2ec6e3ad..f66c8e160f 100644 --- a/src/services/litegraphService.ts +++ b/src/services/litegraphService.ts @@ -1,6 +1,6 @@ import _ from 'es-toolkit/compat' -import { downloadFile } from '@/base/common/downloadUtil' +import { downloadFile, openFileInNewTab } from '@/base/common/downloadUtil' import { useSelectedLiteGraphItems } from '@/composables/canvas/useSelectedLiteGraphItems' import { useSubgraphOperations } from '@/composables/graph/useSubgraphOperations' import { useNodeAnimatedImage } from '@/composables/node/useNodeAnimatedImage' @@ -686,7 +686,7 @@ export const useLitegraphService = () => { callback: () => { const url = new URL(img.src) url.searchParams.delete('preview') - window.open(url, '_blank') + void openFileInNewTab(url.toString()) } }, ...getCopyImageOption(img),