Skip to content

Commit a94574d

Browse files
fix: open image in new tab on cloud fetches as blob to avoid GCS auto-download (#9122)
## 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)
1 parent 78fe639 commit a94574d

File tree

5 files changed

+182
-20
lines changed

5 files changed

+182
-20
lines changed

src/base/common/downloadUtil.test.ts

Lines changed: 119 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,28 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
22

33
import {
44
downloadFile,
5-
extractFilenameFromContentDisposition
5+
extractFilenameFromContentDisposition,
6+
openFileInNewTab
67
} from '@/base/common/downloadUtil'
78

8-
let mockIsCloud = false
9+
const { mockIsCloud } = vi.hoisted(() => ({
10+
mockIsCloud: { value: false }
11+
}))
912

1013
vi.mock('@/platform/distribution/types', () => ({
1114
get isCloud() {
12-
return mockIsCloud
15+
return mockIsCloud.value
1316
}
1417
}))
1518

19+
vi.mock('@/i18n', () => ({
20+
t: (key: string) => key
21+
}))
22+
23+
vi.mock('@/platform/updates/common/toastStore', () => ({
24+
useToastStore: vi.fn(() => ({ addAlert: vi.fn() }))
25+
}))
26+
1627
// Global stubs
1728
const createObjectURLSpy = vi
1829
.spyOn(URL, 'createObjectURL')
@@ -26,7 +37,7 @@ describe('downloadUtil', () => {
2637
let fetchMock: ReturnType<typeof vi.fn>
2738

2839
beforeEach(() => {
29-
mockIsCloud = false
40+
mockIsCloud.value = false
3041
fetchMock = vi.fn()
3142
vi.stubGlobal('fetch', fetchMock)
3243
createObjectURLSpy.mockClear().mockReturnValue('blob:mock-url')
@@ -154,7 +165,7 @@ describe('downloadUtil', () => {
154165
})
155166

156167
it('streams downloads via blob when running in cloud', async () => {
157-
mockIsCloud = true
168+
mockIsCloud.value = true
158169
const testUrl = 'https://storage.googleapis.com/bucket/file.bin'
159170
const blob = new Blob(['test'])
160171
const blobFn = vi.fn().mockResolvedValue(blob)
@@ -173,6 +184,7 @@ describe('downloadUtil', () => {
173184
expect(fetchMock).toHaveBeenCalledWith(testUrl)
174185
const fetchPromise = fetchMock.mock.results[0].value as Promise<Response>
175186
await fetchPromise
187+
await Promise.resolve() // let fetchAsBlob return
176188
const blobPromise = blobFn.mock.results[0].value as Promise<Blob>
177189
await blobPromise
178190
await Promise.resolve()
@@ -183,7 +195,7 @@ describe('downloadUtil', () => {
183195
})
184196

185197
it('logs an error when cloud fetch fails', async () => {
186-
mockIsCloud = true
198+
mockIsCloud.value = true
187199
const testUrl = 'https://storage.googleapis.com/bucket/missing.bin'
188200
const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {})
189201
fetchMock.mockResolvedValue({
@@ -197,14 +209,15 @@ describe('downloadUtil', () => {
197209
expect(fetchMock).toHaveBeenCalledWith(testUrl)
198210
const fetchPromise = fetchMock.mock.results[0].value as Promise<Response>
199211
await fetchPromise
200-
await Promise.resolve()
212+
await Promise.resolve() // let fetchAsBlob throw
213+
await Promise.resolve() // let .catch handler run
201214
expect(consoleSpy).toHaveBeenCalled()
202215
expect(createObjectURLSpy).not.toHaveBeenCalled()
203216
consoleSpy.mockRestore()
204217
})
205218

206219
it('uses filename from Content-Disposition header in cloud mode', async () => {
207-
mockIsCloud = true
220+
mockIsCloud.value = true
208221
const testUrl = 'https://storage.googleapis.com/bucket/abc123.png'
209222
const blob = new Blob(['test'])
210223
const blobFn = vi.fn().mockResolvedValue(blob)
@@ -223,6 +236,7 @@ describe('downloadUtil', () => {
223236
expect(fetchMock).toHaveBeenCalledWith(testUrl)
224237
const fetchPromise = fetchMock.mock.results[0].value as Promise<Response>
225238
await fetchPromise
239+
await Promise.resolve() // let fetchAsBlob return
226240
const blobPromise = blobFn.mock.results[0].value as Promise<Blob>
227241
await blobPromise
228242
await Promise.resolve()
@@ -231,7 +245,7 @@ describe('downloadUtil', () => {
231245
})
232246

233247
it('uses RFC 5987 filename from Content-Disposition header', async () => {
234-
mockIsCloud = true
248+
mockIsCloud.value = true
235249
const testUrl = 'https://storage.googleapis.com/bucket/abc123.png'
236250
const blob = new Blob(['test'])
237251
const blobFn = vi.fn().mockResolvedValue(blob)
@@ -253,14 +267,15 @@ describe('downloadUtil', () => {
253267

254268
const fetchPromise = fetchMock.mock.results[0].value as Promise<Response>
255269
await fetchPromise
270+
await Promise.resolve() // let fetchAsBlob return
256271
const blobPromise = blobFn.mock.results[0].value as Promise<Blob>
257272
await blobPromise
258273
await Promise.resolve()
259274
expect(mockLink.download).toBe('中文.png')
260275
})
261276

262277
it('falls back to provided filename when Content-Disposition is missing', async () => {
263-
mockIsCloud = true
278+
mockIsCloud.value = true
264279
const testUrl = 'https://storage.googleapis.com/bucket/abc123.png'
265280
const blob = new Blob(['test'])
266281
const blobFn = vi.fn().mockResolvedValue(blob)
@@ -278,13 +293,107 @@ describe('downloadUtil', () => {
278293

279294
const fetchPromise = fetchMock.mock.results[0].value as Promise<Response>
280295
await fetchPromise
296+
await Promise.resolve() // let fetchAsBlob return
281297
const blobPromise = blobFn.mock.results[0].value as Promise<Blob>
282298
await blobPromise
283299
await Promise.resolve()
284300
expect(mockLink.download).toBe('my-fallback.png')
285301
})
286302
})
287303

304+
describe('openFileInNewTab', () => {
305+
let windowOpenSpy: ReturnType<typeof vi.spyOn>
306+
307+
beforeEach(() => {
308+
vi.useFakeTimers()
309+
windowOpenSpy = vi.spyOn(window, 'open').mockImplementation(() => null)
310+
})
311+
312+
afterEach(() => {
313+
vi.useRealTimers()
314+
})
315+
316+
it('opens URL directly when not in cloud mode', async () => {
317+
mockIsCloud.value = false
318+
const testUrl = 'https://example.com/image.png'
319+
320+
await openFileInNewTab(testUrl)
321+
322+
expect(windowOpenSpy).toHaveBeenCalledWith(testUrl, '_blank')
323+
expect(fetchMock).not.toHaveBeenCalled()
324+
})
325+
326+
it('opens blank tab synchronously then navigates to blob URL in cloud mode', async () => {
327+
mockIsCloud.value = true
328+
const testUrl = 'https://storage.googleapis.com/bucket/image.png'
329+
const blob = new Blob(['test'], { type: 'image/png' })
330+
const mockTab = { location: { href: '' }, closed: false, close: vi.fn() }
331+
windowOpenSpy.mockReturnValue(mockTab as unknown as Window)
332+
fetchMock.mockResolvedValue({
333+
ok: true,
334+
blob: vi.fn().mockResolvedValue(blob)
335+
} as unknown as Response)
336+
337+
await openFileInNewTab(testUrl)
338+
339+
expect(windowOpenSpy).toHaveBeenCalledWith('', '_blank')
340+
expect(fetchMock).toHaveBeenCalledWith(testUrl)
341+
expect(createObjectURLSpy).toHaveBeenCalledWith(blob)
342+
expect(mockTab.location.href).toBe('blob:mock-url')
343+
})
344+
345+
it('revokes blob URL after timeout in cloud mode', async () => {
346+
mockIsCloud.value = true
347+
const blob = new Blob(['test'], { type: 'image/png' })
348+
const mockTab = { location: { href: '' }, closed: false, close: vi.fn() }
349+
windowOpenSpy.mockReturnValue(mockTab as unknown as Window)
350+
fetchMock.mockResolvedValue({
351+
ok: true,
352+
blob: vi.fn().mockResolvedValue(blob)
353+
} as unknown as Response)
354+
355+
await openFileInNewTab('https://example.com/image.png')
356+
357+
expect(revokeObjectURLSpy).not.toHaveBeenCalled()
358+
vi.advanceTimersByTime(60_000)
359+
expect(revokeObjectURLSpy).toHaveBeenCalledWith('blob:mock-url')
360+
})
361+
362+
it('closes blank tab and logs error when cloud fetch fails', async () => {
363+
mockIsCloud.value = true
364+
const testUrl = 'https://storage.googleapis.com/bucket/missing.png'
365+
const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {})
366+
const mockTab = { location: { href: '' }, closed: false, close: vi.fn() }
367+
windowOpenSpy.mockReturnValue(mockTab as unknown as Window)
368+
fetchMock.mockResolvedValue({
369+
ok: false,
370+
status: 404
371+
} as unknown as Response)
372+
373+
await openFileInNewTab(testUrl)
374+
375+
expect(mockTab.close).toHaveBeenCalled()
376+
expect(consoleSpy).toHaveBeenCalled()
377+
consoleSpy.mockRestore()
378+
})
379+
380+
it('revokes blob URL immediately if tab was closed by user', async () => {
381+
mockIsCloud.value = true
382+
const blob = new Blob(['test'], { type: 'image/png' })
383+
const mockTab = { location: { href: '' }, closed: true, close: vi.fn() }
384+
windowOpenSpy.mockReturnValue(mockTab as unknown as Window)
385+
fetchMock.mockResolvedValue({
386+
ok: true,
387+
blob: vi.fn().mockResolvedValue(blob)
388+
} as unknown as Response)
389+
390+
await openFileInNewTab('https://example.com/image.png')
391+
392+
expect(revokeObjectURLSpy).toHaveBeenCalledWith('blob:mock-url')
393+
expect(mockTab.location.href).toBe('')
394+
})
395+
})
396+
288397
describe('extractFilenameFromContentDisposition', () => {
289398
it('returns null for null header', () => {
290399
expect(extractFilenameFromContentDisposition(null)).toBeNull()

src/base/common/downloadUtil.ts

Lines changed: 58 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
/**
22
* Utility functions for downloading files
33
*/
4+
import { t } from '@/i18n'
45
import { isCloud } from '@/platform/distribution/types'
6+
import { useToastStore } from '@/platform/updates/common/toastStore'
57

68
// Constants
79
const DEFAULT_DOWNLOAD_FILENAME = 'download.png'
@@ -112,14 +114,23 @@ export function extractFilenameFromContentDisposition(
112114
return null
113115
}
114116

115-
const downloadViaBlobFetch = async (
116-
href: string,
117-
fallbackFilename: string
118-
): Promise<void> => {
119-
const response = await fetch(href)
117+
/**
118+
* Fetch a URL and return its body as a Blob.
119+
* Shared by download and open-in-new-tab cloud paths.
120+
*/
121+
async function fetchAsBlob(url: string): Promise<Response> {
122+
const response = await fetch(url)
120123
if (!response.ok) {
121-
throw new Error(`Failed to fetch ${href}: ${response.status}`)
124+
throw new Error(`Failed to fetch ${url}: ${response.status}`)
122125
}
126+
return response
127+
}
128+
129+
async function downloadViaBlobFetch(
130+
href: string,
131+
fallbackFilename: string
132+
): Promise<void> {
133+
const response = await fetchAsBlob(href)
123134

124135
// Try to get filename from Content-Disposition header (set by backend)
125136
const contentDisposition = response.headers.get('Content-Disposition')
@@ -129,3 +140,44 @@ const downloadViaBlobFetch = async (
129140
const blob = await response.blob()
130141
downloadBlob(headerFilename ?? fallbackFilename, blob)
131142
}
143+
144+
/**
145+
* Open a file URL in a new browser tab.
146+
* On cloud, fetches the resource as a blob first to avoid GCS redirects
147+
* that would trigger an auto-download instead of displaying the file.
148+
*
149+
* Opens the tab synchronously to preserve the user-gesture context
150+
* (browsers block window.open after an await), then navigates it to
151+
* the blob URL once the fetch completes.
152+
*/
153+
export async function openFileInNewTab(url: string): Promise<void> {
154+
if (!isCloud) {
155+
window.open(url, '_blank')
156+
return
157+
}
158+
159+
// Open immediately to preserve user-gesture activation.
160+
const tab = window.open('', '_blank')
161+
162+
try {
163+
const response = await fetchAsBlob(url)
164+
const blob = await response.blob()
165+
const blobUrl = URL.createObjectURL(blob)
166+
167+
if (tab && !tab.closed) {
168+
tab.location.href = blobUrl
169+
// Revoke after the tab has had time to load the blob.
170+
setTimeout(() => URL.revokeObjectURL(blobUrl), 60_000)
171+
} else {
172+
URL.revokeObjectURL(blobUrl)
173+
}
174+
} catch (error) {
175+
tab?.close()
176+
console.error('Failed to open image:', error)
177+
useToastStore().addAlert(
178+
t('toastMessages.errorOpenImage', {
179+
error: error instanceof Error ? error.message : String(error)
180+
})
181+
)
182+
}
183+
}

src/composables/graph/useImageMenuOptions.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { useI18n } from 'vue-i18n'
22

3-
import { downloadFile } from '@/base/common/downloadUtil'
3+
import { downloadFile, openFileInNewTab } from '@/base/common/downloadUtil'
44
import type { LGraphNode } from '@/lib/litegraph/src/LGraphNode'
55
import { useCommandStore } from '@/stores/commandStore'
66

@@ -23,7 +23,7 @@ export function useImageMenuOptions() {
2323
if (!img) return
2424
const url = new URL(img.src)
2525
url.searchParams.delete('preview')
26-
window.open(url.toString(), '_blank')
26+
void openFileInNewTab(url.toString())
2727
}
2828

2929
const copyImage = async (node: LGraphNode) => {

src/locales/en/main.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1910,6 +1910,7 @@
19101910
"nodeDefinitionsUpdated": "Node definitions updated",
19111911
"errorSaveSetting": "Error saving setting {id}: {err}",
19121912
"errorCopyImage": "Error copying image: {error}",
1913+
"errorOpenImage": "Error opening image: {error}",
19131914
"noTemplatesToExport": "No templates to export",
19141915
"failedToFetchLogs": "Failed to fetch server logs",
19151916
"migrateToLitegraphReroute": "Reroute nodes will be removed in future versions. Click to migrate to litegraph-native reroute.",

src/services/litegraphService.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import _ from 'es-toolkit/compat'
22

3-
import { downloadFile } from '@/base/common/downloadUtil'
3+
import { downloadFile, openFileInNewTab } from '@/base/common/downloadUtil'
44
import { useSelectedLiteGraphItems } from '@/composables/canvas/useSelectedLiteGraphItems'
55
import { useSubgraphOperations } from '@/composables/graph/useSubgraphOperations'
66
import { useNodeAnimatedImage } from '@/composables/node/useNodeAnimatedImage'
@@ -686,7 +686,7 @@ export const useLitegraphService = () => {
686686
callback: () => {
687687
const url = new URL(img.src)
688688
url.searchParams.delete('preview')
689-
window.open(url, '_blank')
689+
void openFileInNewTab(url.toString())
690690
}
691691
},
692692
...getCopyImageOption(img),

0 commit comments

Comments
 (0)