From 04795219a0b66fdd1ef73748d803adfdfc0d676f Mon Sep 17 00:00:00 2001 From: Paul Nothaft Date: Wed, 27 May 2026 13:16:36 +0200 Subject: [PATCH] fix(lightbox): eliminate download lag on Android by skipping the blob round-trip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `savePhotoToDevice` previously buffered the full image through JS as a Blob on every platform before clicking . 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. --- .../gallery.savePhotoToDevice.test.ts | 37 +++++---- frontend/src/services/gallery.service.ts | 81 ++++++++++++------- 2 files changed, 76 insertions(+), 42 deletions(-) diff --git a/frontend/src/services/__tests__/gallery.savePhotoToDevice.test.ts b/frontend/src/services/__tests__/gallery.savePhotoToDevice.test.ts index ece86d3c..a6e777c6 100644 --- a/frontend/src/services/__tests__/gallery.savePhotoToDevice.test.ts +++ b/frontend/src/services/__tests__/gallery.savePhotoToDevice.test.ts @@ -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 , + // 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 () => { diff --git a/frontend/src/services/gallery.service.ts b/frontend/src/services/gallery.service.ts index 3cd8fee2..ff393c9b 100644 --- a/frontend/src/services/gallery.service.ts +++ b/frontend/src/services/gallery.service.ts @@ -68,39 +68,49 @@ 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 - // 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 { + 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 - // browsers that don't actually accept files at share() time. - const file = new File([fetched.blob], resolvedFilename, { - type: fetched.blob.type || 'image/jpeg', - }); - const canShareFile = - typeof navigator !== 'undefined' && - typeof navigator.canShare === 'function' && - navigator.canShare({ files: [file] }); + // 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 + // browsers that don't actually accept files at share() time. + const file = new File([fetched.blob], resolvedFilename, { + type: fetched.blob.type || 'image/jpeg', + }); + const canShareFile = + typeof navigator !== 'undefined' && + typeof navigator.canShare === 'function' && + navigator.canShare({ files: [file] }); - if (canShareFile) { - try { - await navigator.share({ files: [file], title: resolvedFilename }); - return; - } catch (err) { - // AbortError = user dismissed the share sheet. Don't fall back — - // they made a choice. Any other failure (NotAllowedError, - // DataError, etc.) is unexpected; surface a download instead so - // the user still gets the file. - if ((err as DOMException)?.name === 'AbortError') return; - } + if (canShareFile) { + try { + await navigator.share({ files: [file], title: resolvedFilename }); + return; + } catch (err) { + // AbortError = user dismissed the share sheet. Don't fall back — + // they made a choice. Any other failure (NotAllowedError, + // DataError, etc.) is unexpected; surface a download instead so + // the user still gets the file. + if ((err as DOMException)?.name === 'AbortError') return; } } @@ -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.