fix(lightbox): restrict Web Share save-to-Photos path to iOS (#554)
PR #531 routed the single-photo download through navigator.share() whenever canShare({files}) returned true, on the assumption that any mobile share sheet would expose a "Save Image" action. That holds on iOS — Safari's share sheet has a first-party "Save to Photos" entry — but on Android the system share sheet only lists installed apps that registered an image/* intent (WhatsApp, Telegram, Drive, etc.). There is no built-in save-to-Gallery action, so Android users tapping the download button got an app-picker instead of the file saved to their device. Fix: gate the Web Share branch behind a UA-based isIOS() check. Android, desktop, and everything else fall through to the existing <a download> path (file lands in Downloads, visible in the Photos / Gallery app afterwards — same behaviour as before #531). iOS — including iPadOS 13+, which reports as MacIntel + touch — keeps the share-sheet flow that drops directly into Photos. UA-sniff is the only available signal here: canShare({files}) is true on both iOS Safari and Chrome Android, so feature detection cannot distinguish them. Tests pin all six scenarios — iOS share path, Android download fallback (even with canShare=true), desktop, iPadOS-as-Mac detected as iOS, regular Mac NOT detected as iOS, AbortError dismissal preserved (no surprise fallback), and non-Abort share() rejection falls back to download.
This commit is contained in:
@@ -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 <a download>.
|
||||||
|
*/
|
||||||
|
|
||||||
|
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 <a download> 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<typeof vi.fn>;
|
||||||
|
canShare: ReturnType<typeof vi.fn>;
|
||||||
|
}>) => {
|
||||||
|
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);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -3,6 +3,25 @@ import type { GalleryInfo, GalleryData, GalleryStats, ResolvedGalleryIdentifier
|
|||||||
import { normalizeRequirePassword } from '../utils/accessControl';
|
import { normalizeRequirePassword } from '../utils/accessControl';
|
||||||
import { parseContentDispositionFilename } from '../utils/contentDisposition';
|
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 = {
|
export const galleryService = {
|
||||||
// Verify share token
|
// Verify share token
|
||||||
async verifyToken(slug: string, token: string): Promise<{ valid: boolean }> {
|
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
|
// Save single photo. iOS routes through the Web Share API so the
|
||||||
// regular browser download elsewhere (#531).
|
// share sheet's "Save Image" action lands the file in Photos;
|
||||||
//
|
// everywhere else (Android, desktop) uses a regular <a download>
|
||||||
// On iOS Safari 15+ and Chrome Android the OS share sheet opened by
|
// because their share sheets don't expose a save-to-gallery
|
||||||
// navigator.share() includes "Save Image" / "Save to Photos", which
|
// action — #531 originally extended this to "all mobile with
|
||||||
// is what non-technical clients actually want — straight into the
|
// canShare", which surfaced a useless app-picker on Android (#554).
|
||||||
// Photos / Gallery app instead of the Files folder. Desktop browsers
|
|
||||||
// and Firefox don't implement Web Share File support, so they get the
|
|
||||||
// existing <a download> path (file lands in Downloads, same as before).
|
|
||||||
async savePhotoToDevice(slug: string, photoId: number, filename: string): Promise<void> {
|
async savePhotoToDevice(slug: string, photoId: number, filename: string): Promise<void> {
|
||||||
const fetched = await this.fetchPhotoBlob(slug, photoId);
|
const fetched = await this.fetchPhotoBlob(slug, photoId);
|
||||||
const resolvedFilename = fetched.serverFilename || filename;
|
const resolvedFilename = fetched.serverFilename || filename;
|
||||||
|
|
||||||
// canShare() returns false on browsers without Web Share file support
|
if (isIOS()) {
|
||||||
// (desktop, older Safari, all Firefox as of writing). Probe with a
|
// canShare() returns false on browsers without Web Share file
|
||||||
// representative File so the negotiation is accurate — `canShare({
|
// support. Probe with a representative File so the negotiation
|
||||||
// files: [] })` returns true on some browsers that don't actually
|
// is accurate — `canShare({ files: [] })` returns true on some
|
||||||
// accept files at share() time.
|
// browsers that don't actually accept files at share() time.
|
||||||
const file = new File([fetched.blob], resolvedFilename, {
|
const file = new File([fetched.blob], resolvedFilename, {
|
||||||
type: fetched.blob.type || 'image/jpeg',
|
type: fetched.blob.type || 'image/jpeg',
|
||||||
});
|
});
|
||||||
const canShareFile =
|
const canShareFile =
|
||||||
typeof navigator !== 'undefined' &&
|
typeof navigator !== 'undefined' &&
|
||||||
typeof navigator.canShare === 'function' &&
|
typeof navigator.canShare === 'function' &&
|
||||||
navigator.canShare({ files: [file] });
|
navigator.canShare({ files: [file] });
|
||||||
|
|
||||||
if (canShareFile) {
|
if (canShareFile) {
|
||||||
try {
|
try {
|
||||||
await navigator.share({ files: [file], title: resolvedFilename });
|
await navigator.share({ files: [file], title: resolvedFilename });
|
||||||
return;
|
return;
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
// AbortError = user dismissed the share sheet. Don't fall back —
|
// AbortError = user dismissed the share sheet. Don't fall back —
|
||||||
// they made a choice. Any other failure (NotAllowedError,
|
// they made a choice. Any other failure (NotAllowedError,
|
||||||
// DataError, etc.) is unexpected; surface a download instead so
|
// DataError, etc.) is unexpected; surface a download instead so
|
||||||
// the user still gets the file.
|
// the user still gets the file.
|
||||||
if ((err as DOMException)?.name === 'AbortError') return;
|
if ((err as DOMException)?.name === 'AbortError') return;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user