fix: Stop audio playback when the playing message is deleted

.changeset/stop-audio-on-message-delete.md

@@ -0,0 +1,5 @@
+---
+'@rocket.chat/meteor': patch
+---
+
+Stops the shared audio player and hides the Now Playing card when the message that owns the playing audio attachment is deleted

.changeset/stop-audio-on-quoted-message-delete.md

@@ -0,0 +1,6 @@
+---
+'@rocket.chat/core-typings': patch
+'@rocket.chat/meteor': patch
+---
+
+Stops the shared audio player and hides the Now Playing card when the audio is no longer the listener's to hear: when the original message of a quoted audio attachment is deleted, when the listener leaves or is removed from the room the audio belongs to, and — for quotes created from now on — when the original is deleted in another room it was quoted from. Pinning a message while its audio plays no longer stops playback on a later bulk delete that excludes pinned messages, for as long as that message stays rendered

apps/meteor/client/components/message/content/attachments/QuoteAttachment.tsx

@@ -16,6 +16,7 @@ import AttachmentDetails from './structure/AttachmentDetails';
 import AttachmentInner from './structure/AttachmentInner';
 import AttachmentMessageLink from './structure/AttachmentMessageLink';
 import { toPlainTextRoot } from '../../../../lib/toPlainTextRoot';
