Regression: Broken message jump onto threads

apps/meteor/app/ui-utils/client/lib/RoomHistoryManager.ts

@@ -272,7 +272,7 @@ class RoomHistoryManagerClass extends Emitter {
 	}
 
 	public async getSurroundingMessages(message?: Pick<IMessage, '_id' | 'rid'> & { ts?: Date }, atBottomRef?: MutableRefObject<boolean>) {
-		if (!message || !message.rid) {
+		if (!message?.rid) {
 			return;
 		}
 
@@ -295,13 +295,8 @@ class RoomHistoryManagerClass extends Emitter {
 				500,
 			);
 
-			msgElement.addClass('highlight');
 			setHighlightMessage(message._id);
 
-			setTimeout(() => {
-				msgElement.removeClass('highlight');
-			}, 500);
-
 			setTimeout(() => {
 				clearHighlightMessage();
 			}, 1000);
@@ -342,17 +337,12 @@ class RoomHistoryManagerClass extends Emitter {
 				500,
 			);
 
-			msgElement.addClass('highlight');
 			setHighlightMessage(message._id);
 
 			room.isLoading.set(false);
 			const messages = wrapper[0];
 			if (atBottomRef) atBottomRef.current = !result.moreAfter && messages.scrollTop >= messages.scrollHeight - messages.clientHeight;
 
-			setTimeout(() => {
-				msgElement.removeClass('highlight');
-			}, 500);
-
 			setTimeout(() => {
 				clearHighlightMessage();
 			}, 1000);

apps/meteor/app/ui-utils/client/lib/openRoom.tsx

@@ -36,13 +36,17 @@ export async function openRoom(type: RoomType, name: string, render = true) {
 				if (room._id !== name && type === 'd') {
 					// Redirect old url using username to rid
 					RoomManager.close(type + name);
-					return FlowRouter.go('direct', { rid: room._id }, FlowRouter.current().queryParams);
+					FlowRouter.go('direct', { rid: room._id }, FlowRouter.current().queryParams);
+					return;
 				}
 
 				RoomManager.open({ typeName: type + name, rid: room._id });
 
 				c.stop();
-				if (room._id === Session.get('openedRoom') && !FlowRouter.getQueryParam('msg')) {
+
+				const messageId = FlowRouter.getQueryParam('msg');
+
+				if (room._id === Session.get('openedRoom') && !messageId) {
 					return;
 				}
 
@@ -72,31 +76,27 @@ export async function openRoom(type: RoomType, name: string, render = true) {
 					await callWithErrorHandling('openRoom', room._id);
 				}
 
-				if (FlowRouter.getQueryParam('msg')) {
-					const messageId = FlowRouter.getQueryParam('msg');
+				if (messageId) {
 					const msg = { _id: messageId, rid: room._id };
 
 					const message = Messages.findOne({ _id: msg._id }) || (await callWithErrorHandling('getMessages', [msg._id]))[0];
 
 					if (message && (message.tmid || message.tcount)) {
-						return FlowRouter.setParams({ tab: 'thread', context: message.tmid || message._id });
+						FlowRouter.withReplaceState(() => {
+							FlowRouter.setParams({ tab: 'thread', context: message.tmid || message._id });
+						});
+						return;
 					}
 
 					RoomHistoryManager.getSurroundingMessages(msg);
-					FlowRouter.setQueryParams({
-						msg: null,
-					});
+					FlowRouter.setQueryParams({ msg: null });
 				}
 
 				return callbacks.run('enter-room', sub);
 			} catch (error) {
 				c.stop();
 
-				if (FlowRouter.getQueryParam('msg')) {
-					FlowRouter.setQueryParams({
-						msg: null,
-					});
-				}
+				FlowRouter.setQueryParams({ msg: null });
 
 				if (type === 'd') {
 					try {
@@ -107,6 +107,7 @@ export async function openRoom(type: RoomType, name: string, render = true) {
 						console.error(error);
 					}
 				}
+
 				appLayout.render(
 					<MainLayout>
 						<RoomNotFound />

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

@@ -1,4 +1,5 @@
 import type { IMessage } from '@rocket.chat/core-typings';
+import { isThreadMessage } from '@rocket.chat/core-typings';
 import { FlowRouter } from 'meteor/kadira:flow-router';
 
 import { ChatRoom } from '../../../app/models/client';
@@ -10,21 +11,22 @@ export const jumpToMessage = async (message: IMessage) => {
 		(Template.instance() as any)?.tabBar?.close();
 	}
 
-	if (message.tmid) {
-		return FlowRouter.go(
-			FlowRouter.getRouteName(),
+	if (isThreadMessage(message)) {
+		const { route, queryParams } = FlowRouter.current();
+		FlowRouter.go(
+			route?.name ?? '/',
 			{
 				tab: 'thread',
 				context: message.tmid,
 				rid: message.rid,
-				jump: message._id,
 				name: ChatRoom.findOne({ _id: message.rid })?.name ?? '',
 			},
 			{
-				...FlowRouter.current().queryParams,
-				jump: message._id,
+				...queryParams,
+				msg: message._id,
 			},
 		);
+		return;
 	}
 
 	if (Session.get('openedRoom') === message.rid) {

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

@@ -1,14 +1,14 @@
-export const waitForElement = async (
+export const waitForElement = async <TElement extends Element>(
 	selector: string,
-	{ parent = document.documentElement }: { parent?: HTMLElement } = {},
-): Promise<Element> => {
-	const element = parent.querySelector(selector);
-	return new Promise((resolve) => {
+	{ parent = document.documentElement, signal }: { parent?: Element; signal?: AbortSignal } = {},
+): Promise<TElement> => {
+	const element = parent.querySelector<TElement>(selector);
+	return new Promise((resolve, reject) => {
 		if (element) {
 			return resolve(element);
 		}
 		const observer = new MutationObserver((_, obs) => {
-			const element = parent.querySelector(selector);
+			const element = parent.querySelector<TElement>(selector);
 			if (element) {
 				obs.disconnect(); // stop observing
 				return resolve(element);
@@ -18,5 +18,10 @@ export const waitForElement = async (
 			childList: true,
 			subtree: true,
 		});
+
+		signal?.addEventListener('abort', () => {
+			observer.disconnect();
+			reject(new DOMException('Aborted', 'AbortError'));
+		});
 	});
 };

apps/meteor/client/views/room/contextualBar/Threads/components/ThreadChat.tsx

@@ -2,8 +2,7 @@ import type { IMessage, IThreadMainMessage } from '@rocket.chat/core-typings';
 import { isEditedMessage } from '@rocket.chat/core-typings';
 import { Box, CheckBox, Field } from '@rocket.chat/fuselage';
 import { useUniqueId } from '@rocket.chat/fuselage-hooks';
-import { useCurrentRoute, useMethod, useQueryStringParameter, useRoute, useTranslation, useUserPreference } from '@rocket.chat/ui-contexts';
-import type { VFC } from 'react';
+import { useMethod, useTranslation, useUserPreference } from '@rocket.chat/ui-contexts';
 import React, { useState, useEffect, useCallback } from 'react';
 
 import { callbacks } from '../../../../../../lib/callbacks';
@@ -21,7 +20,7 @@ type ThreadChatProps = {
 	mainMessage: IThreadMainMessage;
 };
 
-const ThreadChat: VFC<ThreadChatProps> = ({ mainMessage }) => {
+const ThreadChat = ({ mainMessage }: ThreadChatProps) => {
 	const [fileUploadTriggerProps, fileUploadOverlayProps] = useFileUploadDropTarget();
 
 	const sendToChannelPreference = useUserPreference<'always' | 'never' | 'default'>('alsoSendThreadToChannel');
@@ -86,20 +85,6 @@ const ThreadChat: VFC<ThreadChatProps> = ({ mainMessage }) => {
 		};
 	}, [mainMessage._id, readThreads, room._id]);
 
-	const jump = useQueryStringParameter('jump');
-
-	const [currentRouteName, currentRouteParams, currentRouteQueryStringParams] = useCurrentRoute();
-	if (!currentRouteName) {
-		throw new Error('No route name');
-	}
-	const currentRoute = useRoute(currentRouteName);
-
-	const handleJumpTo = useCallback(() => {
-		const newQueryStringParams = { ...currentRouteQueryStringParams };
-		delete newQueryStringParams.jump;
-		currentRoute.replace(currentRouteParams, newQueryStringParams);
-	}, [currentRoute, currentRouteParams, currentRouteQueryStringParams]);
-
 	const subscription = useRoomSubscription();
 	const sendToChannelID = useUniqueId();
 	const t = useTranslation();
@@ -109,7 +94,7 @@ const ThreadChat: VFC<ThreadChatProps> = ({ mainMessage }) => {
 			<DropTargetOverlay {...fileUploadOverlayProps} />
 			<Box is='section' display='flex' flexDirection='column' flexGrow={1} flexShrink={1} flexBasis='auto' height='full'>
 				<MessageListErrorBoundary>
-					<ThreadMessageList mainMessage={mainMessage} jumpTo={jump} onJumpTo={handleJumpTo} />
+					<ThreadMessageList mainMessage={mainMessage} />
 				</MessageListErrorBoundary>
 
 				<ComposerContainer

apps/meteor/client/views/room/contextualBar/Threads/components/ThreadMessageList.tsx

@@ -45,14 +45,12 @@ const isMessageSequential = (current: IMessage, previous: IMessage | undefined,
 
 type ThreadMessageListProps = {
 	mainMessage: IThreadMainMessage;
-	jumpTo?: string;
-	onJumpTo?: (mid: IMessage['_id']) => void;
 };
 
-const ThreadMessageList = ({ mainMessage, jumpTo, onJumpTo }: ThreadMessageListProps): ReactElement => {
+const ThreadMessageList = ({ mainMessage }: ThreadMessageListProps): ReactElement => {
 	const { messages, loading } = useLegacyThreadMessages(mainMessage._id);
 	const { listWrapperRef: listWrapperScrollRef, listRef: listScrollRef, onScroll: handleScroll } = useLegacyThreadMessageListScrolling();
-	const { parentRef: listJumpRef } = useLegacyThreadMessageJump(jumpTo, { enabled: !loading, onJumpTo });
+	const { parentRef: listJumpRef } = useLegacyThreadMessageJump({ enabled: !loading });
 
 	const listRef = useMergedRefs<HTMLElement | null>(listScrollRef, listJumpRef);
 	const hideUsernames = useUserPreference<boolean>('hideUsernames');

apps/meteor/client/views/room/contextualBar/Threads/hooks/useLegacyThreadMessageJump.ts

@@ -1,13 +1,27 @@
-import type { IMessage } from '@rocket.chat/core-typings';
+import { useCurrentRoute, useQueryStringParameter, useRoute } from '@rocket.chat/ui-contexts';
 import { useRef, useEffect } from 'react';
 
-export const useLegacyThreadMessageJump = (
-	mid: IMessage['_id'] | undefined,
-	{ enabled = true, onJumpTo }: { enabled?: boolean; onJumpTo?: (mid: IMessage['_id']) => void },
-) => {
+import { waitForElement } from '../../../../../lib/utils/waitForElement';
+import { clearHighlightMessage, setHighlightMessage } from '../../../MessageList/providers/messageHighlightSubscription';
+
+export const useLegacyThreadMessageJump = ({ enabled = true }: { enabled?: boolean }) => {
+	const mid = useQueryStringParameter('msg');
+
+	const [currentRouteName, currentRouteParams, currentRouteQueryStringParams] = useCurrentRoute();
+	if (!currentRouteName) {
+		throw new Error('No route name');
+	}
+	const currentRoute = useRoute(currentRouteName);
+
+	const clearQueryStringParameter = () => {
+		const newQueryStringParams = { ...currentRouteQueryStringParams };
+		delete newQueryStringParams.msg;
+		currentRoute.replace(currentRouteParams, newQueryStringParams);
+	};
+
 	const parentRef = useRef<HTMLElement>(null);
-	const onJumpToRef = useRef(onJumpTo);
-	onJumpToRef.current = onJumpTo;
+	const clearQueryStringParameterRef = useRef(clearQueryStringParameter);
+	clearQueryStringParameterRef.current = clearQueryStringParameter;
 
 	useEffect(() => {
 		const parent = parentRef.current;
@@ -16,25 +30,36 @@ export const useLegacyThreadMessageJump = (
 			return;
 		}
 
-		const messageElement = parent.querySelector(`[data-id="${mid}"]`);
-		if (!messageElement) {
-			return;
-		}
+		const abortController = new AbortController();
+
+		waitForElement<HTMLElement>(`[data-id='${mid}']`, { parent, signal: abortController.signal }).then((messageElement) => {
+			if (abortController.signal.aborted) {
+				return;
+			}
+
+			setHighlightMessage(mid);
+			clearQueryStringParameterRef.current?.();
+
+			setTimeout(() => {
+				clearHighlightMessage();
+			}, 1000);
+
+			setTimeout(() => {
+				if (abortController.signal.aborted) {
+					return;
+				}
 
-		messageElement.classList.add('highlight');
+				messageElement.scrollIntoView({
+					behavior: 'smooth',
+					block: 'nearest',
+				});
+			}, 300);
+		});
 
-		const removeClass = () => {
-			messageElement.classList.remove('highlight');
-			messageElement.removeEventListener('animationend', removeClass);
+		return () => {
+			abortController.abort();
 		};
-		messageElement.addEventListener('animationend', removeClass);
-
-		setTimeout(() => {
-			messageElement.scrollIntoView();
-			const onJumpTo = onJumpToRef.current;
-			onJumpTo?.(mid);
-		}, 300);
-	}, [enabled, mid, onJumpTo]);
+	}, [enabled, mid]);
 
 	return { parentRef };
 };