From b9f5f4908696a2f6b6784f02bd9701e5a91471ce Mon Sep 17 00:00:00 2001 From: Johannes Marbach Date: Thu, 29 Jun 2023 19:44:49 +0200 Subject: [PATCH 1/5] Handle newlines in user pills Fixes: vector-im/element-web#10994 --- src/Markdown.ts | 6 +++--- src/editor/serialize.ts | 4 +++- test/editor/deserialize-test.ts | 8 ++++++++ test/editor/serialize-test.ts | 7 ++++++- 4 files changed, 20 insertions(+), 5 deletions(-) diff --git a/src/Markdown.ts b/src/Markdown.ts index 05efdcfd56d..3fa6e07218e 100644 --- a/src/Markdown.ts +++ b/src/Markdown.ts @@ -22,7 +22,7 @@ import { logger } from "matrix-js-sdk/src/logger"; import { linkify } from "./linkify-matrix"; -const ALLOWED_HTML_TAGS = ["sub", "sup", "del", "u"]; +const ALLOWED_HTML_TAGS = ["sub", "sup", "del", "u", "br", "br/"]; // These types of node are definitely text const TEXT_NODES = ["text", "softbreak", "linebreak", "paragraph", "document"]; @@ -36,8 +36,8 @@ function isAllowedHtmlTag(node: commonmark.Node): boolean { return true; } - // Regex won't work for tags with attrs, but we only - // allow anyway. + // Regex won't work for tags with attrs, but the oens we allow + // shouldn't really have any anyway. const matches = /^<\/?(.*)>$/.exec(node.literal); if (matches && matches.length == 2) { const tag = matches[1]; diff --git a/src/editor/serialize.ts b/src/editor/serialize.ts index 6bc45a5657d..00882f27941 100644 --- a/src/editor/serialize.ts +++ b/src/editor/serialize.ts @@ -48,7 +48,9 @@ export function mdSerialize(model: EditorModel): string { case Type.UserPill: return ( html + - `[${part.text.replace(/[[\\\]]/g, (c) => "\\" + c)}](${makeGenericPermalink(part.resourceId)})` + `[${part.text.replace(/[[\\\]]/g, (c) => "\\" + c).replace(/\n/g, "
")}](${makeGenericPermalink( + part.resourceId, + )})` ); } }, ""); diff --git a/test/editor/deserialize-test.ts b/test/editor/deserialize-test.ts index 908b83a6512..a7c4595eb4e 100644 --- a/test/editor/deserialize-test.ts +++ b/test/editor/deserialize-test.ts @@ -183,6 +183,14 @@ describe("editor/deserialize", function () { expect(parts[1]).toStrictEqual({ type: "user-pill", text: "Alice]", resourceId: "@alice:hs.tld" }); expect(parts[2]).toStrictEqual({ type: "plain", text: "!" }); }); + it("user pill with displayname containing linebreak square bracket", function () { + const html = 'Hi Alice
123
!'; + const parts = normalize(parseEvent(htmlMessage(html), createPartCreator())); + expect(parts.length).toBe(3); + expect(parts[0]).toStrictEqual({ type: "plain", text: "Hi " }); + expect(parts[1]).toStrictEqual({ type: "user-pill", text: "Alice123", resourceId: "@alice:hs.tld" }); + expect(parts[2]).toStrictEqual({ type: "plain", text: "!" }); + }); it("room pill", function () { const html = 'Try #room:hs.tld?'; const parts = normalize(parseEvent(htmlMessage(html), createPartCreator())); diff --git a/test/editor/serialize-test.ts b/test/editor/serialize-test.ts index ce4815658f7..b78ae308c85 100644 --- a/test/editor/serialize-test.ts +++ b/test/editor/serialize-test.ts @@ -63,6 +63,12 @@ describe("editor/serialize", function () { const html = htmlSerializeIfNeeded(model, {}); expect(html).toBe('Displayname]'); }); + it("displaynames containing a newline work", function () { + const pc = createPartCreator(); + const model = new EditorModel([pc.userPill("Display\nname", "@user:server")], pc); + const html = htmlSerializeIfNeeded(model, {}); + expect(html).toBe('Display
name
'); + }); it("escaped markdown should not retain backslashes", function () { const pc = createPartCreator(); const model = new EditorModel([pc.plain("\\*hello\\* world")], pc); @@ -96,7 +102,6 @@ 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); From 468748daa1bac8d4e4ad7b81a03e65478edf568c Mon Sep 17 00:00:00 2001 From: Johannes Marbach Date: Mon, 3 Jul 2023 19:41:26 +0200 Subject: [PATCH 2/5] Fix typo in comment Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- src/Markdown.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Markdown.ts b/src/Markdown.ts index 3fa6e07218e..709c34f31b8 100644 --- a/src/Markdown.ts +++ b/src/Markdown.ts @@ -36,7 +36,7 @@ function isAllowedHtmlTag(node: commonmark.Node): boolean { return true; } - // Regex won't work for tags with attrs, but the oens we allow + // Regex won't work for tags with attrs, but the tags we allow // shouldn't really have any anyway. const matches = /^<\/?(.*)>$/.exec(node.literal); if (matches && matches.length == 2) { From 31c70402f60050ad81a5cbddea3901461e45130a Mon Sep 17 00:00:00 2001 From: Johannes Marbach Date: Mon, 3 Jul 2023 19:59:12 +0200 Subject: [PATCH 3/5] Refactor link generation for better readability --- src/editor/serialize.ts | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/src/editor/serialize.ts b/src/editor/serialize.ts index 00882f27941..35c16afeba2 100644 --- a/src/editor/serialize.ts +++ b/src/editor/serialize.ts @@ -36,22 +36,20 @@ export function mdSerialize(model: EditorModel): string { case Type.PillCandidate: case Type.AtRoomPill: return html + part.text; - case Type.RoomPill: + case Type.RoomPill: { + const url = makeGenericPermalink(part.resourceId); + // Escape square brackets and backslashes // Here we use the resourceId for compatibility with non-rich text clients // See https://github.com/vector-im/element-web/issues/16660 - return ( - html + - `[${part.resourceId.replace(/[[\\\]]/g, (c) => "\\" + c)}](${makeGenericPermalink( - part.resourceId, - )})` - ); - case Type.UserPill: - return ( - html + - `[${part.text.replace(/[[\\\]]/g, (c) => "\\" + c).replace(/\n/g, "
")}](${makeGenericPermalink( - part.resourceId, - )})` - ); + const title = part.resourceId.replace(/[[\\\]]/g, (c) => "\\" + c); + return html + `[${title}](${url})`; + } + case Type.UserPill: { + const url = makeGenericPermalink(part.resourceId); + // Escape square brackets and backslashes; convert newlines to HTML + const title = part.text.replace(/[[\\\]]/g, (c) => "\\" + c).replace(/\n/g, "
"); + return html + `[${title}](${url})`; + } } }, ""); } From d6d1933f261c619cf274348ac7f3acb061ddbb5a Mon Sep 17 00:00:00 2001 From: Johannes Marbach Date: Mon, 3 Jul 2023 20:00:03 +0200 Subject: [PATCH 4/5] Use `
` instead of `
` --- src/editor/serialize.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/editor/serialize.ts b/src/editor/serialize.ts index 35c16afeba2..8391183a654 100644 --- a/src/editor/serialize.ts +++ b/src/editor/serialize.ts @@ -47,7 +47,7 @@ export function mdSerialize(model: EditorModel): string { case Type.UserPill: { const url = makeGenericPermalink(part.resourceId); // Escape square brackets and backslashes; convert newlines to HTML - const title = part.text.replace(/[[\\\]]/g, (c) => "\\" + c).replace(/\n/g, "
"); + const title = part.text.replace(/[[\\\]]/g, (c) => "\\" + c).replace(/\n/g, "
"); return html + `[${title}](${url})`; } } From 33ba3bec1c3cd7bd81cfe5b5c0e49934bdd41fba Mon Sep 17 00:00:00 2001 From: Johannes Marbach Date: Tue, 4 Jul 2023 19:55:16 +0200 Subject: [PATCH 5/5] Fix copy/paste error --- test/editor/deserialize-test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/editor/deserialize-test.ts b/test/editor/deserialize-test.ts index a7c4595eb4e..275f34ca8f1 100644 --- a/test/editor/deserialize-test.ts +++ b/test/editor/deserialize-test.ts @@ -183,8 +183,8 @@ describe("editor/deserialize", function () { expect(parts[1]).toStrictEqual({ type: "user-pill", text: "Alice]", resourceId: "@alice:hs.tld" }); expect(parts[2]).toStrictEqual({ type: "plain", text: "!" }); }); - it("user pill with displayname containing linebreak square bracket", function () { - const html = 'Hi Alice
123
!'; + it("user pill with displayname containing linebreak", function () { + const html = 'Hi Alice
123
!'; const parts = normalize(parseEvent(htmlMessage(html), createPartCreator())); expect(parts.length).toBe(3); expect(parts[0]).toStrictEqual({ type: "plain", text: "Hi " });