From a8fb5f35f8790f378ad65b8a953c3d1a22d7f0d9 Mon Sep 17 00:00:00 2001 From: Tol Wassman Date: Fri, 7 Nov 2025 02:49:56 +0000 Subject: [PATCH 1/4] recalculate mentions of forwarded messages In transformEvent(), parse event body back into an EditorModel, and pass this into attachMentions(), so that it actually recalculates mentions. --- .../views/dialogs/ForwardDialog.tsx | 29 ++++++++++++------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/src/components/views/dialogs/ForwardDialog.tsx b/src/components/views/dialogs/ForwardDialog.tsx index 32aab5d6e98..6b4f737fa03 100644 --- a/src/components/views/dialogs/ForwardDialog.tsx +++ b/src/components/views/dialogs/ForwardDialog.tsx @@ -58,6 +58,10 @@ import { getKeyBindingsManager } from "../../../KeyBindingsManager"; import { KeyBindingAction } from "../../../accessibility/KeyboardShortcuts"; import { OverflowTileView } from "../rooms/OverflowTileView"; import { attachMentions } from "../../../utils/messages"; +import { CommandPartCreator } from "../../../editor/parts"; +import SettingsStore from "../../../settings/SettingsStore"; +import { parseEvent } from "../../../editor/deserialize"; +import EditorModel from "../../../editor/model"; const AVATAR_SIZE = 30; @@ -184,13 +188,13 @@ const Entry: React.FC> = ({ room, type, content, matrixClient: * 1. Strip all relations. * 2. Convert location events into a static pin-drop location share, * and remove description from self-location shares. - * 3. Pass through attachMentions() to strip mentions (as no EditorModel is present to recalculate from). + * 3. Parse the event back into an EditorModel and recalculate mentions. * * @param event - The MatrixEvent to transform. - * @param userId - Current user MXID (passed through to attachMentions()). + * @param cli - The MatrixClient (used for recalculation of mentions). * @returns The transformed event type and content. */ -const transformEvent = (event: MatrixEvent, userId: string): { type: string; content: IContent } => { +const transformEvent = (event: MatrixEvent, cli: MatrixClient): { type: string; content: IContent } => { const { // eslint-disable-next-line @typescript-eslint/no-unused-vars "m.relates_to": _, // strip relations - in future we will attach a relation pointing at the original event @@ -225,12 +229,17 @@ const transformEvent = (event: MatrixEvent, userId: string): { type: string; con }; } - // Mentions can leak information about the context of the original message, - // so pass through attachMentions() to recalculate mentions. - // Currently, this strips all mentions (forces an empty m.mentions), - // as there is no EditorModel to parse pills from. - // Future improvements could actually recalculate mentions based on the message body. - attachMentions(userId, content, null, undefined); + // Mentions can leak information about the context of the original message, so: + // 1. Parse the event's message body back into an EditorModel, then + // 2. Pass through attachMentions() to recalculate mentions. + const room = cli.getRoom(event.getRoomId())!; + const partCreator = new CommandPartCreator(room, cli); + const parts = parseEvent(event, partCreator, { + shouldEscape: SettingsStore.getValue("MessageComposerInput.useMarkdown"), + }); + const model = new EditorModel(parts, partCreator); // Temporary EditorModel to pass through + const userId = cli.getSafeUserId(); + attachMentions(userId, content, model, undefined); return { type, content }; }; @@ -242,7 +251,7 @@ const ForwardDialog: React.FC = ({ matrixClient: cli, event, permalinkCr cli.getProfileInfo(userId).then((info) => setProfileInfo(info)); }, [cli, userId]); - const { type, content } = transformEvent(event, userId); + const { type, content } = transformEvent(event, cli); // For the message preview we fake the sender as ourselves const mockEvent = new MatrixEvent({ From 1a697f18afd11bff701172bf376c53a08658f66a Mon Sep 17 00:00:00 2001 From: Tol Wassman Date: Fri, 7 Nov 2025 04:57:18 +0000 Subject: [PATCH 2/4] refactor ForwardDialog-test.tsx Refactor test for stripping mentions on forwards to allow for more tests of mention recalculation --- .../views/dialogs/ForwardDialog-test.tsx | 56 ++++++++++--------- 1 file changed, 31 insertions(+), 25 deletions(-) diff --git a/test/unit-tests/components/views/dialogs/ForwardDialog-test.tsx b/test/unit-tests/components/views/dialogs/ForwardDialog-test.tsx index 370f612c9cf..3352b20682a 100644 --- a/test/unit-tests/components/views/dialogs/ForwardDialog-test.tsx +++ b/test/unit-tests/components/views/dialogs/ForwardDialog-test.tsx @@ -248,36 +248,42 @@ describe("ForwardDialog", () => { expect(firstButton.getAttribute("aria-disabled")).toBeTruthy(); expect(secondButton.getAttribute("aria-disabled")).toBeFalsy(); }); - - it("strips mentions from forwarded messages", async () => { - const messageWithMention = mkEvent({ - type: "m.room.message", - room: sourceRoom, - user: "@bob:example.org", - content: { - "msgtype": "m.text", - "body": "Hi @alice:example.org", - "m.mentions": { - user_ids: ["@alice:example.org"], - }, - }, - event: true, - }); - - const { container } = mountForwardDialog(messageWithMention); + + describe("Mention recalculation", () => { const roomId = "a"; - - // Click the send button. - act(() => { + const sendClick = (container: HTMLElement): void => act(() => { const sendButton = container.querySelector(".mx_ForwardList_sendButton"); fireEvent.click(sendButton!); }); - - // Expected content should have mentions empty. - expect(mockClient.sendEvent).toHaveBeenCalledWith(roomId, messageWithMention.getType(), { - ...messageWithMention.getContent(), - "m.mentions": {}, + const makeMessage = (body: string, mentions: Object, formattedBody?: string) => { + return mkEvent({ + type: "m.room.message", + room: sourceRoom, + user: "@bob:example.org", + content: { + "msgtype": "m.text", + "body": body, + "m.mentions": mentions, + ...(formattedBody && { + "format": "org.matrix.custom.html", + "formatted_body": formattedBody, + }), + }, + event: true, + }); + }; + + it("strips extra mentions", async () => { + const message = makeMessage("Hi Alice", {user_ids: [aliceId]}); + const { container } = mountForwardDialog(message); + sendClick(container); + // Expected content should have mentions empty. + expect(mockClient.sendEvent).toHaveBeenCalledWith(roomId, message.getType(), { + ...message.getContent(), + "m.mentions": {}, + }); }); + }); describe("Location events", () => { From 67bcd9c2606fcce2d2c82d7de3f08ec0b845dcc8 Mon Sep 17 00:00:00 2001 From: Tol Wassman Date: Fri, 7 Nov 2025 05:05:08 +0000 Subject: [PATCH 3/4] add test to recalculate mention pills Fails due to not mocking room membership --- .../views/dialogs/ForwardDialog-test.tsx | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/test/unit-tests/components/views/dialogs/ForwardDialog-test.tsx b/test/unit-tests/components/views/dialogs/ForwardDialog-test.tsx index 3352b20682a..c8e3f98fbf3 100644 --- a/test/unit-tests/components/views/dialogs/ForwardDialog-test.tsx +++ b/test/unit-tests/components/views/dialogs/ForwardDialog-test.tsx @@ -255,7 +255,7 @@ describe("ForwardDialog", () => { const sendButton = container.querySelector(".mx_ForwardList_sendButton"); fireEvent.click(sendButton!); }); - const makeMessage = (body: string, mentions: Object, formattedBody?: string) => { + const makeMessage = (body: string, mentions: object, formattedBody?: string) => { return mkEvent({ type: "m.room.message", room: sourceRoom, @@ -284,6 +284,18 @@ describe("ForwardDialog", () => { }); }); + // TODO: mock room membership + it("recalculates mention pills", async () => { + const message = makeMessage("Hi Alice", {user_ids: [aliceId]}, `Hi Alice`); + const { container } = mountForwardDialog(message); + sendClick(container); + // Expected content should have mentions empty. + expect(mockClient.sendEvent).toHaveBeenCalledWith(roomId, message.getType(), { + ...message.getContent(), + "m.mentions": {user_ids: [aliceId]}, + }); + }); + }); describe("Location events", () => { From 68b5787bf4ba819ed9b644a17fc3c9f648107a09 Mon Sep 17 00:00:00 2001 From: Tol Wassman Date: Fri, 7 Nov 2025 05:12:56 +0000 Subject: [PATCH 4/4] fix lint --- .../views/dialogs/ForwardDialog-test.tsx | 30 +++++++++++-------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/test/unit-tests/components/views/dialogs/ForwardDialog-test.tsx b/test/unit-tests/components/views/dialogs/ForwardDialog-test.tsx index c8e3f98fbf3..7011bc780cd 100644 --- a/test/unit-tests/components/views/dialogs/ForwardDialog-test.tsx +++ b/test/unit-tests/components/views/dialogs/ForwardDialog-test.tsx @@ -248,13 +248,14 @@ describe("ForwardDialog", () => { expect(firstButton.getAttribute("aria-disabled")).toBeTruthy(); expect(secondButton.getAttribute("aria-disabled")).toBeFalsy(); }); - + describe("Mention recalculation", () => { const roomId = "a"; - const sendClick = (container: HTMLElement): void => act(() => { - const sendButton = container.querySelector(".mx_ForwardList_sendButton"); - fireEvent.click(sendButton!); - }); + const sendClick = (container: HTMLElement): void => + act(() => { + const sendButton = container.querySelector(".mx_ForwardList_sendButton"); + fireEvent.click(sendButton!); + }); const makeMessage = (body: string, mentions: object, formattedBody?: string) => { return mkEvent({ type: "m.room.message", @@ -265,16 +266,16 @@ describe("ForwardDialog", () => { "body": body, "m.mentions": mentions, ...(formattedBody && { - "format": "org.matrix.custom.html", - "formatted_body": formattedBody, + format: "org.matrix.custom.html", + formatted_body: formattedBody, }), }, event: true, }); }; - + it("strips extra mentions", async () => { - const message = makeMessage("Hi Alice", {user_ids: [aliceId]}); + const message = makeMessage("Hi Alice", { user_ids: [aliceId] }); const { container } = mountForwardDialog(message); sendClick(container); // Expected content should have mentions empty. @@ -283,19 +284,22 @@ describe("ForwardDialog", () => { "m.mentions": {}, }); }); - + // TODO: mock room membership it("recalculates mention pills", async () => { - const message = makeMessage("Hi Alice", {user_ids: [aliceId]}, `Hi Alice`); + const message = makeMessage( + "Hi Alice", + { user_ids: [aliceId] }, + `Hi Alice`, + ); const { container } = mountForwardDialog(message); sendClick(container); // Expected content should have mentions empty. expect(mockClient.sendEvent).toHaveBeenCalledWith(roomId, message.getType(), { ...message.getContent(), - "m.mentions": {user_ids: [aliceId]}, + "m.mentions": { user_ids: [aliceId] }, }); }); - }); describe("Location events", () => {