+import { getMessageIdFromPermalink } from '../../../../lib/utils/getMessageIdFromPermalink';
 
 // TODO: remove this team collaboration
 const quoteStyles = css`
@@ -44,6 +45,7 @@ export type QuoteAttachmentProps = {
 export const QuoteAttachment = ({ attachment, source, path }: QuoteAttachmentProps) => {
 	const formatTime = useTimeAgo();
 	const displayAvatarPreference = useUserPreference<boolean>('displayAvatars');
+	const originMid = getMessageIdFromPermalink(attachment.message_link);
 
 	return (
 		<>
@@ -73,7 +75,7 @@ export const QuoteAttachment = ({ attachment, source, path }: QuoteAttachmentPro
 							<Attachments
 								attachments={attachment.attachments}
 								id={attachment.attachments[0]?.title_link}
-								source={source && { rid: source.rid, mid: source.mid, name: attachment.author_name }}
+								source={source && { ...source, originMid, originTs: attachment.ts, originRid: attachment.rid }}
 								keyPrefix={path}
 							/>
 						</AttachmentInner>

apps/meteor/client/components/message/content/attachments/file/AudioAttachment.tsx

@@ -1,7 +1,7 @@
 import type { AudioAttachmentProps } from '@rocket.chat/core-typings';
 import { AudioPlayerControls, Box } from '@rocket.chat/fuselage';
 import { useMediaUrl } from '@rocket.chat/ui-contexts';
-import { useMemo, useState } from 'react';
+import { useEffect, useMemo, useState } from 'react';
 
 import { useMediaPlayer } from '../../../../../providers/MediaPlayerProvider';
 import type { PersistentAudioTrack } from '../../../../../providers/MediaPlayerProvider';
@@ -15,6 +15,17 @@ export type AudioAttachmentSource = {
 	mid?: string;
 	username?: string;
 	name?: string;
+	ts?: Date;
+	/** Discussion room id the owning message links to. Immutable once set, so it is safe to snapshot. */
+	drid?: string;
+	/** Whether the owning message is pinned. Mutable, so the player is refreshed while this is rendered. */
+	pinned?: boolean;
+	/** When the audio is rendered inside a quote, the id of the original message that holds the attachment. */
+	originMid?: string;
+	/** Timestamp of the original quoted message. */
+	originTs?: Date;
+	/** Room of the original quoted message, which may differ from the room the quote is rendered in. */
+	originRid?: string;
 };
 
 type AudioAttachmentComponentProps = AudioAttachmentProps & {
@@ -36,7 +47,7 @@ const AudioAttachment = ({
 	const getURL = useMediaUrl();
 	const src = useMemo(() => getURL(url), [getURL, url]);
 
-	const { play, toggle, seek, cyclePlaybackRate, isActive, playing, currentTime, duration, playbackRate } = useMediaPlayer();
+	const { play, toggle, seek, cyclePlaybackRate, isActive, updateTrack, playing, currentTime, duration, playbackRate } = useMediaPlayer();
 
 	const track = useMemo<PersistentAudioTrack>(
 		() => ({
@@ -49,11 +60,41 @@ const AudioAttachment = ({
 			mid: source?.mid,
 			username: source?.username,
 			name: source?.name,
+			ts: source?.ts,
+			drid: source?.drid,
+			pinned: source?.pinned,
+			originMid: source?.originMid,
+			originTs: source?.originTs,
+			originRid: source?.originRid,
 		}),
-		[source?.mid, source?.rid, source?.username, source?.name, url, src, type, title, size],
+		[
+			source?.mid,
+			source?.rid,
+			source?.username,
+			source?.name,
+			source?.ts,
+			source?.drid,
+			source?.pinned,
+			source?.originMid,
+			source?.originTs,
+			source?.originRid,
+			url,
+			src,
+			type,
+			title,
+			size,
+		],
 	);
 
 	const active = isActive(track.id);
+
+	// The shared player keeps the descriptor it was handed, so hand it a fresh one whenever this
+	// message re-renders with different mutable state while it owns playback.
+	useEffect(() => {
+		if (active) {
+			updateTrack(track);
+		}
+	}, [active, track, updateTrack]);
 	const [previewDuration, setPreviewDuration] = useState(0);
 
 	return (

apps/meteor/client/components/message/variants/room/RoomMessageContent.tsx

@@ -60,7 +60,15 @@ const RoomMessageContent = ({ message, unread, all, mention, searchText }: RoomM
 			{!!quotes?.length && (
 				<Attachments
 					attachments={quotes}
-					source={{ rid: message.rid, mid: message._id, username: message.u.username, name: message.u.name }}
+					source={{
+						rid: message.rid,
+						mid: message._id,
+						username: message.u.username,
+						name: message.u.name,
+						ts: message.ts,
+						pinned: message.pinned,
+						drid: message.drid,
+					}}
 				/>
 			)}
 
@@ -83,7 +91,15 @@ const RoomMessageContent = ({ message, unread, all, mention, searchText }: RoomM
 				<Attachments
 					id={message.files?.[0]?._id}
 					attachments={attachments}
-					source={{ rid: message.rid, mid: message._id, username: message.u.username, name: message.u.name }}
+					source={{
+						rid: message.rid,
+						mid: message._id,
+						username: message.u.username,
+						name: message.u.name,
+						ts: message.ts,
+						pinned: message.pinned,
+						drid: message.drid,
+					}}
 				/>
 			)}
 

apps/meteor/client/components/message/variants/thread/ThreadMessageContent.tsx

@@ -54,7 +54,15 @@ const ThreadMessageContent = ({ message }: ThreadMessageContentProps) => {
 			{!!quotes?.length && (
 				<Attachments
 					attachments={quotes}
-					source={{ rid: message.rid, mid: message._id, username: message.u.username, name: message.u.name }}
+					source={{
+						rid: message.rid,
+						mid: message._id,
+						username: message.u.username,
+						name: message.u.name,
+						ts: message.ts,
+						pinned: message.pinned,
+						drid: message.drid,
+					}}
 				/>
 			)}
 
@@ -79,7 +87,15 @@ const ThreadMessageContent = ({ message }: ThreadMessageContentProps) => {
 				<Attachments
 					id={message.files?.[0]?._id}
 					attachments={attachments}
-					source={{ rid: message.rid, mid: message._id, username: message.u.username, name: message.u.name }}
+					source={{
+						rid: message.rid,
+						mid: message._id,
+						username: message.u.username,
+						name: message.u.name,
+						ts: message.ts,
+						pinned: message.pinned,
+						drid: message.drid,
+					}}
 				/>
 			)}
 

apps/meteor/client/lib/utils/getMessageIdFromPermalink.spec.ts

@@ -0,0 +1,26 @@
+import { getMessageIdFromPermalink } from './getMessageIdFromPermalink';
+
+describe('getMessageIdFromPermalink', () => {
+	it('returns the msg query parameter from an absolute permalink', () => {
+		expect(getMessageIdFromPermalink('https://open.rocket.chat/channel/general?msg=abc123')).toBe('abc123');
+	});
+
+	it('returns the msg query parameter when other parameters are present', () => {
+		expect(getMessageIdFromPermalink('https://open.rocket.chat/group/team?tab=thread&msg=xyz789&foo=bar')).toBe('xyz789');
+	});
+
+	it('returns the msg query parameter from a relative permalink', () => {
+		expect(getMessageIdFromPermalink('/direct/abc?msg=def456')).toBe('def456');
+	});
+
+	it('returns undefined when there is no msg query parameter', () => {
+		expect(getMessageIdFromPermalink('https://open.rocket.chat/channel/general')).toBeUndefined();
+		expect(getMessageIdFromPermalink('https://open.rocket.chat/channel/general?msg=')).toBeUndefined();
+	});
+
+	it('returns undefined for empty or unparsable input', () => {
+		expect(getMessageIdFromPermalink(undefined)).toBeUndefined();
+		expect(getMessageIdFromPermalink('')).toBeUndefined();
+		expect(getMessageIdFromPermalink('http://[invalid')).toBeUndefined();
+	});
+});

apps/meteor/client/lib/utils/getMessageIdFromPermalink.ts

@@ -0,0 +1,17 @@
+/**
+ * Extracts the message id from a message permalink such as
+ * `https://open.rocket.chat/channel/general?msg=abc123`.
+ * Returns `undefined` when the link has no `msg` query parameter or cannot be parsed.
+ */
+export const getMessageIdFromPermalink = (permalink: string | undefined): string | undefined => {
+	if (!permalink) {
+		return undefined;
+	}
+
+	try {
+		const msgId = new URL(permalink, 'http://localhost').searchParams.get('msg');
+		return msgId || undefined;
+	} catch {
+		return undefined;
+	}
+};

