Pillify permalinks to rooms and users (#10388)

2023-03-16 15:01:09 +01:00 · 2023-03-16 15:01:09 +01:00 · a86a8e7f8e
commit a86a8e7f8e
parent d850c95099
5 changed files with 128 additions and 105 deletions
--- 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();

--- 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 = (
--- 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,19 +257,7 @@ 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");
@ -275,19 +265,10 @@ describe("<TextualBody />", () => {
        });

        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");
@ -295,23 +276,13 @@ describe("<TextualBody />", () => {
        });

        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",
--- 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"
--- 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,
        },
    };