From 2bde31dcffcc8a01b4dfbd4b3940fda6c4856448 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 3 Feb 2023 08:59:21 +0000 Subject: [PATCH] Switch to linkify-react for element Linkification as it handles React subtrees without exploding (#10060 * Switch to linkify-react instead of our faulty implementation Fixes a series of soft crashes where errors include "The node to be removed is not a child of this node." * Improve types * Fix types * Update snapshots * Add test * Fix test --- cypress/e2e/spaces/spaces.spec.ts | 29 ++++++- package.json | 1 + src/HtmlUtils.tsx | 14 +++- src/SlashCommands.tsx | 9 +- src/components/structures/MatrixChat.tsx | 2 +- src/components/structures/SpaceHierarchy.tsx | 35 +++++--- src/components/views/elements/Linkify.tsx | 39 --------- src/components/views/elements/RoomTopic.tsx | 15 ++-- src/components/views/messages/TextualBody.tsx | 2 +- .../views/rooms/LinkPreviewWidget.tsx | 19 +---- src/components/views/rooms/RoomInfoLine.tsx | 2 +- src/linkify-matrix.ts | 8 +- .../__snapshots__/RoomView-test.tsx.snap | 32 +++----- .../views/elements/Linkify-test.tsx | 82 ------------------- yarn.lock | 5 ++ 15 files changed, 101 insertions(+), 193 deletions(-) delete mode 100644 src/components/views/elements/Linkify.tsx delete mode 100644 test/components/views/elements/Linkify-test.tsx diff --git a/cypress/e2e/spaces/spaces.spec.ts b/cypress/e2e/spaces/spaces.spec.ts index acf914c60f..f89fa297d0 100644 --- a/cypress/e2e/spaces/spaces.spec.ts +++ b/cypress/e2e/spaces/spaces.spec.ts @@ -17,6 +17,7 @@ limitations under the License. /// import type { MatrixClient } from "matrix-js-sdk/src/client"; +import type { Preset } from "matrix-js-sdk/src/@types/partials"; import type { ICreateRoomOpts } from "matrix-js-sdk/src/@types/requests"; import { HomeserverInstance } from "../../plugins/utils/homeserver"; import Chainable = Cypress.Chainable; @@ -32,7 +33,7 @@ function openSpaceContextMenu(spaceName: string): Chainable { return cy.get(".mx_SpacePanel_contextMenu"); } -function spaceCreateOptions(spaceName: string): ICreateRoomOpts { +function spaceCreateOptions(spaceName: string, roomIds: string[] = []): ICreateRoomOpts { return { creation_content: { type: "m.space", @@ -44,6 +45,7 @@ function spaceCreateOptions(spaceName: string): ICreateRoomOpts { name: spaceName, }, }, + ...roomIds.map(spaceChildInitialState), ], }; } @@ -283,4 +285,29 @@ describe("Spaces", () => { cy.checkA11y(undefined, axeOptions); cy.get(".mx_SpacePanel").percySnapshotElement("Space panel expanded", { widths: [258] }); }); + + it("should not soft crash when joining a room from space hierarchy which has a link in its topic", () => { + cy.getBot(homeserver, { displayName: "BotBob" }).then({ timeout: 10000 }, async (bot) => { + const { room_id: roomId } = await bot.createRoom({ + preset: "public_chat" as Preset, + name: "Test Room", + topic: "This is a topic https://github.com/matrix-org/matrix-react-sdk/pull/10060 with a link", + }); + const { room_id: spaceId } = await bot.createRoom(spaceCreateOptions("Test Space", [roomId])); + await bot.invite(spaceId, user.userId); + }); + + cy.getSpacePanelButton("Test Space").should("exist"); + cy.wait(500); // without this we can end up clicking too quickly and it ends up having no effect + cy.viewSpaceByName("Test Space"); + cy.contains(".mx_AccessibleButton", "Accept").click(); + + cy.contains(".mx_SpaceHierarchy_roomTile.mx_AccessibleButton", "Test Room").within(() => { + cy.contains("Join").should("exist").realHover().click(); + cy.contains("View", { timeout: 5000 }).should("exist").click(); + }); + + // Assert we get shown the new room intro, and thus not the soft crash screen + cy.get(".mx_NewRoomIntro").should("exist"); + }); }); diff --git a/package.json b/package.json index 8b8c3209f5..fe618fc40b 100644 --- a/package.json +++ b/package.json @@ -86,6 +86,7 @@ "jszip": "^3.7.0", "katex": "^0.16.0", "linkify-element": "4.0.0-beta.4", + "linkify-react": "4.0.0-beta.4", "linkify-string": "4.0.0-beta.4", "linkifyjs": "4.0.0-beta.4", "lodash": "^4.17.20", diff --git a/src/HtmlUtils.tsx b/src/HtmlUtils.tsx index e7d19f0834..f2452327e5 100644 --- a/src/HtmlUtils.tsx +++ b/src/HtmlUtils.tsx @@ -17,16 +17,17 @@ See the License for the specific language governing permissions and limitations under the License. */ -import React, { ReactNode } from "react"; +import React, { ReactElement, ReactNode } from "react"; import sanitizeHtml from "sanitize-html"; import cheerio from "cheerio"; import classNames from "classnames"; import EMOJIBASE_REGEX from "emojibase-regex"; -import { split } from "lodash"; +import { merge, split } from "lodash"; import katex from "katex"; import { decode } from "html-entities"; import { IContent } from "matrix-js-sdk/src/models/event"; import { Optional } from "matrix-events-sdk"; +import _Linkify from "linkify-react"; import { _linkifyElement, @@ -682,6 +683,15 @@ export function topicToHtml( ); } +/* Wrapper around linkify-react merging in our default linkify options */ +export function Linkify({ as, options, children }: React.ComponentProps): ReactElement { + return ( + <_Linkify as={as} options={merge({}, linkifyMatrixOptions, options)}> + {children} + + ); +} + /** * Linkifies the given string. This is a wrapper around 'linkifyjs/string'. * diff --git a/src/SlashCommands.tsx b/src/SlashCommands.tsx index 9b23bd4138..3434090d8e 100644 --- a/src/SlashCommands.tsx +++ b/src/SlashCommands.tsx @@ -33,7 +33,7 @@ import dis from "./dispatcher/dispatcher"; import { _t, _td, ITranslatableError, newTranslatableError } from "./languageHandler"; import Modal from "./Modal"; import MultiInviter from "./utils/MultiInviter"; -import { linkifyElement, topicToHtml } from "./HtmlUtils"; +import { Linkify, topicToHtml } from "./HtmlUtils"; import QuestionDialog from "./components/views/dialogs/QuestionDialog"; import WidgetUtils from "./utils/WidgetUtils"; import { textToHtmlRainbow } from "./utils/colour"; @@ -501,14 +501,11 @@ export const Commands = [ ? ContentHelpers.parseTopicContent(content) : { text: _t("This room has no topic.") }; - const ref = (e): void => { - if (e) linkifyElement(e); - }; - const body = topicToHtml(topic.text, topic.html, ref, true); + const body = topicToHtml(topic.text, topic.html, undefined, true); Modal.createDialog(InfoDialog, { title: room.name, - description:
{body}
, + description: {body}, hasCloseButton: true, className: "markdown-body", }); diff --git a/src/components/structures/MatrixChat.tsx b/src/components/structures/MatrixChat.tsx index e517aaaf83..44424619a7 100644 --- a/src/components/structures/MatrixChat.tsx +++ b/src/components/structures/MatrixChat.tsx @@ -136,9 +136,9 @@ import { SdkContextClass, SDKContext } from "../../contexts/SDKContext"; import { viewUserDeviceSettings } from "../../actions/handlers/viewUserDeviceSettings"; import { cleanUpBroadcasts, VoiceBroadcastResumer } from "../../voice-broadcast"; import GenericToast from "../views/toasts/GenericToast"; -import { Linkify } from "../views/elements/Linkify"; import RovingSpotlightDialog, { Filter } from "../views/dialogs/spotlight/SpotlightDialog"; import { findDMForUser } from "../../utils/dm/findDMForUser"; +import { Linkify } from "../../HtmlUtils"; // legacy export export { default as Views } from "../../Views"; diff --git a/src/components/structures/SpaceHierarchy.tsx b/src/components/structures/SpaceHierarchy.tsx index 7d6888a197..60a80bc25f 100644 --- a/src/components/structures/SpaceHierarchy.tsx +++ b/src/components/structures/SpaceHierarchy.tsx @@ -51,7 +51,7 @@ import TextWithTooltip from "../views/elements/TextWithTooltip"; import { useStateToggle } from "../../hooks/useStateToggle"; import { getChildOrder } from "../../stores/spaces/SpaceStore"; import AccessibleTooltipButton from "../views/elements/AccessibleTooltipButton"; -import { linkifyElement, topicToHtml } from "../../HtmlUtils"; +import { Linkify, topicToHtml } from "../../HtmlUtils"; import { useDispatcher } from "../../hooks/useDispatcher"; import { Action } from "../../dispatcher/actions"; import { IState, RovingTabIndexProvider, useRovingTabIndex } from "../../accessibility/RovingTabIndex"; @@ -210,6 +210,25 @@ const Tile: React.FC = ({ topic = room.topic; } + let topicSection: ReactNode | undefined; + if (topic) { + topicSection = ( + + {" · "} + {topic} + + ); + } + let joinedSection: ReactElement | undefined; if (joinedRoom) { joinedSection =
{_t("Joined")}
; @@ -231,19 +250,9 @@ const Tile: React.FC = ({ {joinedSection} {suggestedSection} -
e && linkifyElement(e)} - onClick={(ev) => { - // prevent clicks on links from bubbling up to the room tile - if ((ev.target as HTMLElement).tagName === "A") { - ev.stopPropagation(); - } - }} - > +
{description} - {topic && " · "} - {topic} + {topicSection}
diff --git a/src/components/views/elements/Linkify.tsx b/src/components/views/elements/Linkify.tsx deleted file mode 100644 index 077ccd76d0..0000000000 --- a/src/components/views/elements/Linkify.tsx +++ /dev/null @@ -1,39 +0,0 @@ -/* -Copyright 2022 The Matrix.org Foundation C.I.C. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -import React, { useLayoutEffect, useRef } from "react"; - -import { linkifyElement } from "../../../HtmlUtils"; - -interface Props { - as?: string; - children: React.ReactNode; - onClick?: (ev: MouseEvent) => void; -} - -export function Linkify({ as = "div", children, onClick }: Props): JSX.Element { - const ref = useRef(); - - useLayoutEffect(() => { - linkifyElement(ref.current); - }, [children]); - - return React.createElement(as, { - children, - ref, - onClick, - }); -} diff --git a/src/components/views/elements/RoomTopic.tsx b/src/components/views/elements/RoomTopic.tsx index 7f2d068d35..e760d38218 100644 --- a/src/components/views/elements/RoomTopic.tsx +++ b/src/components/views/elements/RoomTopic.tsx @@ -29,9 +29,8 @@ import InfoDialog from "../dialogs/InfoDialog"; import { useDispatcher } from "../../../hooks/useDispatcher"; import MatrixClientContext from "../../../contexts/MatrixClientContext"; import AccessibleButton from "./AccessibleButton"; -import { Linkify } from "./Linkify"; import TooltipTarget from "./TooltipTarget"; -import { topicToHtml } from "../../../HtmlUtils"; +import { Linkify, topicToHtml } from "../../../HtmlUtils"; interface IProps extends React.HTMLProps { room?: Room; @@ -71,12 +70,14 @@ export default function RoomTopic({ room, ...props }: IProps): JSX.Element { description: (
{ - if ((ev.target as HTMLElement).tagName.toUpperCase() === "A") { - modal.close(); - } + options={{ + attributes: { + onClick() { + modal.close(); + }, + }, }} + as="p" > {body} diff --git a/src/components/views/messages/TextualBody.tsx b/src/components/views/messages/TextualBody.tsx index 1679c08f12..9f6bed732d 100644 --- a/src/components/views/messages/TextualBody.tsx +++ b/src/components/views/messages/TextualBody.tsx @@ -436,7 +436,7 @@ export default class TextualBody extends React.Component { private onBodyLinkClick = (e: MouseEvent): void => { let target = e.target as HTMLLinkElement; // links processed by linkifyjs have their own handler so don't handle those here - if (target.classList.contains(linkifyOpts.className)) return; + if (target.classList.contains(linkifyOpts.className as string)) return; if (target.nodeName !== "A") { // Jump to parent as the `` may contain children, e.g. an anchor wrapping an inline code section target = target.closest("a"); diff --git a/src/components/views/rooms/LinkPreviewWidget.tsx b/src/components/views/rooms/LinkPreviewWidget.tsx index 225434db00..23bd19ff4d 100644 --- a/src/components/views/rooms/LinkPreviewWidget.tsx +++ b/src/components/views/rooms/LinkPreviewWidget.tsx @@ -19,7 +19,7 @@ import { decode } from "html-entities"; import { MatrixEvent } from "matrix-js-sdk/src/models/event"; import { IPreviewUrlResponse } from "matrix-js-sdk/src/client"; -import { linkifyElement } from "../../../HtmlUtils"; +import { Linkify } from "../../../HtmlUtils"; import SettingsStore from "../../../settings/SettingsStore"; import Modal from "../../../Modal"; import * as ImageUtils from "../../../ImageUtils"; @@ -35,21 +35,8 @@ interface IProps { } export default class LinkPreviewWidget extends React.Component { - private readonly description = createRef(); private image = createRef(); - public componentDidMount(): void { - if (this.description.current) { - linkifyElement(this.description.current); - } - } - - public componentDidUpdate(): void { - if (this.description.current) { - linkifyElement(this.description.current); - } - } - private onImageClick = (ev): void => { const p = this.props.preview; if (ev.button != 0 || ev.metaKey) return; @@ -155,8 +142,8 @@ export default class LinkPreviewWidget extends React.Component { {" - " + p["og:site_name"]} )}
-
- {description} +
+ {description}
diff --git a/src/components/views/rooms/RoomInfoLine.tsx b/src/components/views/rooms/RoomInfoLine.tsx index 18b5b1c766..114a613dca 100644 --- a/src/components/views/rooms/RoomInfoLine.tsx +++ b/src/components/views/rooms/RoomInfoLine.tsx @@ -37,7 +37,7 @@ const RoomInfoLine: FC = ({ room }) => { const summary = useAsyncMemo(async (): Promise> | null> => { if (room.getMyMembership() !== "invite") return null; try { - return room.client.getRoomSummary(room.roomId); + return await room.client.getRoomSummary(room.roomId); } catch (e) { return null; } diff --git a/src/linkify-matrix.ts b/src/linkify-matrix.ts index f1a9b91c55..9ac9d8ed95 100644 --- a/src/linkify-matrix.ts +++ b/src/linkify-matrix.ts @@ -16,7 +16,7 @@ limitations under the License. */ import * as linkifyjs from "linkifyjs"; -import { registerCustomProtocol, registerPlugin } from "linkifyjs"; +import { Opts, registerCustomProtocol, registerPlugin } from "linkifyjs"; import linkifyElement from "linkify-element"; import linkifyString from "linkify-string"; import { RoomMember } from "matrix-js-sdk/src/models/room-member"; @@ -139,9 +139,9 @@ export const ELEMENT_URL_PATTERN = "(?:app|beta|staging|develop)\\.element\\.io/" + ")(#.*)"; -export const options = { - events: function (href: string, type: Type | string): Partial { - switch (type) { +export const options: Opts = { + events: function (href: string, type: string): Partial { + switch (type as Type) { case Type.URL: { // intercept local permalinks to users and show them like userids (in userinfo of current room) try { diff --git a/test/components/structures/__snapshots__/RoomView-test.tsx.snap b/test/components/structures/__snapshots__/RoomView-test.tsx.snap index 47318525d5..8944103413 100644 --- a/test/components/structures/__snapshots__/RoomView-test.tsx.snap +++ b/test/components/structures/__snapshots__/RoomView-test.tsx.snap @@ -62,11 +62,9 @@ exports[`RoomView for a local room in state CREATING should match the snapshot 1
-
- -
+
@@ -161,11 +159,9 @@ exports[`RoomView for a local room in state ERROR should match the snapshot 1`]
-
- -
+
@@ -356,11 +352,9 @@ exports[`RoomView for a local room in state NEW should match the snapshot 1`] =
-
- -
+
@@ -623,11 +617,9 @@ exports[`RoomView for a local room in state NEW that is encrypted should match t
-
- -
+
diff --git a/test/components/views/elements/Linkify-test.tsx b/test/components/views/elements/Linkify-test.tsx deleted file mode 100644 index 2fa841b1f1..0000000000 --- a/test/components/views/elements/Linkify-test.tsx +++ /dev/null @@ -1,82 +0,0 @@ -/* -Copyright 2021 The Matrix.org Foundation C.I.C. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - http://www.apache.org/licenses/LICENSE-2.0 -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -import { fireEvent, render } from "@testing-library/react"; -import React, { useState } from "react"; - -import { Linkify } from "../../../../src/components/views/elements/Linkify"; - -describe("Linkify", () => { - it("linkifies the context", () => { - const { container } = render(https://perdu.com); - expect(container.innerHTML).toBe( - '
", - ); - }); - - it("correctly linkifies a room alias", () => { - const { container } = render(#element-web:matrix.org); - expect(container.innerHTML).toBe( - "", - ); - }); - - it("changes the root tag name", () => { - const TAG_NAME = "p"; - - const { container } = render(Hello world!); - - expect(container.querySelectorAll("p")).toHaveLength(1); - }); - - it("relinkifies on update", () => { - function DummyTest() { - const [n, setN] = useState(0); - function onClick() { - setN(n + 1); - } - - // upon clicking the element, change the content, and expect - // linkify to update - return ( -
- {n % 2 === 0 ? "https://perdu.com" : "https://matrix.org"} -
- ); - } - - const { container } = render(); - - expect(container.innerHTML).toBe( - "", - ); - - fireEvent.click(container.querySelector("div")); - - expect(container.innerHTML).toBe( - "", - ); - }); -}); diff --git a/yarn.lock b/yarn.lock index f5f4930289..b548d9f913 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6258,6 +6258,11 @@ linkify-element@4.0.0-beta.4: resolved "https://registry.yarnpkg.com/linkify-element/-/linkify-element-4.0.0-beta.4.tgz#31bb5dff7430c4debc34030466bd8f3e297793a7" integrity sha512-dsu5qxk6MhQHxXUlPjul33JknQPx7Iv/N8zisH4JtV31qVk0qZg/5gn10Hr76GlMuixcdcxVvGHNfVcvbut13w== +linkify-react@4.0.0-beta.4: + version "4.0.0-beta.4" + resolved "https://registry.yarnpkg.com/linkify-react/-/linkify-react-4.0.0-beta.4.tgz#75311ade523a52d43054dd841d724d746d43f60d" + integrity sha512-o4vFe28vtk6i8a6tbtkLyusIyhLJSYoHC3gEpmJEVqi6Hy3aguVEenYmtaOjmAQehDrBYeHv9s4qcneZOf7SWQ== + linkify-string@4.0.0-beta.4: version "4.0.0-beta.4" resolved "https://registry.yarnpkg.com/linkify-string/-/linkify-string-4.0.0-beta.4.tgz#0982509bc6ce81c554bff8d7121057193b84ea32"