mirror of
https://github.com/yusufipk/OpenFrame.git
synced 2026-09-11 17:46:06 +00:00
fix(download): warn before the tab closes mid-download
Bunny and direct downloads are pulled through fetch() so we can save them under our own filename. The browser does not treat that as a download, so closing the tab discarded everything received so far without a word. Register a reference counted beforeunload guard while those transfers are in flight, and while a project manifest is being pulled file by file. Browser owned downloads (same-origin proxy, the over-10GB fallback, asset downloads) survive a tab close on their own and stay unguarded.
This commit is contained in:
@@ -52,6 +52,7 @@ import {
|
||||
type ProjectDownloadManifest,
|
||||
} from '@/lib/client/project-download';
|
||||
import { downloadProgressPercent } from '@/lib/client/download-file';
|
||||
import { beginUnloadGuard } from '@/lib/client/unload-guard';
|
||||
import {
|
||||
createDownloadProgressToast,
|
||||
type DownloadProgressToastHandle,
|
||||
@@ -209,6 +210,7 @@ export function ProjectContentClient({
|
||||
|
||||
setIsDownloading(true);
|
||||
let progressToast: DownloadProgressToastHandle | null = null;
|
||||
let releaseUnloadGuard: (() => void) | null = null;
|
||||
try {
|
||||
const response = await fetch(`/api/projects/${projectId}/download${query}`, {
|
||||
cache: 'no-store',
|
||||
@@ -232,6 +234,9 @@ export function ProjectContentClient({
|
||||
title: `Downloading ${manifest.totalFiles} files`,
|
||||
description: 'Starting…',
|
||||
});
|
||||
// The files are pulled one by one through this tab, so closing it drops
|
||||
// everything that hasn't been saved yet. Warn before that happens.
|
||||
releaseUnloadGuard = beginUnloadGuard();
|
||||
await runProjectDownloadManifest(manifest, (p) => {
|
||||
const percent = downloadProgressPercent({
|
||||
receivedBytes: p.receivedBytes,
|
||||
@@ -250,6 +255,7 @@ export function ProjectContentClient({
|
||||
progressToast?.dismiss();
|
||||
toast.error('Failed to start project download');
|
||||
} finally {
|
||||
releaseUnloadGuard?.();
|
||||
setIsDownloading(false);
|
||||
}
|
||||
},
|
||||
|
||||
@@ -22,6 +22,7 @@ import {
|
||||
createDownloadProgressToast,
|
||||
type DownloadProgressToastHandle,
|
||||
} from '@/components/download-progress-toast';
|
||||
import { beginUnloadGuard } from '@/lib/client/unload-guard';
|
||||
|
||||
function sanitizeDownloadFileName(value: string): string {
|
||||
return value
|
||||
@@ -94,6 +95,7 @@ export function useDownloadActions({ activeVersion, video }: UseDownloadActionsP
|
||||
isDownloadingRef.current = true;
|
||||
setActiveDownloadTarget(target);
|
||||
let progressToast: DownloadProgressToastHandle | null = null;
|
||||
let releaseUnloadGuard: (() => void) | null = null;
|
||||
try {
|
||||
let downloadUrl: string | null = null;
|
||||
|
||||
@@ -167,6 +169,9 @@ export function useDownloadActions({ activeVersion, video }: UseDownloadActionsP
|
||||
title: `Downloading “${baseName}”`,
|
||||
description: 'Starting…',
|
||||
});
|
||||
// The bytes only exist in this tab until the blob is saved, so warn
|
||||
// before the page goes away instead of losing the whole transfer.
|
||||
releaseUnloadGuard = beginUnloadGuard();
|
||||
const saved = await downloadNamedFile(downloadUrl, `${baseName}.${fallbackExt}`, (p) => {
|
||||
progressToast?.update({
|
||||
description: downloadProgressLabel(p),
|
||||
@@ -195,6 +200,7 @@ export function useDownloadActions({ activeVersion, video }: UseDownloadActionsP
|
||||
toast.error('Failed to start download');
|
||||
}
|
||||
} finally {
|
||||
releaseUnloadGuard?.();
|
||||
isDownloadingRef.current = false;
|
||||
setActiveDownloadTarget(null);
|
||||
}
|
||||
|
||||
@@ -0,0 +1,48 @@
|
||||
'use client';
|
||||
|
||||
/**
|
||||
* Downloads that we pull through fetch() live inside the page: closing the tab
|
||||
* (or reloading) throws away every byte received so far and the browser gives no
|
||||
* warning, because as far as it knows nothing is downloading. While one of those
|
||||
* is in flight we register a beforeunload handler so the user gets the native
|
||||
* "leave site?" dialog instead of silently losing the transfer.
|
||||
*
|
||||
* Plain navigation downloads (the `download` attribute / a redirect to the CDN)
|
||||
* are owned by the browser and survive a tab close, so they must NOT be guarded.
|
||||
*/
|
||||
|
||||
let activeCount = 0;
|
||||
|
||||
function handleBeforeUnload(event: BeforeUnloadEvent) {
|
||||
event.preventDefault();
|
||||
// Legacy browsers only show the dialog when returnValue is set; the string
|
||||
// itself is ignored, every browser shows its own wording.
|
||||
event.returnValue = '';
|
||||
}
|
||||
|
||||
/** Registers the guard and returns a release function. Safe to call again while
|
||||
* another download is already guarded — the listener is reference counted and
|
||||
* only detaches once the last one releases. Releasing twice is a no-op. */
|
||||
export function beginUnloadGuard(): () => void {
|
||||
if (typeof window === 'undefined') return () => {};
|
||||
|
||||
if (activeCount === 0) {
|
||||
window.addEventListener('beforeunload', handleBeforeUnload);
|
||||
}
|
||||
activeCount += 1;
|
||||
|
||||
let released = false;
|
||||
return () => {
|
||||
if (released) return;
|
||||
released = true;
|
||||
activeCount -= 1;
|
||||
if (activeCount === 0) {
|
||||
window.removeEventListener('beforeunload', handleBeforeUnload);
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
/** Test helper: number of downloads currently holding the guard. */
|
||||
export function unloadGuardCount(): number {
|
||||
return activeCount;
|
||||
}
|
||||
@@ -652,3 +652,69 @@ describe('useDownloadActions repeated clicks', () => {
|
||||
expect(clicked).toHaveLength(1);
|
||||
});
|
||||
});
|
||||
|
||||
describe('useDownloadActions guarding the tab', () => {
|
||||
function fireBeforeUnload(): BeforeUnloadEvent {
|
||||
const event = new Event('beforeunload', { cancelable: true }) as BeforeUnloadEvent;
|
||||
window.dispatchEvent(event);
|
||||
return event;
|
||||
}
|
||||
|
||||
// Closing the tab used to throw away a half-pulled file without a word,
|
||||
// because the browser has no idea a fetch-driven download is running.
|
||||
it('warns before the tab closes while the bytes are being pulled', async () => {
|
||||
const pending = deferred<unknown>();
|
||||
const harness = renderDownload();
|
||||
fetchMock.mockImplementation((url: string) => {
|
||||
if (typeof url === 'string' && url.includes('prepare=1')) {
|
||||
return Promise.resolve(prepareResponse(true, { data: {} }));
|
||||
}
|
||||
return pending.promise;
|
||||
});
|
||||
|
||||
let started: Promise<void> | undefined;
|
||||
await act(async () => {
|
||||
started = harness.result.current.startDownload();
|
||||
// Let the prepare call settle so the byte fetch is the pending one.
|
||||
await Promise.resolve();
|
||||
});
|
||||
|
||||
expect(fireBeforeUnload().defaultPrevented).toBe(true);
|
||||
|
||||
await act(async () => {
|
||||
pending.resolve(fileResponse());
|
||||
await started;
|
||||
});
|
||||
|
||||
expect(fireBeforeUnload().defaultPrevented).toBe(false);
|
||||
});
|
||||
|
||||
it('releases the guard when the download fails', async () => {
|
||||
downloadResponse = fileResponse({ ok: false });
|
||||
const harness = renderDownload();
|
||||
|
||||
await act(async () => {
|
||||
await harness.result.current.startDownload();
|
||||
});
|
||||
|
||||
expect(fireBeforeUnload().defaultPrevented).toBe(false);
|
||||
});
|
||||
|
||||
// A same-origin proxy download is handed to the browser, which keeps going
|
||||
// after the tab closes, so nothing should block the unload there.
|
||||
it('does not warn for a browser-owned download', async () => {
|
||||
const harness = renderDownload({
|
||||
activeVersion: makeVersion({
|
||||
providerId: 'r2',
|
||||
originalUrl: '/api/upload/video/abc.mp4',
|
||||
}),
|
||||
});
|
||||
|
||||
await act(async () => {
|
||||
await harness.result.current.startDownload();
|
||||
});
|
||||
|
||||
expect(clicked).toHaveLength(1);
|
||||
expect(fireBeforeUnload().defaultPrevented).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,74 @@
|
||||
import { describe, it, expect, beforeEach, afterEach, vi, type MockInstance } from 'vitest';
|
||||
import { beginUnloadGuard, unloadGuardCount } from '@/lib/client/unload-guard';
|
||||
|
||||
let addSpy: MockInstance<typeof window.addEventListener>;
|
||||
let removeSpy: MockInstance<typeof window.removeEventListener>;
|
||||
|
||||
/** True when something cancelled the unload, which is what makes the browser
|
||||
* show its "leave site?" dialog. */
|
||||
function unloadWasBlocked(): boolean {
|
||||
const event = new Event('beforeunload', { cancelable: true });
|
||||
window.dispatchEvent(event);
|
||||
return event.defaultPrevented;
|
||||
}
|
||||
|
||||
function listenerCalls(spy: MockInstance<typeof window.addEventListener>): number {
|
||||
return spy.mock.calls.filter(([type]) => type === 'beforeunload').length;
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
addSpy = vi.spyOn(window, 'addEventListener');
|
||||
removeSpy = vi.spyOn(window, 'removeEventListener');
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
// A leaked guard would block the unload for the rest of the session, so a
|
||||
// test that leaves one behind must fail here rather than in the next test.
|
||||
expect(unloadGuardCount()).toBe(0);
|
||||
});
|
||||
|
||||
describe('beginUnloadGuard', () => {
|
||||
it('cancels the unload while a download holds it', () => {
|
||||
const release = beginUnloadGuard();
|
||||
const blocked = unloadWasBlocked();
|
||||
release();
|
||||
|
||||
expect(blocked).toBe(true);
|
||||
});
|
||||
|
||||
it('lets the page go once the download is released', () => {
|
||||
beginUnloadGuard()();
|
||||
|
||||
expect(unloadWasBlocked()).toBe(false);
|
||||
});
|
||||
|
||||
it('keeps the listener until the last concurrent download releases', () => {
|
||||
const releaseA = beginUnloadGuard();
|
||||
const releaseB = beginUnloadGuard();
|
||||
expect(listenerCalls(addSpy)).toBe(1);
|
||||
|
||||
releaseA();
|
||||
const stillBlocked = unloadWasBlocked();
|
||||
releaseB();
|
||||
|
||||
expect(stillBlocked).toBe(true);
|
||||
expect(listenerCalls(removeSpy)).toBe(1);
|
||||
expect(unloadWasBlocked()).toBe(false);
|
||||
});
|
||||
|
||||
// The download hook releases from a finally block, and a caller could hold
|
||||
// the returned function longer; a double release must not drop a guard
|
||||
// another download still holds.
|
||||
it('ignores a second release', () => {
|
||||
const releaseA = beginUnloadGuard();
|
||||
const releaseB = beginUnloadGuard();
|
||||
|
||||
releaseA();
|
||||
releaseA();
|
||||
const stillBlocked = unloadWasBlocked();
|
||||
releaseB();
|
||||
|
||||
expect(stillBlocked).toBe(true);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user