From 7afd4f7adc6639b65cf16f0244f26c73c09f699a Mon Sep 17 00:00:00 2001 From: Michael Weimann Date: Wed, 15 Mar 2023 13:35:56 +0100 Subject: [PATCH 1/2] Pillify permalinks --- src/components/views/messages/TextualBody.tsx | 5 +--- src/utils/pillify.tsx | 24 ++++++++++++++++--- 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/src/components/views/messages/TextualBody.tsx b/src/components/views/messages/TextualBody.tsx index 5f00959ccb0..05f6da046db 100644 --- a/src/components/views/messages/TextualBody.tsx +++ b/src/components/views/messages/TextualBody.tsx @@ -92,11 +92,8 @@ export default class TextualBody extends React.Component { const showLineNumbers = SettingsStore.getValue("showCodeLineNumbers"); this.activateSpoilers([content]); - // pillifyLinks BEFORE linkifyElement because plain room/user URLs in the composer - // are still sent as plaintext URLs. If these are ever pillified in the composer, - // we should be pillify them here by doing the linkifying BEFORE the pillifying. - pillifyLinks([content], this.props.mxEvent, this.pills); HtmlUtils.linkifyElement(content); + pillifyLinks([content], this.props.mxEvent, this.pills); this.calculateUrlPreview(); diff --git a/src/utils/pillify.tsx b/src/utils/pillify.tsx index 7e3f998f6e7..4418e88e5a2 100644 --- a/src/utils/pillify.tsx +++ b/src/utils/pillify.tsx @@ -23,6 +23,25 @@ import { MatrixClientPeg } from "../MatrixClientPeg"; import SettingsStore from "../settings/SettingsStore"; import { Pill, PillType, pillRoomNotifLen, pillRoomNotifPos } from "../components/views/elements/Pill"; import { parsePermalink } from "./permalinks/Permalinks"; +import { PermalinkParts } from "./permalinks/PermalinkConstructor"; + +/** + * A node here is an A element with a href attribute tag. + * + * It should not be pillified if the permalink parser result contains an event Id. + * + * It should be pillified if the permalink parser returns a result and one of the following conditions match: + * - Text content equals href. This is the case when sending a plain permalink inside a message. + * - The link does not have the "linkified" class. + * Composer completions already create an A tag. + * Linkify will not linkify things again. → There won't be a "linkified" class. + */ +const shouldBePillified = (node: Element, href: string, parts: PermalinkParts | null): boolean => { + if (!parts || parts.eventId) return false; + + const textContent = node.textContent; + return href === textContent || !node.classList.contains("linkified"); +}; /** * Recurses depth-first through a DOM tree, converting matrix.to links @@ -51,9 +70,8 @@ export function pillifyLinks(nodes: ArrayLike, mxEvent: MatrixEvent, pi } else if (node.tagName === "A" && node.getAttribute("href")) { const href = node.getAttribute("href")!; const parts = parsePermalink(href); - // If the link is a (localised) matrix.to link, replace it with a pill - // We don't want to pill event permalinks, so those are ignored. - if (parts && !parts.eventId) { + + if (shouldBePillified(node, href, parts)) { const pillContainer = document.createElement("span"); const pill = ( From 7d8156330ddbe3d0c6fe806dc910303bbd41da38 Mon Sep 17 00:00:00 2001 From: Michael Weimann Date: Wed, 15 Mar 2023 15:13:23 +0100 Subject: [PATCH 2/2] Extend TextualBody-test --- .../views/messages/TextualBody-test.tsx | 166 +++++++----------- .../__snapshots__/TextualBody-test.tsx.snap | 31 ++++ test/test-utils/test-utils.ts | 7 + 3 files changed, 106 insertions(+), 98 deletions(-) diff --git a/test/components/views/messages/TextualBody-test.tsx b/test/components/views/messages/TextualBody-test.tsx index f5466c46cfe..194b32c4304 100644 --- a/test/components/views/messages/TextualBody-test.tsx +++ b/test/components/views/messages/TextualBody-test.tsx @@ -37,6 +37,17 @@ const mkRoomTextMessage = (body: string): MatrixEvent => { }); }; +const mkFormattedMessage = (body: string, formattedBody: string): MatrixEvent => { + return mkMessage({ + msg: body, + formattedMsg: formattedBody, + format: "org.matrix.custom.html", + room: "room_id", + user: "sender", + event: true, + }); +}; + describe("", () => { afterEach(() => { jest.spyOn(MatrixClientPeg, "get").mockRestore(); @@ -156,6 +167,15 @@ describe("", () => { ); }); + it("should pillify an MXID permalink", () => { + const ev = mkRoomTextMessage("Chat with https://matrix.to/#/@user:example.com"); + const { container } = getComponent({ mxEvent: ev }); + const content = container.querySelector(".mx_EventTile_body"); + expect(content.innerHTML).toMatchInlineSnapshot( + `"Chat with Member"`, + ); + }); + it("should not pillify room aliases", () => { const ev = mkRoomTextMessage("Visit #room:example.com"); const { container } = getComponent({ mxEvent: ev }); @@ -164,6 +184,15 @@ describe("", () => { `"Visit #room:example.com"`, ); }); + + it("should pillify a room alias permalink", () => { + const ev = mkRoomTextMessage("Visit https://matrix.to/#/#room:example.com"); + const { container } = getComponent({ mxEvent: ev }); + const content = container.querySelector(".mx_EventTile_body"); + expect(content.innerHTML).toMatchInlineSnapshot( + `"Visit #room:example.com"`, + ); + }); }); describe("renders formatted m.text correctly", () => { @@ -183,19 +212,10 @@ describe("", () => { }); it("italics, bold, underline and strikethrough render as expected", () => { - const ev = mkEvent({ - type: "m.room.message", - room: "room_id", - user: "sender", - content: { - body: "foo *baz* __bar__ del u", - msgtype: "m.text", - format: "org.matrix.custom.html", - formatted_body: "foo baz bar del u", - }, - event: true, - }); - + const ev = mkFormattedMessage( + "foo *baz* __bar__ del u", + "foo baz bar del u", + ); const { container } = getComponent({ mxEvent: ev }, matrixClient); expect(container).toHaveTextContent("foo baz bar del u"); const content = container.querySelector(".mx_EventTile_body"); @@ -207,19 +227,10 @@ describe("", () => { }); it("spoilers get injected properly into the DOM", () => { - const ev = mkEvent({ - type: "m.room.message", - room: "room_id", - user: "sender", - content: { - body: "Hey [Spoiler for movie](mxc://someserver/somefile)", - msgtype: "m.text", - format: "org.matrix.custom.html", - formatted_body: 'Hey the movie was awesome', - }, - event: true, - }); - + const ev = mkFormattedMessage( + "Hey [Spoiler for movie](mxc://someserver/somefile)", + 'Hey the movie was awesome', + ); const { container } = getComponent({ mxEvent: ev }, matrixClient); expect(container).toHaveTextContent("Hey (movie) the movie was awesome"); const content = container.querySelector(".mx_EventTile_body"); @@ -234,19 +245,10 @@ describe("", () => { }); it("linkification is not applied to code blocks", () => { - const ev = mkEvent({ - type: "m.room.message", - room: "room_id", - user: "sender", - content: { - body: "Visit `https://matrix.org/`\n```\nhttps://matrix.org/\n```", - msgtype: "m.text", - format: "org.matrix.custom.html", - formatted_body: "

Visit https://matrix.org/

\n
https://matrix.org/\n
\n", - }, - event: true, - }); - + const ev = mkFormattedMessage( + "Visit `https://matrix.org/`\n```\nhttps://matrix.org/\n```", + "

Visit https://matrix.org/

\n
https://matrix.org/\n
\n", + ); const { container } = getComponent({ mxEvent: ev }, matrixClient); expect(container).toHaveTextContent("Visit https://matrix.org/ 1https://matrix.org/"); const content = container.querySelector(".mx_EventTile_body"); @@ -255,19 +257,7 @@ describe("", () => { // If pills were rendered within a Portal/same shadow DOM then it'd be easier to test it("pills get injected correctly into the DOM", () => { - const ev = mkEvent({ - type: "m.room.message", - room: "room_id", - user: "sender", - content: { - body: "Hey User", - msgtype: "m.text", - format: "org.matrix.custom.html", - formatted_body: 'Hey Member', - }, - event: true, - }); - + const ev = mkFormattedMessage("Hey User", 'Hey Member'); const { container } = getComponent({ mxEvent: ev }, matrixClient); expect(container).toHaveTextContent("Hey Member"); const content = container.querySelector(".mx_EventTile_body"); @@ -275,19 +265,10 @@ describe("", () => { }); it("pills do not appear in code blocks", () => { - const ev = mkEvent({ - type: "m.room.message", - room: "room_id", - user: "sender", - content: { - body: "`@room`\n```\n@room\n```", - msgtype: "m.text", - format: "org.matrix.custom.html", - formatted_body: "

@room

\n
@room\n
\n", - }, - event: true, - }); - + const ev = mkFormattedMessage( + "`@room`\n```\n@room\n```", + "

@room

\n
@room\n
\n", + ); const { container } = getComponent({ mxEvent: ev }); expect(container).toHaveTextContent("@room 1@room"); const content = container.querySelector(".mx_EventTile_body"); @@ -295,23 +276,13 @@ describe("", () => { }); it("pills do not appear for event permalinks", () => { - const ev = mkEvent({ - type: "m.room.message", - room: "room_id", - user: "sender", - content: { - body: - "An [event link](https://matrix.to/#/!ZxbRYPQXDXKGmDnJNg:example.com/" + - "$16085560162aNpaH:example.com?via=example.com) with text", - msgtype: "m.text", - format: "org.matrix.custom.html", - formatted_body: - 'An event link with text', - }, - event: true, - }); + const ev = mkFormattedMessage( + "An [event link](https://matrix.to/#/!ZxbRYPQXDXKGmDnJNg:example.com/" + + "$16085560162aNpaH:example.com?via=example.com) with text", + 'An event link with text', + ); const { container } = getComponent({ mxEvent: ev }, matrixClient); expect(container).toHaveTextContent("An event link with text"); const content = container.querySelector(".mx_EventTile_body"); @@ -324,23 +295,12 @@ describe("", () => { }); it("pills appear for room links with vias", () => { - const ev = mkEvent({ - type: "m.room.message", - room: "room_id", - user: "sender", - content: { - body: - "A [room link](https://matrix.to/#/!ZxbRYPQXDXKGmDnJNg:example.com" + - "?via=example.com&via=bob.com) with vias", - msgtype: "m.text", - format: "org.matrix.custom.html", - formatted_body: - 'A room link with vias', - }, - event: true, - }); - + const ev = mkFormattedMessage( + "A [room link](https://matrix.to/#/!ZxbRYPQXDXKGmDnJNg:example.com" + + "?via=example.com&via=bob.com) with vias", + 'A room link with vias', + ); const { container } = getComponent({ mxEvent: ev }, matrixClient); expect(container).toHaveTextContent("A room name with vias"); const content = container.querySelector(".mx_EventTile_body"); @@ -356,6 +316,16 @@ describe("", () => { ); }); + it("pills appear for an MXID permalink", () => { + const ev = mkFormattedMessage( + "Chat with [@user:example.com](https://matrix.to/#/@user:example.com)", + 'Chat with @user:example.com', + ); + const { container } = getComponent({ mxEvent: ev }, matrixClient); + const content = container.querySelector(".mx_EventTile_body"); + expect(content).toMatchSnapshot(); + }); + it("renders formatted body without html corretly", () => { const ev = mkEvent({ type: "m.room.message", diff --git a/test/components/views/messages/__snapshots__/TextualBody-test.tsx.snap b/test/components/views/messages/__snapshots__/TextualBody-test.tsx.snap index 36c53927371..166749bd3ba 100644 --- a/test/components/views/messages/__snapshots__/TextualBody-test.tsx.snap +++ b/test/components/views/messages/__snapshots__/TextualBody-test.tsx.snap @@ -41,6 +41,37 @@ exports[` renders formatted m.text correctly linkification is not `; +exports[` renders formatted m.text correctly pills appear for an MXID permalink 1`] = ` + + Chat with + + + + + + Member + + + + + +`; + exports[` renders formatted m.text correctly pills do not appear in code blocks 1`] = `