Pillify permalinks to rooms and users (#10388)

weeman1337 · toomore · commit b8b4a37f13d0 · 2023-04-06T15:03:35.000+08:00
diff --git 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<IBodyProps, IState> {
         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
@@ -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<Element>, 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 = (
diff --git 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("<TextualBody />", () => {
     afterEach(() => {
         jest.spyOn(MatrixClientPeg, "get").mockRestore();
@@ -156,6 +167,15 @@ describe("<TextualBody />", () => {
             );
         });
 
+        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 <span><bdi><a class="mx_Pill mx_UserPill mx_UserPill_me" href="https://matrix.to/#/@user:example.com"><img class="mx_BaseAvatar mx_BaseAvatar_image" src="mxc://avatar.url/image.png" style="width: 16px; height: 16px;" alt="" data-testid="avatar-img" aria-hidden="true"><span class="mx_Pill_linkText">Member</span></a></bdi></span>"`,
+            );
+        });
+
         it("should not pillify room aliases", () => {
             const ev = mkRoomTextMessage("Visit #room:example.com");
             const { container } = getComponent({ mxEvent: ev });
@@ -164,6 +184,15 @@ describe("<TextualBody />", () => {
                 `"Visit <a href="https://matrix.to/#/#room:example.com" class="linkified" rel="noreferrer noopener">#room:example.com</a>"`,
             );
         });
+
+        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 <span><bdi><a class="mx_Pill mx_RoomPill" href="https://matrix.to/#/#room:example.com"><span class="mx_Pill_linkText">#room:example.com</span></a></bdi></span>"`,
+            );
+        });
     });
 
     describe("renders formatted m.text correctly", () => {
@@ -183,19 +212,10 @@ describe("<TextualBody />", () => {
         });
 
         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>del</del> <u>u</u>",
-                    msgtype: "m.text",
-                    format: "org.matrix.custom.html",
-                    formatted_body: "foo <em>baz</em> <strong>bar</strong> <del>del</del> <u>u</u>",
-                },
-                event: true,
-            });
-
+            const ev = mkFormattedMessage(
+                "foo *baz* __bar__ <del>del</del> <u>u</u>",
+                "foo <em>baz</em> <strong>bar</strong> <del>del</del> <u>u</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("<TextualBody />", () => {
         });
 
         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 <span data-mx-spoiler="movie">the movie was awesome</span>',
-                },
-                event: true,
-            });
-
+            const ev = mkFormattedMessage(
+                "Hey [Spoiler for movie](mxc://someserver/somefile)",
+                'Hey <span data-mx-spoiler="movie">the movie was awesome</span>',
+            );
             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("<TextualBody />", () => {
         });
 
         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: "<p>Visit <code>https://matrix.org/</code></p>\n<pre>https://matrix.org/\n</pre>\n",
-                },
-                event: true,
-            });
-
+            const ev = mkFormattedMessage(
+                "Visit `https://matrix.org/`\n```\nhttps://matrix.org/\n```",
+                "<p>Visit <code>https://matrix.org/</code></p>\n<pre>https://matrix.org/\n</pre>\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,63 +257,32 @@ describe("<TextualBody />", () => {
 
         // 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 <a href="https://matrix.to/#/@user:server">Member</a>',
-                },
-                event: true,
-            });
-
+            const ev = mkFormattedMessage("Hey User", 'Hey <a href="https://matrix.to/#/@user:server">Member</a>');
             const { container } = getComponent({ mxEvent: ev }, matrixClient);
             expect(container).toHaveTextContent("Hey Member");
             const content = container.querySelector(".mx_EventTile_body");
             expect(content).toMatchSnapshot();
         });
 
         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: "<p><code>@room</code></p>\n<pre><code>@room\n</code></pre>\n",
-                },
-                event: true,
-            });
-
+            const ev = mkFormattedMessage(
+                "`@room`\n```\n@room\n```",
+                "<p><code>@room</code></p>\n<pre><code>@room\n</code></pre>\n",
+            );
             const { container } = getComponent({ mxEvent: ev });
             expect(container).toHaveTextContent("@room 1@room");
             const content = container.querySelector(".mx_EventTile_body");
             expect(content).toMatchSnapshot();
         });
 
         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 <a href="https://matrix.to/#/!ZxbRYPQXDXKGmDnJNg:example.com/' +
-                        '$16085560162aNpaH:example.com?via=example.com">event link</a> 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 <a href="https://matrix.to/#/!ZxbRYPQXDXKGmDnJNg:example.com/' +
+                    '$16085560162aNpaH:example.com?via=example.com">event link</a> 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("<TextualBody />", () => {
         });
 
         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 <a href="https://matrix.to/#/!ZxbRYPQXDXKGmDnJNg:example.com' +
-                        '?via=example.com&amp;via=bob.com">room link</a> 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 <a href="https://matrix.to/#/!ZxbRYPQXDXKGmDnJNg:example.com' +
+                    '?via=example.com&amp;via=bob.com">room link</a> 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("<TextualBody />", () => {
             );
         });
 
+        it("pills appear for an MXID permalink", () => {
+            const ev = mkFormattedMessage(
+                "Chat with [@user:example.com](https://matrix.to/#/@user:example.com)",
+                'Chat with <a href="https://matrix.to/#/@user:example.com">@user:example.com</a>',
+            );
+            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
@@ -41,6 +41,37 @@ exports[`<TextualBody /> renders formatted m.text correctly linkification is not
 </span>
 `;
 
+exports[`<TextualBody /> renders formatted m.text correctly pills appear for an MXID permalink 1`] = `
+<span
+  class="mx_EventTile_body markdown-body"
+  dir="auto"
+>
+  Chat with 
+  <span>
+    <bdi>
+      <a
+        class="mx_Pill mx_UserPill"
+        href="https://matrix.to/#/@user:example.com"
+      >
+        <img
+          alt=""
+          aria-hidden="true"
+          class="mx_BaseAvatar mx_BaseAvatar_image"
+          data-testid="avatar-img"
+          src="mxc://avatar.url/image.png"
+          style="width: 16px; height: 16px;"
+        />
+        <span
+          class="mx_Pill_linkText"
+        >
+          Member
+        </span>
+      </a>
+    </bdi>
+  </span>
+</span>
+`;
+
 exports[`<TextualBody /> renders formatted m.text correctly pills do not appear in code blocks 1`] = `
 <span
   class="mx_EventTile_body markdown-body"
diff --git a/test/test-utils/test-utils.ts b/test/test-utils/test-utils.ts
@@ -473,15 +473,21 @@ export type MessageEventProps = MakeEventPassThruProps & {
  * @param {number} opts.ts The timestamp for the event.
  * @param {boolean} opts.event True to make a MatrixEvent.
  * @param {string=} opts.msg Optional. The content.body for the event.
+ * @param {string=} opts.format Optional. The content.format for the event.
+ * @param {string=} opts.formattedMsg Optional. The content.formatted_body for the event.
  * @return {Object|MatrixEvent} The event
  */
 export function mkMessage({
     msg,
+    format,
+    formattedMsg,
     relatesTo,
     ...opts
 }: MakeEventPassThruProps & {
     room: Room["roomId"];
     msg?: string;
+    format?: string;
+    formattedMsg?: string;
 }): MatrixEvent {
     if (!opts.room || !opts.user) {
         throw new Error("Missing .room or .user from options");
@@ -493,6 +499,7 @@ export function mkMessage({
         content: {
             msgtype: "m.text",
             body: message,
+            ...(format && formattedMsg ? { format, formatted_body: formattedMsg } : {}),
             ["m.relates_to"]: relatesTo,
         },
     };
