fix: video attachments not reproducing on iOS

app/containers/message/hooks/__tests__/useFile.test.ts

@@ -2,106 +2,47 @@ import { act, renderHook } from '@testing-library/react-native';
 
 import { useFile } from '../useFile';
 import { type IAttachment } from '../../../../definitions';
-import { getMessageById } from '../../../../lib/database/services/Message';
-import { getThreadMessageById } from '../../../../lib/database/services/ThreadMessage';
-
-jest.mock('../../../../lib/database/services/Message', () => ({
-	getMessageById: jest.fn()
-}));
-
-jest.mock('../../../../lib/database/services/ThreadMessage', () => ({
-	getThreadMessageById: jest.fn()
-}));
-
-const mockGetMessageById = getMessageById as jest.Mock;
-const mockGetThreadMessageById = getThreadMessageById as jest.Mock;
 
 const file = { title: 'original.png', title_link: '/original' } as IAttachment;
 
-// Flushes the async checkMessage effect (two awaited DB reads) inside act so any
-// setIsMessagePersisted update settles before assertions run.
-const flushEffect = () =>
-	act(async () => {
-		await Promise.resolve();
-		await Promise.resolve();
-	});
-
 describe('useFile', () => {
-	beforeEach(() => {
-		jest.clearAllMocks();
-		mockGetThreadMessageById.mockResolvedValue(undefined);
-		mockGetMessageById.mockResolvedValue(undefined);
-	});
-
-	it('returns the file prop and treats forwarded merges as a no-op while the thread message persists', async () => {
-		mockGetThreadMessageById.mockResolvedValue({ id: 'thread-message' });
-		const { result } = renderHook(() => useFile(file, 'msg-id'));
-
-		await flushEffect();
-		expect(mockGetThreadMessageById).toHaveBeenCalledWith('msg-id');
-		expect(mockGetMessageById).not.toHaveBeenCalled();
-		expect(result.current[0]).toBe(file);
-
-		act(() => result.current[1]({ title_link: '/forwarded' }));
-		expect(result.current[0]).toBe(file);
-	});
-
-	it('stays persisted when no thread message exists but the message is found', async () => {
-		mockGetThreadMessageById.mockResolvedValue(undefined);
-		mockGetMessageById.mockResolvedValue({ id: 'message' });
-		const { result } = renderHook(() => useFile(file, 'msg-id'));
-
-		await flushEffect();
-		expect(mockGetMessageById).toHaveBeenCalledWith('msg-id');
-		expect(result.current[0]).toBe(file);
+	it('returns the file prop untouched until something is merged', () => {
+		const { result } = renderHook(() => useFile(file));
 
-		act(() => result.current[1]({ title_link: '/forwarded' }));
 		expect(result.current[0]).toBe(file);
 	});
 
-	it('becomes not-persisted and returns the merged localFile when neither message is found', async () => {
-		const { result } = renderHook(() => useFile(file, 'msg-id'));
+	it('merges an override on top of the file prop', () => {
+		const { result } = renderHook(() => useFile(file));
 
-		await flushEffect();
-		expect(mockGetMessageById).toHaveBeenCalledWith('msg-id');
+		act(() => result.current[1]({ title_link: 'file:///local/original.png' }));
 
-		act(() => result.current[1]({ title_link: '/forwarded' }));
-		expect(result.current[0].title_link).toBe('/forwarded');
 		expect(result.current[0]).not.toBe(file);
+		expect(result.current[0].title_link).toBe('file:///local/original.png');
 		expect(result.current[0].title).toBe('original.png');
 	});
 
-	it('starts not-persisted when messageId is empty and merges forwarded files into localFile', async () => {
-		const { result } = renderHook(() => useFile(file, ''));
+	it('accumulates successive overrides', () => {
+		const { result } = renderHook(() => useFile(file));
 
-		await flushEffect();
-		expect(result.current[0]).toBe(file);
+		act(() => result.current[1]({ title_link: 'file:///local/original.png' }));
+		act(() => result.current[1]({ e2e: 'done' }));
 
-		act(() => result.current[1]({ title_link: '/forwarded' }));
-		expect(result.current[0]).not.toBe(file);
-		expect(result.current[0].title_link).toBe('/forwarded');
+		expect(result.current[0].title_link).toBe('file:///local/original.png');
+		expect(result.current[0].e2e).toBe('done');
 	});
 
-	// Documents the current contract: isMessagePersisted is seeded once and the effect only ever
-	// flips it to false, so it can't recover to true on a later messageId change. Not fixed: the
-	// only caller keys its list by that same id, so a real id change always remounts the hook fresh.
-	it('stays stuck not-persisted after rerendering with a real persisted messageId (documents current contract)', async () => {
-		mockGetThreadMessageById.mockResolvedValue(undefined);
-		mockGetMessageById.mockResolvedValue(undefined);
-		const { result, rerender } = renderHook(({ messageId }: { messageId: string }) => useFile(file, messageId), {
-			initialProps: { messageId: '' }
+	// The override is what the download resolved to locally, so it has to win over a later `file`
+	// prop that still carries the remote url — that stale prop is exactly the bug this hook fixes.
+	it('keeps overrides applied when the file prop changes', () => {
+		const { result, rerender } = renderHook(({ f }: { f: IAttachment }) => useFile(f), {
+			initialProps: { f: file }
 		});
 
-		await flushEffect();
-		expect(result.current[0]).toBe(file);
+		act(() => result.current[1]({ title_link: 'file:///local/original.png' }));
+		rerender({ f: { ...file, title: 'renamed.png' } as IAttachment });
 
-		mockGetMessageById.mockResolvedValue({ id: 'msg-id' });
-		rerender({ messageId: 'msg-id' });
-		await flushEffect();
-
-		expect(mockGetMessageById).toHaveBeenCalledWith('msg-id');
-		act(() => result.current[1]({ title_link: '/forwarded' }));
-		expect(result.current[0]).not.toBe(file);
-		expect(result.current[0].title_link).toBe('/forwarded');
+		expect(result.current[0].title).toBe('renamed.png');
+		expect(result.current[0].title_link).toBe('file:///local/original.png');
 	});
 });

app/containers/message/hooks/useFile.tsx

@@ -1,32 +1,26 @@
-import { useEffect, useState } from 'react';
+import { useState } from 'react';
 
 import { type IAttachment } from '../../../definitions';
-import { getMessageById } from '../../../lib/database/services/Message';
-import { getThreadMessageById } from '../../../lib/database/services/ThreadMessage';
 
-export const useFile = (file: IAttachment, messageId: string) => {
+/**
+ * Keeps local overrides for an attachment — currently the local uri a download resolved to —
+ * merged on top of the `file` prop.
+ *
+ * Overrides are applied unconditionally rather than only for non-persisted messages. Waiting on the
+ * database round trip is not reliable: `persistMessage` silently no-ops whenever it can't find a
+ * row for the message id, which happens for forwarded messages and for the Files/Mentions/Starred/
+ * Pinned lists, whose attachments are built from REST payloads that carry no message id. Those
+ * attachments were left pointing at the remote url even after the file was cached on disk, so
+ * opening them streamed instead of playing the local file.
+ */
+export const useFile = (file: IAttachment) => {
 	'use memo';
 
-	const [localFile, setLocalFile] = useState(file);
-	const [isMessagePersisted, setIsMessagePersisted] = useState(!!messageId);
-	useEffect(() => {
-		const checkMessage = async () => {
-			const threadMessage = await getThreadMessageById(messageId);
-			if (!threadMessage) {
-				const message = await getMessageById(messageId);
-				if (!message) {
-					setIsMessagePersisted(false);
-				}
-			}
-		};
-		checkMessage();
-	}, [messageId]);
+	const [overrides, setOverrides] = useState<Partial<IAttachment> | null>(null);
 
-	const manageForwardedFile = (f: Partial<IAttachment>) => {
-		if (isMessagePersisted) {
-			return;
-		}
-		setLocalFile(prev => ({ ...prev, ...f }));
+	const mergeFile = (f: Partial<IAttachment>) => {
+		setOverrides(prev => ({ ...prev, ...f }));
 	};
-	return [isMessagePersisted ? file : localFile, manageForwardedFile] as const;
+
+	return [overrides ? { ...file, ...overrides } : file, mergeFile] as const;
 };

app/containers/message/hooks/useMediaAutoDownload.tsx

@@ -82,7 +82,7 @@ export const useMediaAutoDownload = ({
 	const baseUrl = useBaseUrl();
 	const user = useMessageUser();
 	const [status, dispatchDownloadEvent] = useReducer(downloadStatusReducer, 'to-download');
-	const [currentFile, setCurrentFile] = useFile(file, id ?? '');
+	const [currentFile, setCurrentFile] = useFile(file);
 	const originalUrl = getOriginalURL(file);
 	const url = formatAttachmentUrl(
 		file.title_link || getFileProperty(currentFile, fileType, 'url'),

app/lib/methods/handleMediaDownload.ts

@@ -249,11 +249,17 @@ const persistMessage = async (messageId: string, uri: string, encryption: boolea
 			})
 		);
 	}
-	if (batch.length) {
-		await db.write(async () => {
-			await db.batch(batch);
-		});
+	if (!batch.length) {
+		// No row to point at the cached file. Expected for forwarded messages and for the
+		// Files/Mentions/Starred/Pinned lists, whose attachments come from REST payloads that carry no
+		// message id. useFile keeps the local uri in component state so playback still works, but the
+		// database keeps the remote url — worth knowing about rather than failing silently.
+		console.log(`[handleMediaDownload] no message found for id "${messageId}", cached uri not persisted`);
+		return;
 	}
+	await db.write(async () => {
+		await db.batch(batch);
+	});
 };
 
 export function downloadMediaFile({

app/lib/methods/helpers/formatAttachmentUrl.ts

@@ -10,6 +10,21 @@ function setParamInUrl({ url, token, userId }: { url: string; token: string; use
 	return urlObj.toString();
 }
 
+/**
+ * Percent-encodes a url without double-encoding one that already is. The server hands us attachment
+ * paths already encoded (`/file-upload/<id>/Screen%20Recording.mov`), and a plain `encodeURI` turns
+ * every `%` into `%25`, producing a path the server can't resolve. Decoding first makes the result
+ * the same whichever form we were given. `decodeURI` leaves escapes for reserved characters alone,
+ * so query string values survive the round trip; it throws on a malformed escape, hence the guard.
+ */
+export const encodeAttachmentUrl = (url: string): string => {
+	try {
+		return encodeURI(decodeURI(url));
+	} catch {
+		return url;
+	}
+};
+
 export const formatAttachmentUrl = (
 	attachmentUrl: string | undefined,
 	userId: string,
@@ -28,17 +43,17 @@ export const formatAttachmentUrl = (
 		}
 
 		if (attachmentUrl.includes('rc_token')) {
-			return encodeURI(attachmentUrl);
+			return encodeAttachmentUrl(attachmentUrl);
 		}
 
-		if (protectFiles) return setParamInUrl({ url: attachmentUrl, token, userId });
-		return attachmentUrl;
+		if (protectFiles) return encodeAttachmentUrl(setParamInUrl({ url: attachmentUrl, token, userId }));
+		return encodeAttachmentUrl(attachmentUrl);
 	}
 	let cdnPrefix = store?.getState().settings.CDN_PREFIX as string;
 	cdnPrefix = cdnPrefix?.trim();
 	if (cdnPrefix && cdnPrefix.startsWith('http')) {
 		server = cdnPrefix.replace(/\/+$/, '');
 	}
-	if (protectFiles) return setParamInUrl({ url: `${server}${attachmentUrl}`, token, userId });
-	return `${server}${attachmentUrl}`;
+	if (protectFiles) return encodeAttachmentUrl(setParamInUrl({ url: `${server}${attachmentUrl}`, token, userId }));
+	return encodeAttachmentUrl(`${server}${attachmentUrl}`);
 };

app/views/AttachmentView.tsx

@@ -59,9 +59,9 @@ const RenderContent = ({
 	}, [navigation]);
 
 	if (attachment.image_url) {
-		const url = formatAttachmentUrl(attachment.title_link || attachment.image_url, user.id, user.token, baseUrl);
-		const uri = encodeURI(url);
-		const isAnimated = attachment.image_type === 'image/gif' || /\.gif(\?|$)/i.test(url);
+		// formatAttachmentUrl already encodes the url; encoding again would turn `%20` into `%2520`.
+		const uri = formatAttachmentUrl(attachment.title_link || attachment.image_url, user.id, user.token, baseUrl);
+		const isAnimated = attachment.image_type === 'image/gif' || /\.gif(\?|$)/i.test(uri);
 		return (
 			<ImageViewer
 				uri={uri}
@@ -74,8 +74,7 @@ const RenderContent = ({
 		);
 	}
 	if (attachment.video_url) {
-		const url = formatAttachmentUrl(attachment.title_link || attachment.video_url, user.id, user.token, baseUrl);
-		const uri = encodeURI(url);
+		const uri = formatAttachmentUrl(attachment.title_link || attachment.video_url, user.id, user.token, baseUrl);
 		return (
 			<Video
 				source={{ uri }}