apps/meteor/client/providers/MediaPlayerProvider/MediaPlayerContext.ts

@@ -25,6 +25,34 @@ export type PersistentAudioTrack = {
 	username?: string;
 	/** Display name of the sender. */
 	name?: string;
+	/**
+	 * Timestamp of the message the audio belongs to (used to match bulk-delete criteria).
+	 * Only immutable message state is kept here: a track is replaced on the shared element
+	 * only when its id changes, so anything that can change mid-playback would go stale.
+	 */
+	ts?: Date;
+	/**
+	 * Discussion room id the owning message links to (used to match bulk-delete criteria).
+	 * A message is created with its `drid` and never gains or loses one, so this snapshot
+	 * stays accurate for as long as the track lives.
+	 */
+	drid?: string;
+	/**
+	 * Whether the owning message is pinned (used to match bulk-delete criteria). Unlike `drid`
+	 * this can change while the track is active, so the provider refreshes it from the rendered
+	 * message; it can still drift while that message is unmounted.
+	 */
+	pinned?: boolean;
+	/** When played from a quote, the id of the original message that holds the attachment (its deletion also closes the player). */
+	originMid?: string;
+	/** Timestamp of the original quoted message (used to match bulk-delete criteria). */
+	originTs?: Date;
+	/**
+	 * Room of the original quoted message. A quote may point at another room, in which case the
+	 * player also watches that room for deletions. Absent on quotes stored before the origin room
+	 * was persisted, which fall back to assuming the quoting room.
+	 */
+	originRid?: string;
 };
 
 export type MediaPlayerContextValue = {
@@ -43,6 +71,12 @@ export type MediaPlayerContextValue = {
 	cyclePlaybackRate: () => void;
 	/** Stops playback and clears the active track. */
 	close: () => void;
+	/**
+	 * Refreshes the mutable metadata of the active track when `next` describes it. Ignored when no
+	 * track is active or `next` is a different one, so a re-rendering message can keep the player's
+	 * copy of its own state current without disturbing playback.
+	 */
+	updateTrack: (next: PersistentAudioTrack) => void;
 	/** Whether the given track id is the one currently owned by the shared element. */
 	isActive: (id: string) => boolean;
 };
@@ -60,6 +94,7 @@ export const MediaPlayerContext = createContext<MediaPlayerContextValue>({
 	seek: noop,
 	cyclePlaybackRate: noop,
 	close: noop,
+	updateTrack: noop,
 	isActive: () => false,
 });
 

apps/meteor/client/providers/MediaPlayerProvider/MediaPlayerProvider.spec.tsx

@@ -0,0 +1,103 @@
+import { mockAppRoot } from '@rocket.chat/mock-providers';
+import { act, renderHook } from '@testing-library/react';
+import type { ReactNode } from 'react';
+
+import type { PersistentAudioTrack } from './MediaPlayerContext';
+import { useMediaPlayer } from './MediaPlayerContext';
+import MediaPlayerProvider from './MediaPlayerProvider';
+
+const buildTrack = (overrides: Partial<PersistentAudioTrack> = {}): PersistentAudioTrack => ({
+	id: 'mid1:url',
+	url: 'https://example.com/audio.mp3',
+	title: 'audio.mp3',
+	rid: 'room1',
+	mid: 'mid1',
+	username: 'john.doe',
+	ts: new Date('2024-01-01T00:00:00.000Z'),
+	pinned: false,
+	...overrides,
+});
+
+const wrapper = ({ children }: { children: ReactNode }) => {
+	const AppRoot = mockAppRoot().build();
+	return (
+		<AppRoot>
+			<MediaPlayerProvider>{children}</MediaPlayerProvider>
+		</AppRoot>
+	);
+};
+
+// jsdom does not implement media playback.
+beforeAll(() => {
+	Object.defineProperty(HTMLMediaElement.prototype, 'play', { configurable: true, value: jest.fn().mockResolvedValue(undefined) });
+	Object.defineProperty(HTMLMediaElement.prototype, 'load', { configurable: true, value: jest.fn() });
+});
+
+describe('MediaPlayerProvider updateTrack', () => {
+	it('adopts new mutable state for the active track', () => {
+		const { result } = renderHook(() => useMediaPlayer(), { wrapper });
+
+		act(() => result.current.play(buildTrack({ pinned: false })));
+
+		expect(result.current.track?.pinned).toBe(false);
+
+		act(() => result.current.updateTrack(buildTrack({ pinned: true })));
+
+		expect(result.current.track?.pinned).toBe(true);
+	});
+
+	// A message is not expected to gain a discussion id after it exists, so this is defensive:
+	// should the descriptors ever disagree, the player takes the one the message currently renders
+	// rather than keeping a value it can no longer justify.
+	it('refreshes the discussion id when the supplied descriptor differs', () => {
+		const { result } = renderHook(() => useMediaPlayer(), { wrapper });
+
+		act(() => result.current.play(buildTrack()));
+
+		expect(result.current.track?.drid).toBeUndefined();
+
+		act(() => result.current.updateTrack(buildTrack({ drid: 'disc1' })));
+
+		expect(result.current.track?.drid).toBe('disc1');
+	});
+
+	it('ignores an update describing a different track', () => {
+		const { result } = renderHook(() => useMediaPlayer(), { wrapper });
+
+		act(() => result.current.play(buildTrack({ pinned: false })));
+		act(() => result.current.updateTrack(buildTrack({ id: 'mid2:url', mid: 'mid2', pinned: true })));
+
+		expect(result.current.track?.id).toBe('mid1:url');
+		expect(result.current.track?.pinned).toBe(false);
+	});
+
+	it('ignores an update when no track is active', () => {
+		const { result } = renderHook(() => useMediaPlayer(), { wrapper });
+
+		act(() => result.current.updateTrack(buildTrack({ pinned: true })));
+
+		expect(result.current.track).toBeNull();
+	});
+
+	it('keeps the same track object when nothing mutable changed, so the player does not re-render', () => {
+		const { result } = renderHook(() => useMediaPlayer(), { wrapper });
+
+		act(() => result.current.play(buildTrack()));
+
+		const before = result.current.track;
+
+		act(() => result.current.updateTrack(buildTrack()));
+
+		expect(result.current.track).toBe(before);
+	});
+
+	it('does not disturb the url or identity of the active track', () => {
+		const { result } = renderHook(() => useMediaPlayer(), { wrapper });
+
+		act(() => result.current.play(buildTrack()));
+		act(() => result.current.updateTrack(buildTrack({ pinned: true })));
+
+		expect(result.current.track?.url).toBe('https://example.com/audio.mp3');
+		expect(result.current.track?.mid).toBe('mid1');
+	});
+});