diff --git a/frontend/src/services/__tests__/gallery.savePhotoToDevice.test.ts b/frontend/src/services/__tests__/gallery.savePhotoToDevice.test.ts new file mode 100644 index 00000000..ece86d3c --- /dev/null +++ b/frontend/src/services/__tests__/gallery.savePhotoToDevice.test.ts @@ -0,0 +1,162 @@ +/** + * Regression coverage for issue #554. + * + * `savePhotoToDevice` originally (PR #531) routed through navigator.share + * whenever `canShare({files})` returned true, on the assumption that any + * mobile share sheet would expose a "Save Image" action. That's only true + * on iOS — Android's share sheet only lists installed apps that handle + * image/* intents, so the user gets an app-picker instead of a save + * dialog. These tests pin the iOS-only gating: iOS goes through Web Share, + * everywhere else falls through to . + */ + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +const IOS_UA = 'Mozilla/5.0 (iPhone; CPU iPhone OS 17_2 like Mac OS X) AppleWebKit/605.1.15'; +const IPADOS_UA = 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15'; +const ANDROID_UA = 'Mozilla/5.0 (Linux; Android 14; Pixel 8) AppleWebKit/537.36 Chrome/120.0.0.0'; +const DESKTOP_UA = 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 Version/17.0 Safari/605.1.15'; + +// Stub the photo blob fetch + the trigger so the test +// only exercises the iOS-vs-other branching logic. fetchPhotoBlob is +// network; triggerBrowserDownload calls document.createElement + click +// which is side-effecty in jsdom (and not what we're testing here). +const fetchedBlob = { blob: new Blob(['x'], { type: 'image/jpeg' }), serverFilename: 'IMG_0001.jpg' }; + +let galleryService: typeof import('../gallery.service').galleryService; + +const installNavigator = (overrides: Partial<{ + userAgent: string; + platform: string; + maxTouchPoints: number; + share: ReturnType; + canShare: ReturnType; +}>) => { + const desc = (value: any) => ({ value, configurable: true, writable: true }); + Object.defineProperties(navigator, { + userAgent: desc(overrides.userAgent ?? ''), + platform: desc(overrides.platform ?? ''), + maxTouchPoints: desc(overrides.maxTouchPoints ?? 0), + }); + // share / canShare don't exist on jsdom's navigator by default, so + // they're plain assignments rather than defineProperty. + (navigator as any).share = overrides.share; + (navigator as any).canShare = overrides.canShare; +}; + +describe('galleryService.savePhotoToDevice — iOS gating (#554)', () => { + beforeEach(async () => { + vi.resetModules(); + galleryService = (await import('../gallery.service')).galleryService; + + vi.spyOn(galleryService, 'fetchPhotoBlob').mockResolvedValue(fetchedBlob as any); + vi.spyOn(galleryService, 'triggerBrowserDownload').mockImplementation(() => undefined); + }); + + afterEach(() => { + vi.restoreAllMocks(); + delete (navigator as any).share; + delete (navigator as any).canShare; + }); + + it('routes through navigator.share on iOS when canShare({files}) is true', async () => { + const share = vi.fn().mockResolvedValue(undefined); + const canShare = vi.fn().mockReturnValue(true); + installNavigator({ userAgent: IOS_UA, share, canShare }); + + await galleryService.savePhotoToDevice('slug', 1, 'fallback.jpg'); + + expect(share).toHaveBeenCalledTimes(1); + expect(canShare).toHaveBeenCalledWith({ files: expect.any(Array) }); + expect(galleryService.triggerBrowserDownload).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. + const share = vi.fn().mockResolvedValue(undefined); + const canShare = vi.fn().mockReturnValue(true); + installNavigator({ userAgent: ANDROID_UA, share, canShare }); + + 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', + ); + }); + + it('falls through to a regular download on desktop Safari (no share / canShare APIs)', async () => { + installNavigator({ userAgent: DESKTOP_UA }); + + await galleryService.savePhotoToDevice('slug', 1, 'fallback.jpg'); + + expect(galleryService.triggerBrowserDownload).toHaveBeenCalledTimes(1); + }); + + it('detects iPadOS 13+ (reports as MacIntel + touch) as iOS', async () => { + const share = vi.fn().mockResolvedValue(undefined); + const canShare = vi.fn().mockReturnValue(true); + installNavigator({ + userAgent: IPADOS_UA, + platform: 'MacIntel', + maxTouchPoints: 5, + share, + canShare, + }); + + await galleryService.savePhotoToDevice('slug', 1, 'fallback.jpg'); + + expect(share).toHaveBeenCalledTimes(1); + expect(galleryService.triggerBrowserDownload).not.toHaveBeenCalled(); + }); + + it('does NOT treat a regular Mac (MacIntel + no touch) as iOS', async () => { + const share = vi.fn().mockResolvedValue(undefined); + const canShare = vi.fn().mockReturnValue(true); + installNavigator({ + userAgent: DESKTOP_UA, + platform: 'MacIntel', + maxTouchPoints: 0, + share, + canShare, + }); + + await galleryService.savePhotoToDevice('slug', 1, 'fallback.jpg'); + + expect(share).not.toHaveBeenCalled(); + expect(galleryService.triggerBrowserDownload).toHaveBeenCalledTimes(1); + }); + + it('does not fall back to download when the user dismisses the iOS share sheet (AbortError)', async () => { + // AbortError signals a deliberate user dismissal; falling back to a + // download would surprise them with a file landing in Downloads + // anyway, defeating the dismissal. + const abortErr = Object.assign(new Error('user cancelled'), { name: 'AbortError' }); + const share = vi.fn().mockRejectedValue(abortErr); + const canShare = vi.fn().mockReturnValue(true); + installNavigator({ userAgent: IOS_UA, share, canShare }); + + await galleryService.savePhotoToDevice('slug', 1, 'fallback.jpg'); + + expect(share).toHaveBeenCalledTimes(1); + expect(galleryService.triggerBrowserDownload).not.toHaveBeenCalled(); + }); + + it('does fall back to download when navigator.share() rejects with a non-Abort error', async () => { + const otherErr = Object.assign(new Error('not allowed'), { name: 'NotAllowedError' }); + const share = vi.fn().mockRejectedValue(otherErr); + const canShare = vi.fn().mockReturnValue(true); + installNavigator({ userAgent: IOS_UA, share, canShare }); + + await galleryService.savePhotoToDevice('slug', 1, 'fallback.jpg'); + + expect(galleryService.triggerBrowserDownload).toHaveBeenCalledTimes(1); + }); +}); diff --git a/frontend/src/services/gallery.service.ts b/frontend/src/services/gallery.service.ts index 2f47de0e..3cd8fee2 100644 --- a/frontend/src/services/gallery.service.ts +++ b/frontend/src/services/gallery.service.ts @@ -3,6 +3,25 @@ import type { GalleryInfo, GalleryData, GalleryStats, ResolvedGalleryIdentifier import { normalizeRequirePassword } from '../utils/accessControl'; import { parseContentDispositionFilename } from '../utils/contentDisposition'; +// iOS is the only platform whose system share sheet exposes a +// first-party "Save Image" / "Save to Photos" action for files +// shared via navigator.share(). On Android the share sheet only +// lists installed apps that registered an image/* intent (WhatsApp, +// Telegram, etc.) — there is no built-in save-to-gallery action, +// so the share path produces a useless app-picker for users who +// just wanted to save the photo (#554). UA-sniff is the only signal +// available because feature detection (canShare) is true on both. +// +// The MacIntel + maxTouchPoints clause covers iPadOS 13+ which +// identifies as Mac in navigator.userAgent but supports the same +// share-to-Photos flow as iOS Safari. +function isIOS(): boolean { + if (typeof navigator === 'undefined') return false; + const ua = navigator.userAgent || ''; + if (/iPad|iPhone|iPod/.test(ua)) return true; + return navigator.platform === 'MacIntel' && (navigator.maxTouchPoints || 0) > 1; +} + export const galleryService = { // Verify share token async verifyToken(slug: string, token: string): Promise<{ valid: boolean }> { @@ -48,42 +67,40 @@ export const galleryService = { }; }, - // Save single photo via the Web Share API on mobile, falling back to a - // regular browser download elsewhere (#531). - // - // On iOS Safari 15+ and Chrome Android the OS share sheet opened by - // navigator.share() includes "Save Image" / "Save to Photos", which - // is what non-technical clients actually want — straight into the - // Photos / Gallery app instead of the Files folder. Desktop browsers - // and Firefox don't implement Web Share File support, so they get the - // existing path (file lands in Downloads, same as before). + // 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). async savePhotoToDevice(slug: string, photoId: number, filename: string): Promise { const fetched = await this.fetchPhotoBlob(slug, photoId); const resolvedFilename = fetched.serverFilename || filename; - // canShare() returns false on browsers without Web Share file support - // (desktop, older Safari, all Firefox as of writing). 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 (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] }); - 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; + } } }