From abd39c61b170c18b9b734dd0b5469ed5a17848af Mon Sep 17 00:00:00 2001 From: Johannes Marbach Date: Tue, 7 Jun 2022 22:20:32 +0200 Subject: [PATCH] Add support for MD / HTML in room topics (#8215) * Add support for MD / HTML in room topics Setting MD / HTML supported: - /topic command - Room settings overlay - Space settings overlay Display of MD / HTML supported: - /topic command - Room header - Space home Based on extensible events as defined in [MSC1767] Fixes: vector-im/element-web#5180 Signed-off-by: Johannes Marbach [MSC1767]: matrix-org/matrix-spec-proposals#1767 * Fix build error * Add comment to explain origin of styles Co-authored-by: Travis Ralston * Empty commit to retrigger build * Fix import grouping * Fix useTopic test * Add tests for HtmlUtils * Add slash command test * Add further serialize test * Fix ternary formatting Co-authored-by: Travis Ralston * Add blank line Co-authored-by: Travis Ralston * Properly mock SettingsStore access * Remove trailing space * Assert on HTML content and add test for plain text in HTML parameter * Appease the linter * Fix JSDoc comment * Fix toEqual call formatting * Repurpose test for literal HTML case * Empty commit to fix CI Co-authored-by: Travis Ralston Co-authored-by: Travis Ralston --- res/css/_common.scss | 67 +++++++++++++++++++ res/css/views/rooms/_RoomHeader.scss | 6 ++ src/HtmlUtils.tsx | 63 +++++++++++++++++ src/SlashCommands.tsx | 27 +++++--- src/components/views/elements/RoomTopic.tsx | 7 +- .../room_settings/RoomProfileSettings.tsx | 4 +- .../views/spaces/SpaceSettingsGeneralTab.tsx | 6 +- src/editor/serialize.ts | 6 +- src/hooks/room/useTopic.ts | 7 +- src/i18n/strings/en_EN.json | 1 + src/settings/Settings.tsx | 7 ++ test/HtmlUtils-test.tsx | 66 ++++++++++++++++++ test/SlashCommands-test.tsx | 40 +++++++++++ test/editor/serialize-test.ts | 7 ++ test/test-utils/test-utils.ts | 1 + test/useTopic-test.tsx | 2 +- 16 files changed, 298 insertions(+), 19 deletions(-) create mode 100644 test/HtmlUtils-test.tsx create mode 100644 test/SlashCommands-test.tsx diff --git a/res/css/_common.scss b/res/css/_common.scss index 8f315c432f..867a04bf6c 100644 --- a/res/css/_common.scss +++ b/res/css/_common.scss @@ -304,6 +304,73 @@ legend { overflow-y: auto; } +// Styles copied/inspired by GroupLayout, ReplyTile, and EventTile variants. +.mx_Dialog .markdown-body { + font-family: inherit !important; + white-space: normal !important; + line-height: inherit !important; + color: inherit; // inherit the colour from the dark or light theme by default (but not for code blocks) + font-size: $font-14px; + + pre, + code { + font-family: $monospace-font-family !important; + background-color: $codeblock-background-color; + } + + // this selector wrongly applies to code blocks too but we will unset it in the next one + code { + white-space: pre-wrap; // don't collapse spaces in inline code blocks + } + + pre code { + white-space: pre; // we want code blocks to be scrollable and not wrap + + >* { + display: inline; + } + } + + pre { + // have to use overlay rather than auto otherwise Linux and Windows + // Chrome gets very confused about vertical spacing: + // https://github.com/vector-im/vector-web/issues/754 + overflow-x: overlay; + overflow-y: visible; + + &::-webkit-scrollbar-corner { + background: transparent; + } + } +} + +.mx_Dialog .markdown-body h1, +.mx_Dialog .markdown-body h2, +.mx_Dialog .markdown-body h3, +.mx_Dialog .markdown-body h4, +.mx_Dialog .markdown-body h5, +.mx_Dialog .markdown-body h6 { + font-family: inherit !important; + color: inherit; +} + +/* Make h1 and h2 the same size as h3. */ +.mx_Dialog .markdown-body h1, +.mx_Dialog .markdown-body h2 { + font-size: 1.5em; + border-bottom: none !important; // override GFM +} + +.mx_Dialog .markdown-body a { + color: $accent-alt; +} + +.mx_Dialog .markdown-body blockquote { + border-left: 2px solid $blockquote-bar-color; + border-radius: 2px; + padding: 0 10px; +} + .mx_Dialog_fixedWidth { width: 60vw; max-width: 704px; diff --git a/res/css/views/rooms/_RoomHeader.scss b/res/css/views/rooms/_RoomHeader.scss index 7736ea1cc8..d3e5077e44 100644 --- a/res/css/views/rooms/_RoomHeader.scss +++ b/res/css/views/rooms/_RoomHeader.scss @@ -166,6 +166,12 @@ limitations under the License. display: -webkit-box; } +.mx_RoomHeader_topic .mx_Emoji { + // Undo font size increase to prevent vertical cropping and ensure the same size + // as in plain text emojis + font-size: inherit; +} + .mx_RoomHeader_avatar { flex: 0; margin: 0 6px 0 7px; diff --git a/src/HtmlUtils.tsx b/src/HtmlUtils.tsx index 6b9724caab..630bda8787 100644 --- a/src/HtmlUtils.tsx +++ b/src/HtmlUtils.tsx @@ -323,6 +323,18 @@ const composerSanitizeHtmlParams: IExtendedSanitizeOptions = { }, }; +// reduced set of allowed tags to avoid turning topics into Myspace +const topicSanitizeHtmlParams: IExtendedSanitizeOptions = { + ...sanitizeHtmlParams, + allowedTags: [ + 'font', // custom to matrix for IRC-style font coloring + 'del', // for markdown + 'a', 'sup', 'sub', + 'b', 'i', 'u', 'strong', 'em', 'strike', 'br', 'div', + 'span', + ], +}; + abstract class BaseHighlighter { constructor(public highlightClass: string, public highlightLink: string) { } @@ -606,6 +618,57 @@ export function bodyToHtml(content: IContent, highlights: string[], opts: IOpts ; } +/** + * Turn a room topic into html + * @param topic plain text topic + * @param htmlTopic optional html topic + * @param ref React ref to attach to any React components returned + * @param allowExtendedHtml whether to allow extended HTML tags such as headings and lists + * @return The HTML-ified node. + */ +export function topicToHtml( + topic: string, + htmlTopic?: string, + ref?: React.Ref, + allowExtendedHtml = false, +): ReactNode { + if (!SettingsStore.getValue("feature_html_topic")) { + htmlTopic = null; + } + + let isFormattedTopic = !!htmlTopic; + let topicHasEmoji = false; + let safeTopic = ""; + + try { + topicHasEmoji = mightContainEmoji(isFormattedTopic ? htmlTopic : topic); + + if (isFormattedTopic) { + safeTopic = sanitizeHtml(htmlTopic, allowExtendedHtml ? sanitizeHtmlParams : topicSanitizeHtmlParams); + if (topicHasEmoji) { + safeTopic = formatEmojis(safeTopic, true).join(''); + } + } + } catch { + isFormattedTopic = false; // Fall back to plain-text topic + } + + let emojiBodyElements: ReturnType; + if (!isFormattedTopic && topicHasEmoji) { + emojiBodyElements = formatEmojis(topic, false); + } + + return isFormattedTopic ? + : + { emojiBodyElements || topic } + ; +} + /** * Linkifies the given string. This is a wrapper around 'linkifyjs/string'. * diff --git a/src/SlashCommands.tsx b/src/SlashCommands.tsx index 75c9e078e4..8bbc3acd78 100644 --- a/src/SlashCommands.tsx +++ b/src/SlashCommands.tsx @@ -25,6 +25,7 @@ import * as ContentHelpers from 'matrix-js-sdk/src/content-helpers'; import { Element as ChildElement, parseFragment as parseHtml } from "parse5"; import { logger } from "matrix-js-sdk/src/logger"; import { IContent } from 'matrix-js-sdk/src/models/event'; +import { MRoomTopicEventContent } from 'matrix-js-sdk/src/@types/topic'; import { SlashCommand as SlashCommandEvent } from "@matrix-org/analytics-events/types/typescript/SlashCommand"; import { MatrixClientPeg } from './MatrixClientPeg'; @@ -32,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 { linkifyAndSanitizeHtml } from './HtmlUtils'; +import { linkifyElement, topicToHtml } from './HtmlUtils'; import QuestionDialog from "./components/views/dialogs/QuestionDialog"; import WidgetUtils from "./utils/WidgetUtils"; import { textToHtmlRainbow } from "./utils/colour"; @@ -66,6 +67,7 @@ import { XOR } from "./@types/common"; import { PosthogAnalytics } from "./PosthogAnalytics"; import { ViewRoomPayload } from "./dispatcher/payloads/ViewRoomPayload"; import VoipUserMapper from './VoipUserMapper'; +import { htmlSerializeFromMdIfNeeded } from './editor/serialize'; import { leaveRoomBehaviour } from "./utils/leave-behaviour"; // XXX: workaround for https://github.com/microsoft/TypeScript/issues/31816 @@ -463,7 +465,8 @@ export const Commands = [ runFn: function(roomId, args) { const cli = MatrixClientPeg.get(); if (args) { - return success(cli.setRoomTopic(roomId, args)); + const html = htmlSerializeFromMdIfNeeded(args, { forceHTML: false }); + return success(cli.setRoomTopic(roomId, args, html)); } const room = cli.getRoom(roomId); if (!room) { @@ -472,14 +475,19 @@ export const Commands = [ ); } - const topicEvents = room.currentState.getStateEvents('m.room.topic', ''); - const topic = topicEvents && topicEvents.getContent().topic; - const topicHtml = topic ? linkifyAndSanitizeHtml(topic) : _t('This room has no topic.'); + const content: MRoomTopicEventContent = room.currentState.getStateEvents('m.room.topic', '')?.getContent(); + const topic = !!content + ? ContentHelpers.parseTopicContent(content) + : { text: _t('This room has no topic.') }; + + const ref = e => e && linkifyElement(e); + const body = topicToHtml(topic.text, topic.html, ref, true); Modal.createTrackedDialog('Slash Commands', 'Topic', InfoDialog, { title: room.name, - description:
, + description:
{ body }
, hasCloseButton: true, + className: "markdown-body", }); return success(); }, @@ -1333,11 +1341,10 @@ interface ICmd { } /** - * Process the given text for /commands and return a bound method to perform them. + * Process the given text for /commands and returns a parsed command that can be used for running the operation. * @param {string} input The raw text input by the user. - * @return {null|function(): Object} Function returning an object with the property 'error' if there was an error - * processing the command, or 'promise' if a request was sent out. - * Returns null if the input didn't match a command. + * @return {ICmd} The parsed command object. + * Returns an empty object if the input didn't match a command. */ export function getCommand(input: string): ICmd { const { cmd, args } = parseCommandString(input); diff --git a/src/components/views/elements/RoomTopic.tsx b/src/components/views/elements/RoomTopic.tsx index 5cbfb2af27..00160816c2 100644 --- a/src/components/views/elements/RoomTopic.tsx +++ b/src/components/views/elements/RoomTopic.tsx @@ -31,6 +31,7 @@ import MatrixClientContext from "../../../contexts/MatrixClientContext"; import AccessibleButton from "./AccessibleButton"; import { Linkify } from "./Linkify"; import TooltipTarget from "./TooltipTarget"; +import { topicToHtml } from "../../../HtmlUtils"; interface IProps extends React.HTMLProps { room?: Room; @@ -44,6 +45,7 @@ export default function RoomTopic({ const ref = useRef(); const topic = useTopic(room); + const body = topicToHtml(topic?.text, topic?.html, ref); const onClick = useCallback((e: React.MouseEvent) => { props.onClick?.(e); @@ -62,6 +64,7 @@ export default function RoomTopic({ useDispatcher(dis, (payload) => { if (payload.action === Action.ShowRoomTopic) { const canSetTopic = room.currentState.maySendStateEvent(EventType.RoomTopic, client.getUserId()); + const body = topicToHtml(topic?.text, topic?.html, ref, true); const modal = Modal.createDialog(InfoDialog, { title: room.name, @@ -74,7 +77,7 @@ export default function RoomTopic({ } }} > - { topic } + { body } { canSetTopic && - { topic } + { body }
; diff --git a/src/components/views/room_settings/RoomProfileSettings.tsx b/src/components/views/room_settings/RoomProfileSettings.tsx index 85f5ab7600..6e4900478a 100644 --- a/src/components/views/room_settings/RoomProfileSettings.tsx +++ b/src/components/views/room_settings/RoomProfileSettings.tsx @@ -22,6 +22,7 @@ import Field from "../elements/Field"; import { mediaFromMxc } from "../../../customisations/Media"; import AccessibleButton from "../elements/AccessibleButton"; import AvatarSetting from "../settings/AvatarSetting"; +import { htmlSerializeFromMdIfNeeded } from '../../../editor/serialize'; import { chromeFileInputFix } from "../../../utils/BrowserWorkarounds"; interface IProps { @@ -142,7 +143,8 @@ export default class RoomProfileSettings extends React.Component } if (this.state.originalTopic !== this.state.topic) { - await client.setRoomTopic(this.props.roomId, this.state.topic); + const html = htmlSerializeFromMdIfNeeded(this.state.topic, { forceHTML: false }); + await client.setRoomTopic(this.props.roomId, this.state.topic, html); newState.originalTopic = this.state.topic; } diff --git a/src/components/views/spaces/SpaceSettingsGeneralTab.tsx b/src/components/views/spaces/SpaceSettingsGeneralTab.tsx index 8247ee25e9..c6549f4ba2 100644 --- a/src/components/views/spaces/SpaceSettingsGeneralTab.tsx +++ b/src/components/views/spaces/SpaceSettingsGeneralTab.tsx @@ -25,6 +25,7 @@ import AccessibleButton from "../elements/AccessibleButton"; import SpaceBasicSettings from "./SpaceBasicSettings"; import { avatarUrlForRoom } from "../../../Avatar"; import { IDialogProps } from "../dialogs/IDialogProps"; +import { htmlSerializeFromMdIfNeeded } from "../../../editor/serialize"; import { leaveSpace } from "../../../utils/leave-behaviour"; import { getTopic } from "../../../hooks/room/useTopic"; @@ -47,7 +48,7 @@ const SpaceSettingsGeneralTab = ({ matrixClient: cli, space, onFinished }: IProp const canSetName = space.currentState.maySendStateEvent(EventType.RoomName, userId); const nameChanged = name !== space.name; - const currentTopic = getTopic(space); + const currentTopic = getTopic(space).text; const [topic, setTopic] = useState(currentTopic); const canSetTopic = space.currentState.maySendStateEvent(EventType.RoomTopic, userId); const topicChanged = topic !== currentTopic; @@ -77,7 +78,8 @@ const SpaceSettingsGeneralTab = ({ matrixClient: cli, space, onFinished }: IProp } if (topicChanged) { - promises.push(cli.setRoomTopic(space.roomId, topic)); + const htmlTopic = htmlSerializeFromMdIfNeeded(topic, { forceHTML: false }); + promises.push(cli.setRoomTopic(space.roomId, topic, htmlTopic)); } const results = await Promise.allSettled(promises); diff --git a/src/editor/serialize.ts b/src/editor/serialize.ts index 7c4d62e9ab..61e24a64ff 100644 --- a/src/editor/serialize.ts +++ b/src/editor/serialize.ts @@ -62,7 +62,11 @@ export function htmlSerializeIfNeeded( return escapeHtml(textSerialize(model)).replace(/\n/g, '
'); } - let md = mdSerialize(model); + const md = mdSerialize(model); + return htmlSerializeFromMdIfNeeded(md, { forceHTML }); +} + +export function htmlSerializeFromMdIfNeeded(md: string, { forceHTML = false } = {}): string { // copy of raw input to remove unwanted math later const orig = md; diff --git a/src/hooks/room/useTopic.ts b/src/hooks/room/useTopic.ts index b01065c37c..3f276a4e64 100644 --- a/src/hooks/room/useTopic.ts +++ b/src/hooks/room/useTopic.ts @@ -19,14 +19,17 @@ import { EventType } from "matrix-js-sdk/src/@types/event"; import { MatrixEvent } from "matrix-js-sdk/src/models/event"; import { Room } from "matrix-js-sdk/src/models/room"; import { RoomStateEvent } from "matrix-js-sdk/src/models/room-state"; +import { parseTopicContent, TopicState } from "matrix-js-sdk/src/content-helpers"; +import { MRoomTopicEventContent } from "matrix-js-sdk/src/@types/topic"; import { useTypedEventEmitter } from "../useEventEmitter"; export const getTopic = (room: Room) => { - return room?.currentState?.getStateEvents(EventType.RoomTopic, "")?.getContent()?.topic; + const content: MRoomTopicEventContent = room?.currentState?.getStateEvents(EventType.RoomTopic, "")?.getContent(); + return !!content ? parseTopicContent(content) : null; }; -export function useTopic(room: Room): string { +export function useTopic(room: Room): TopicState { const [topic, setTopic] = useState(getTopic(room)); useTypedEventEmitter(room.currentState, RoomStateEvent.Events, (ev: MatrixEvent) => { if (ev.getType() !== EventType.RoomTopic) return; diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index a468502f53..b1fa02d4ce 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -892,6 +892,7 @@ "Offline encrypted messaging using dehydrated devices": "Offline encrypted messaging using dehydrated devices", "Show extensible event representation of events": "Show extensible event representation of events", "Show current avatar and name for users in message history": "Show current avatar and name for users in message history", + "Show HTML representation of room topics": "Show HTML representation of room topics", "Show info about bridges in room settings": "Show info about bridges in room settings", "Use new room breadcrumbs": "Use new room breadcrumbs", "New search experience": "New search experience", diff --git a/src/settings/Settings.tsx b/src/settings/Settings.tsx index 09987c4386..53ff2ceb2d 100644 --- a/src/settings/Settings.tsx +++ b/src/settings/Settings.tsx @@ -330,6 +330,13 @@ export const SETTINGS: {[setting: string]: ISetting} = { supportedLevels: [SettingLevel.ACCOUNT], default: null, }, + "feature_html_topic": { + isFeature: true, + labsGroup: LabGroup.Rooms, + supportedLevels: LEVELS_FEATURE, + displayName: _td("Show HTML representation of room topics"), + default: false, + }, "feature_bridge_state": { isFeature: true, labsGroup: LabGroup.Rooms, diff --git a/test/HtmlUtils-test.tsx b/test/HtmlUtils-test.tsx new file mode 100644 index 0000000000..de862b407a --- /dev/null +++ b/test/HtmlUtils-test.tsx @@ -0,0 +1,66 @@ +/* +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 from 'react'; +import { mount } from 'enzyme'; +import { mocked } from 'jest-mock'; + +import { topicToHtml } from '../src/HtmlUtils'; +import SettingsStore from '../src/settings/SettingsStore'; + +jest.mock("../src/settings/SettingsStore"); + +const enableHtmlTopicFeature = () => { + mocked(SettingsStore).getValue.mockImplementation((arg) => { + return arg === "feature_html_topic"; + }); +}; + +describe('HtmlUtils', () => { + it('converts plain text topic to HTML', () => { + const component = mount(
{ topicToHtml("pizza", null, null, false) }
); + const wrapper = component.render(); + expect(wrapper.children().first().html()).toEqual("pizza"); + }); + + it('converts plain text topic with emoji to HTML', () => { + const component = mount(
{ topicToHtml("pizza 🍕", null, null, false) }
); + const wrapper = component.render(); + expect(wrapper.children().first().html()).toEqual("pizza 🍕"); + }); + + it('converts literal HTML topic to HTML', async () => { + enableHtmlTopicFeature(); + const component = mount(
{ topicToHtml("pizza", null, null, false) }
); + const wrapper = component.render(); + expect(wrapper.children().first().html()).toEqual("<b>pizza</b>"); + }); + + it('converts true HTML topic to HTML', async () => { + enableHtmlTopicFeature(); + const component = mount(
{ topicToHtml("**pizza**", "pizza", null, false) }
); + const wrapper = component.render(); + expect(wrapper.children().first().html()).toEqual("pizza"); + }); + + it('converts true HTML topic with emoji to HTML', async () => { + enableHtmlTopicFeature(); + const component = mount(
{ topicToHtml("**pizza** 🍕", "pizza 🍕", null, false) }
); + const wrapper = component.render(); + expect(wrapper.children().first().html()) + .toEqual("pizza 🍕"); + }); +}); diff --git a/test/SlashCommands-test.tsx b/test/SlashCommands-test.tsx new file mode 100644 index 0000000000..f2b15fb729 --- /dev/null +++ b/test/SlashCommands-test.tsx @@ -0,0 +1,40 @@ +/* +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 { MatrixClient } from 'matrix-js-sdk/src/matrix'; + +import { getCommand } from '../src/SlashCommands'; +import { createTestClient } from './test-utils'; +import { MatrixClientPeg } from '../src/MatrixClientPeg'; + +describe('SlashCommands', () => { + let client: MatrixClient; + + beforeEach(() => { + client = createTestClient(); + jest.spyOn(MatrixClientPeg, 'get').mockReturnValue(client); + }); + + describe('/topic', () => { + it('sets topic', async () => { + const command = getCommand("/topic pizza"); + expect(command.cmd).toBeDefined(); + expect(command.args).toBeDefined(); + await command.cmd.run("room-id", null, command.args); + expect(client.setRoomTopic).toHaveBeenCalledWith("room-id", "pizza", undefined); + }); + }); +}); diff --git a/test/editor/serialize-test.ts b/test/editor/serialize-test.ts index d948285901..dcad03c9c8 100644 --- a/test/editor/serialize-test.ts +++ b/test/editor/serialize-test.ts @@ -94,5 +94,12 @@ describe('editor/serialize', function() { const html = htmlSerializeIfNeeded(model, { useMarkdown: false }); expect(html).toBe('\\*hello\\* world < hey world!'); }); + + it('plaintext remains plaintext even when forcing html', function() { + const pc = createPartCreator(); + const model = new EditorModel([pc.plain("hello world")], pc); + const html = htmlSerializeIfNeeded(model, { forceHTML: true, useMarkdown: false }); + expect(html).toBe("hello world"); + }); }); }); diff --git a/test/test-utils/test-utils.ts b/test/test-utils/test-utils.ts index 6db49c731d..14fdb65046 100644 --- a/test/test-utils/test-utils.ts +++ b/test/test-utils/test-utils.ts @@ -116,6 +116,7 @@ export function createTestClient(): MatrixClient { mxcUrlToHttp: (mxc) => `http://this.is.a.url/${mxc.substring(6)}`, setAccountData: jest.fn(), setRoomAccountData: jest.fn(), + setRoomTopic: jest.fn(), sendTyping: jest.fn().mockResolvedValue({}), sendMessage: () => jest.fn().mockResolvedValue({}), sendStateEvent: jest.fn().mockResolvedValue(undefined), diff --git a/test/useTopic-test.tsx b/test/useTopic-test.tsx index 75096b43e4..7052375eb8 100644 --- a/test/useTopic-test.tsx +++ b/test/useTopic-test.tsx @@ -42,7 +42,7 @@ describe("useTopic", () => { function RoomTopic() { const topic = useTopic(room); - return

{ topic }

; + return

{ topic.text }

; } const wrapper = mount();