fix(lightbox): eliminate download lag on Android by skipping the blob round-trip
`savePhotoToDevice` previously buffered the full image through JS as a Blob on every platform before clicking <a download>. On cellular this added ~5s of dead air between the button press and the browser's download dialog, prompting users to re-click and produce duplicate downloads (#554 follow-up, post-#556). The blob round-trip is only required for the iOS Web Share path (`navigator.share({files})` needs File objects in hand). On Android and desktop the browser can fetch the download URL itself and show its own progress in the notification shade — instantly. So iOS keeps the existing flow; everywhere else gets a direct anchor navigation. The new `triggerDirectDownload` helper uses `api.getUri()` so the path also works in split-origin deployments (where the existing hardcoded `/api/...` pattern used by `downloadAllPhotos` would 404). Tests updated: Android / desktop / regular-Mac branches now assert that `fetchPhotoBlob` is NOT called and `triggerDirectDownload` is invoked with a `/gallery/{slug}/download/{id}` URL. iOS tests unchanged.
This commit is contained in:
@@ -51,6 +51,7 @@ describe('galleryService.savePhotoToDevice — iOS gating (#554)', () => {
|
||||
|
||||
vi.spyOn(galleryService, 'fetchPhotoBlob').mockResolvedValue(fetchedBlob as any);
|
||||
vi.spyOn(galleryService, 'triggerBrowserDownload').mockImplementation(() => undefined);
|
||||
vi.spyOn(galleryService, 'triggerDirectDownload').mockImplementation(() => undefined);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
@@ -69,13 +70,18 @@ describe('galleryService.savePhotoToDevice — iOS gating (#554)', () => {
|
||||
expect(share).toHaveBeenCalledTimes(1);
|
||||
expect(canShare).toHaveBeenCalledWith({ files: expect.any(Array) });
|
||||
expect(galleryService.triggerBrowserDownload).not.toHaveBeenCalled();
|
||||
expect(galleryService.triggerDirectDownload).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('falls through to a regular download on Android even when canShare({files}) is true', async () => {
|
||||
// This is the bug #554 fixes — canShare is true on Chrome Android too,
|
||||
// but the Android share sheet has no "Save Image" action so the user
|
||||
// sees a useless app-picker. The gating must be UA-based, not
|
||||
// capability-based.
|
||||
it('navigates straight to the download URL on Android (no blob round-trip)', async () => {
|
||||
// #554 fix: canShare is true on Chrome Android but the share sheet
|
||||
// has no "Save Image" action, so the share path is iOS-only. The
|
||||
// follow-up issue (Rekoo-PS, post-#556) was that the Android
|
||||
// fallback fetched the blob through JS before clicking <a download>,
|
||||
// adding ~5s of dead air before the browser's download UI appeared
|
||||
// and prompting users to re-click. Going straight to the download
|
||||
// URL hands the fetch to the browser, which shows its own progress
|
||||
// immediately — no spinner needed.
|
||||
const share = vi.fn().mockResolvedValue(undefined);
|
||||
const canShare = vi.fn().mockReturnValue(true);
|
||||
installNavigator({ userAgent: ANDROID_UA, share, canShare });
|
||||
@@ -83,21 +89,23 @@ describe('galleryService.savePhotoToDevice — iOS gating (#554)', () => {
|
||||
await galleryService.savePhotoToDevice('slug', 1, 'fallback.jpg');
|
||||
|
||||
expect(share).not.toHaveBeenCalled();
|
||||
// canShare may or may not be probed on Android; what matters is the
|
||||
// share() call doesn't happen and a download is triggered instead.
|
||||
expect(galleryService.triggerBrowserDownload).toHaveBeenCalledTimes(1);
|
||||
expect(galleryService.triggerBrowserDownload).toHaveBeenCalledWith(
|
||||
fetchedBlob.blob,
|
||||
'IMG_0001.jpg',
|
||||
expect(galleryService.fetchPhotoBlob).not.toHaveBeenCalled();
|
||||
expect(galleryService.triggerBrowserDownload).not.toHaveBeenCalled();
|
||||
expect(galleryService.triggerDirectDownload).toHaveBeenCalledTimes(1);
|
||||
expect(galleryService.triggerDirectDownload).toHaveBeenCalledWith(
|
||||
expect.stringMatching(/\/gallery\/slug\/download\/1$/),
|
||||
'fallback.jpg',
|
||||
);
|
||||
});
|
||||
|
||||
it('falls through to a regular download on desktop Safari (no share / canShare APIs)', async () => {
|
||||
it('navigates straight to the download URL on desktop Safari (no share / canShare APIs)', async () => {
|
||||
installNavigator({ userAgent: DESKTOP_UA });
|
||||
|
||||
await galleryService.savePhotoToDevice('slug', 1, 'fallback.jpg');
|
||||
|
||||
expect(galleryService.triggerBrowserDownload).toHaveBeenCalledTimes(1);
|
||||
expect(galleryService.fetchPhotoBlob).not.toHaveBeenCalled();
|
||||
expect(galleryService.triggerBrowserDownload).not.toHaveBeenCalled();
|
||||
expect(galleryService.triggerDirectDownload).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('detects iPadOS 13+ (reports as MacIntel + touch) as iOS', async () => {
|
||||
@@ -115,6 +123,7 @@ describe('galleryService.savePhotoToDevice — iOS gating (#554)', () => {
|
||||
|
||||
expect(share).toHaveBeenCalledTimes(1);
|
||||
expect(galleryService.triggerBrowserDownload).not.toHaveBeenCalled();
|
||||
expect(galleryService.triggerDirectDownload).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('does NOT treat a regular Mac (MacIntel + no touch) as iOS', async () => {
|
||||
@@ -131,7 +140,7 @@ describe('galleryService.savePhotoToDevice — iOS gating (#554)', () => {
|
||||
await galleryService.savePhotoToDevice('slug', 1, 'fallback.jpg');
|
||||
|
||||
expect(share).not.toHaveBeenCalled();
|
||||
expect(galleryService.triggerBrowserDownload).toHaveBeenCalledTimes(1);
|
||||
expect(galleryService.triggerDirectDownload).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('does not fall back to download when the user dismisses the iOS share sheet (AbortError)', async () => {
|
||||
|
||||
@@ -68,16 +68,27 @@ export const galleryService = {
|
||||
},
|
||||
|
||||
// Save single photo. iOS routes through the Web Share API so the
|
||||
// share sheet's "Save Image" action lands the file in Photos;
|
||||
// everywhere else (Android, desktop) uses a regular <a download>
|
||||
// because their share sheets don't expose a save-to-gallery
|
||||
// action — #531 originally extended this to "all mobile with
|
||||
// canShare", which surfaced a useless app-picker on Android (#554).
|
||||
// share sheet's "Save Image" action lands the file in Photos.
|
||||
// Everywhere else (Android, desktop) navigates a hidden anchor
|
||||
// straight at the download URL — the browser's native download UI
|
||||
// shows up immediately and its progress lives in the notification
|
||||
// shade. Buffering the blob through fetch first (the original
|
||||
// path) added ~5s of dead air on cellular before any visible
|
||||
// feedback, prompting users to re-click and produce duplicate
|
||||
// downloads (#554 follow-up). Direct navigation eliminates the
|
||||
// latency outright rather than masking it with a spinner.
|
||||
async savePhotoToDevice(slug: string, photoId: number, filename: string): Promise<void> {
|
||||
if (!isIOS()) {
|
||||
this.triggerDirectDownload(
|
||||
api.getUri({ url: `/gallery/${slug}/download/${photoId}` }),
|
||||
filename,
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
const fetched = await this.fetchPhotoBlob(slug, photoId);
|
||||
const resolvedFilename = fetched.serverFilename || filename;
|
||||
|
||||
if (isIOS()) {
|
||||
// canShare() returns false on browsers without Web Share file
|
||||
// support. Probe with a representative File so the negotiation
|
||||
// is accurate — `canShare({ files: [] })` returns true on some
|
||||
@@ -102,7 +113,6 @@ export const galleryService = {
|
||||
if ((err as DOMException)?.name === 'AbortError') return;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
this.triggerBrowserDownload(fetched.blob, resolvedFilename);
|
||||
},
|
||||
@@ -157,6 +167,21 @@ export const galleryService = {
|
||||
window.URL.revokeObjectURL(url);
|
||||
},
|
||||
|
||||
// Trigger a browser-native download by navigating a hidden anchor at
|
||||
// the URL directly. The browser fetches the response itself (showing
|
||||
// its own progress UI), so unlike triggerBrowserDownload the JS layer
|
||||
// never materialises the bytes. `filename` is a hint; the server's
|
||||
// Content-Disposition wins per spec, which is what carries the #493
|
||||
// original-camera-filename setting through to disk.
|
||||
triggerDirectDownload(href: string, filename: string): void {
|
||||
const link = document.createElement('a');
|
||||
link.href = href;
|
||||
link.setAttribute('download', filename);
|
||||
document.body.appendChild(link);
|
||||
link.click();
|
||||
link.remove();
|
||||
},
|
||||
|
||||
// Download single photo — kept as the canonical name for the existing
|
||||
// grid + lightbox-action callers that haven't been migrated to the
|
||||
// share-aware savePhotoToDevice path yet.
|
||||
|
||||
Reference in New Issue
Block a user