From a7b429b17096ff25308e739a7ca47d5307a36e92 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 9 May 2022 11:08:44 +0100 Subject: [PATCH 1/4] Fix regression around pasting links --- .../views/rooms/BasicMessageComposer.tsx | 16 +++++++++------- src/editor/dom.ts | 12 +++++++----- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/components/views/rooms/BasicMessageComposer.tsx b/src/components/views/rooms/BasicMessageComposer.tsx index 004c20daef2..b9b178bfbb7 100644 --- a/src/components/views/rooms/BasicMessageComposer.tsx +++ b/src/components/views/rooms/BasicMessageComposer.tsx @@ -28,7 +28,7 @@ import { formatRange, formatRangeAsLink, replaceRangeAndMoveCaret, toggleInlineF from '../../../editor/operations'; import { getCaretOffsetAndText, getRangeForSelection } from '../../../editor/dom'; import Autocomplete, { generateCompletionDomId } from '../rooms/Autocomplete'; -import { getAutoCompleteCreator, Type } from '../../../editor/parts'; +import { getAutoCompleteCreator, Part, Type } from '../../../editor/parts'; import { parseEvent, parsePlainTextMessage } from '../../../editor/deserialize'; import { renderModel } from '../../../editor/render'; import TypingStore from "../../../stores/TypingStore"; @@ -340,21 +340,23 @@ export default class BasicMessageEditor extends React.Component const { model } = this.props; const { partCreator } = model; + const plainText = event.clipboardData.getData("text/plain"); const partsText = event.clipboardData.getData("application/x-element-composer"); - let parts; + + let parts: Part[]; if (partsText) { const serializedTextParts = JSON.parse(partsText); const deserializedParts = serializedTextParts.map(p => partCreator.deserializePart(p)); parts = deserializedParts; } else { - const text = event.clipboardData.getData("text/plain"); - parts = parsePlainTextMessage(text, partCreator, { shouldEscape: false }); + parts = parsePlainTextMessage(plainText, partCreator, { shouldEscape: false }); } - const textToInsert = event.clipboardData.getData("text/plain"); + this.modifiedFlag = true; const range = getRangeForSelection(this.editorRef.current, model, document.getSelection()); - if (textToInsert && linkify.test(textToInsert)) { - formatRangeAsLink(range, textToInsert); + + if (plainText && range.length > 0 && linkify.test(plainText)) { + formatRangeAsLink(range, plainText); } else { replaceRangeAndMoveCaret(range, parts); } diff --git a/src/editor/dom.ts b/src/editor/dom.ts index 6226f74acb8..0700ecb482e 100644 --- a/src/editor/dom.ts +++ b/src/editor/dom.ts @@ -17,6 +17,8 @@ limitations under the License. import { CARET_NODE_CHAR, isCaretNode } from "./render"; import DocumentOffset from "./offset"; +import EditorModel from "./model"; +import Range from "./range"; type Predicate = (node: Node) => boolean; type Callback = (node: Node) => void; @@ -122,7 +124,7 @@ function getTextAndOffsetToNode(editor: HTMLDivElement, selectionNode: Node) { let foundNode = false; let text = ""; - function enterNodeCallback(node) { + function enterNodeCallback(node: HTMLElement) { if (!foundNode) { if (node === selectionNode) { foundNode = true; @@ -148,12 +150,12 @@ function getTextAndOffsetToNode(editor: HTMLDivElement, selectionNode: Node) { return true; } - function leaveNodeCallback(node) { + function leaveNodeCallback(node: HTMLElement) { // if this is not the last DIV (which are only used as line containers atm) // we don't just check if there is a nextSibling because sometimes the caret ends up // after the last DIV and it creates a newline if you type then, // whereas you just want it to be appended to the current line - if (node.tagName === "DIV" && node.nextSibling && node.nextSibling.tagName === "DIV") { + if (node.tagName === "DIV" && (node.nextSibling)?.tagName === "DIV") { text += "\n"; if (!foundNode) { offsetToNode += 1; @@ -167,7 +169,7 @@ function getTextAndOffsetToNode(editor: HTMLDivElement, selectionNode: Node) { } // get text value of text node, ignoring ZWS if it's a caret node -function getTextNodeValue(node) { +function getTextNodeValue(node: Node): string { const nodeText = node.nodeValue; // filter out ZWS for caret nodes if (isCaretNode(node.parentElement)) { @@ -184,7 +186,7 @@ function getTextNodeValue(node) { } } -export function getRangeForSelection(editor, model, selection) { +export function getRangeForSelection(editor: HTMLDivElement, model: EditorModel, selection: Selection): Range { const focusOffset = getSelectionOffsetAndText( editor, selection.focusNode, From a3cd1622007e27b4c43171d9a2f70cd54f3fb77d Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 9 May 2022 12:47:58 +0100 Subject: [PATCH 2/4] Add tests --- .../views/rooms/BasicMessageComposer.tsx | 7 +- src/editor/operations.ts | 6 +- .../views/rooms/BasicMessageComposer-test.tsx | 64 +++++++++++++++ test/editor/mock.ts | 5 +- test/editor/operations-test.ts | 82 +++++++++++++++++-- 5 files changed, 147 insertions(+), 17 deletions(-) create mode 100644 test/components/views/rooms/BasicMessageComposer-test.tsx diff --git a/src/components/views/rooms/BasicMessageComposer.tsx b/src/components/views/rooms/BasicMessageComposer.tsx index b9b178bfbb7..667d5a42a4e 100644 --- a/src/components/views/rooms/BasicMessageComposer.tsx +++ b/src/components/views/rooms/BasicMessageComposer.tsx @@ -92,7 +92,7 @@ function selectionEquals(a: Partial, b: Selection): boolean { interface IProps { model: EditorModel; room: Room; - threadId: string; + threadId?: string; placeholder?: string; label?: string; initialCaret?: DocumentOffset; @@ -333,7 +333,7 @@ export default class BasicMessageEditor extends React.Component private onPaste = (event: ClipboardEvent): boolean => { event.preventDefault(); // we always handle the paste ourselves - if (this.props.onPaste && this.props.onPaste(event, this.props.model)) { + if (this.props.onPaste?.(event, this.props.model)) { // to prevent double handling, allow props.onPaste to skip internal onPaste return true; } @@ -346,8 +346,7 @@ export default class BasicMessageEditor extends React.Component let parts: Part[]; if (partsText) { const serializedTextParts = JSON.parse(partsText); - const deserializedParts = serializedTextParts.map(p => partCreator.deserializePart(p)); - parts = deserializedParts; + parts = serializedTextParts.map(p => partCreator.deserializePart(p)); } else { parts = parsePlainTextMessage(plainText, partCreator, { shouldEscape: false }); } diff --git a/src/editor/operations.ts b/src/editor/operations.ts index f4dd61faa07..39cb6a56769 100644 --- a/src/editor/operations.ts +++ b/src/editor/operations.ts @@ -219,14 +219,12 @@ export function formatRangeAsCode(range: Range): void { export function formatRangeAsLink(range: Range, text?: string) { const { model } = range; const { partCreator } = model; - const linkRegex = /\[(.*?)\]\(.*?\)/g; + const linkRegex = /\[(.*?)]\(.*?\)/g; const isFormattedAsLink = linkRegex.test(range.text); if (isFormattedAsLink) { const linkDescription = range.text.replace(linkRegex, "$1"); const newParts = [partCreator.plain(linkDescription)]; - const prefixLength = 1; - const suffixLength = range.length - (linkDescription.length + 2); - replaceRangeAndAutoAdjustCaret(range, newParts, true, prefixLength, suffixLength); + replaceRangeAndMoveCaret(range, newParts, 0); } else { // We set offset to -1 here so that the caret lands between the brackets replaceRangeAndMoveCaret(range, [partCreator.plain("[" + range.text + "]" + "(" + (text ?? "") + ")")], -1); diff --git a/test/components/views/rooms/BasicMessageComposer-test.tsx b/test/components/views/rooms/BasicMessageComposer-test.tsx new file mode 100644 index 00000000000..9ac9f0cba0d --- /dev/null +++ b/test/components/views/rooms/BasicMessageComposer-test.tsx @@ -0,0 +1,64 @@ +/* +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, ReactWrapper } from 'enzyme'; +import { MatrixClient, Room } from 'matrix-js-sdk/src/matrix'; + +import BasicMessageComposer from '../../../../src/components/views/rooms/BasicMessageComposer'; +import * as TestUtils from "../../../test-utils"; +import { MatrixClientPeg } from "../../../../src/MatrixClientPeg"; +import EditorModel from "../../../../src/editor/model"; +import { createPartCreator, createRenderer } from "../../../editor/mock"; + +describe("BasicMessageComposer", () => { + const renderer = createRenderer(); + const pc = createPartCreator(); + + beforeEach(() => { + TestUtils.stubClient(); + }); + + it("should allow a user to paste a URL without it being mangled", () => { + const model = new EditorModel([], pc, renderer); + + const wrapper = render(model); + + wrapper.find(".mx_BasicMessageComposer_input").simulate("paste", { + clipboardData: { + getData: type => { + if (type === "text/plain") { + return "https://element.io"; + } + }, + }, + }); + + expect(model.parts[0].text).toBe("https://element.io"); + }); +}); + +function render(model: EditorModel): ReactWrapper { + const client: MatrixClient = MatrixClientPeg.get(); + + const roomId = '!1234567890:domain'; + const userId = client.getUserId(); + const room = new Room(roomId, client, userId); + + return mount(( + + )); +} diff --git a/test/editor/mock.ts b/test/editor/mock.ts index bc6eafc8cc0..23cc03be385 100644 --- a/test/editor/mock.ts +++ b/test/editor/mock.ts @@ -18,6 +18,7 @@ import { Room, MatrixClient } from "matrix-js-sdk/src/matrix"; import AutocompleteWrapperModel from "../../src/editor/autocomplete"; import { PartCreator } from "../../src/editor/parts"; +import { Caret } from "../../src/editor/caret"; class MockAutoComplete { public _updateCallback; @@ -78,11 +79,11 @@ export function createPartCreator(completions = []) { } export function createRenderer() { - const render = (c) => { + const render = (c: Caret) => { render.caret = c; render.count += 1; }; render.count = 0; - render.caret = null; + render.caret = null as Caret; return render; } diff --git a/test/editor/operations-test.ts b/test/editor/operations-test.ts index 3e4de224179..39526138439 100644 --- a/test/editor/operations-test.ts +++ b/test/editor/operations-test.ts @@ -17,21 +17,89 @@ limitations under the License. import EditorModel from "../../src/editor/model"; import { createPartCreator, createRenderer } from "./mock"; import { - toggleInlineFormat, - selectRangeOfWordAtCaret, formatRange, formatRangeAsCode, + formatRangeAsLink, + selectRangeOfWordAtCaret, + toggleInlineFormat, } from "../../src/editor/operations"; import { Formatting } from "../../src/components/views/rooms/MessageComposerFormatBar"; import { longestBacktickSequence } from '../../src/editor/deserialize'; +import DocumentPosition from "../../src/editor/position"; const SERIALIZED_NEWLINE = { "text": "\n", "type": "newline" }; -describe('editor/operations: formatting operations', () => { - describe('toggleInlineFormat', () => { - it('works for words', () => { - const renderer = createRenderer(); - const pc = createPartCreator(); +describe("editor/operations: formatting operations", () => { + const renderer = createRenderer(); + const pc = createPartCreator(); + + describe("formatRange", () => { + it.each([ + [Formatting.Bold, "hello **world**!"], + ])("should correctly wrap format %s", (formatting: Formatting, expected: string) => { + const model = new EditorModel([ + pc.plain("hello world!"), + ], pc, renderer); + + const range = model.startRange(model.positionForOffset(6, false), + model.positionForOffset(11, false)); // around "world" + + expect(range.parts[0].text).toBe("world"); + expect(model.serializeParts()).toEqual([{ "text": "hello world!", "type": "plain" }]); + formatRange(range, formatting); + expect(model.serializeParts()).toEqual([{ "text": expected, "type": "plain" }]); + }); + + it("should apply to word range is within if length 0", () => { + const model = new EditorModel([ + pc.plain("hello world!"), + ], pc, renderer); + + const range = model.startRange(model.positionForOffset(6, false)); + + expect(model.serializeParts()).toEqual([{ "text": "hello world!", "type": "plain" }]); + formatRange(range, Formatting.Bold); + expect(model.serializeParts()).toEqual([{ "text": "hello **world!**", "type": "plain" }]); + }); + + it("should do nothing for a range with length 0 at initialisation", () => { + const model = new EditorModel([ + pc.plain("hello world!"), + ], pc, renderer); + + const range = model.startRange(model.positionForOffset(6, false)); + range.setWasEmpty(false); + + expect(model.serializeParts()).toEqual([{ "text": "hello world!", "type": "plain" }]); + formatRange(range, Formatting.Bold); + expect(model.serializeParts()).toEqual([{ "text": "hello world!", "type": "plain" }]); + }); + }); + + describe("formatRangeAsLink", () => { + it.each([ + // Caret is denoted by | in the expectation string + ["testing", "[testing](|)", ""], + ["testing", "[testing](foobar|)", "foobar"], + ["[testing]()", "testing|", ""], + ["[testing](foobar)", "testing|", ""], + ])("%s -> %s", (input: string, expectation: string, text: string) => { + const model = new EditorModel([ + pc.plain(`foo ${input} bar`), + ], pc, renderer); + + const range = model.startRange(model.positionForOffset(4, false), + model.positionForOffset(4 + input.length, false)); // around input + + expect(range.parts[0].text).toBe(input); + formatRangeAsLink(range, text); + expect((renderer.caret).offset).toBe(4 + expectation.indexOf("|")); + expect(model.parts[0].text).toBe("foo " + expectation.replace("|", "") + " bar"); + }); + }); + + describe("toggleInlineFormat", () => { + it("works for words", () => { const model = new EditorModel([ pc.plain("hello world!"), ], pc, renderer); From 5b5ace261431d31ee0a7a40a855f698396697beb Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 9 May 2022 13:20:25 +0100 Subject: [PATCH 3/4] iterate tests --- test/components/views/rooms/BasicMessageComposer-test.tsx | 1 + test/editor/operations-test.ts | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/test/components/views/rooms/BasicMessageComposer-test.tsx b/test/components/views/rooms/BasicMessageComposer-test.tsx index 9ac9f0cba0d..838119492a5 100644 --- a/test/components/views/rooms/BasicMessageComposer-test.tsx +++ b/test/components/views/rooms/BasicMessageComposer-test.tsx @@ -47,6 +47,7 @@ describe("BasicMessageComposer", () => { }, }); + expect(model.parts).toHaveLength(1); expect(model.parts[0].text).toBe("https://element.io"); }); }); diff --git a/test/editor/operations-test.ts b/test/editor/operations-test.ts index 39526138439..ea409a53f89 100644 --- a/test/editor/operations-test.ts +++ b/test/editor/operations-test.ts @@ -83,7 +83,7 @@ describe("editor/operations: formatting operations", () => { ["testing", "[testing](foobar|)", "foobar"], ["[testing]()", "testing|", ""], ["[testing](foobar)", "testing|", ""], - ])("%s -> %s", (input: string, expectation: string, text: string) => { + ])("converts %s -> %s", (input: string, expectation: string, text: string) => { const model = new EditorModel([ pc.plain(`foo ${input} bar`), ], pc, renderer); From 264fe82b6ec36a8ee872974f7f964c040b0cbf2a Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 9 May 2022 13:21:30 +0100 Subject: [PATCH 4/4] Fix mock --- test/editor/mock.ts | 6 +++--- test/editor/operations-test.ts | 3 +-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/test/editor/mock.ts b/test/editor/mock.ts index 23cc03be385..bddddbf7cb6 100644 --- a/test/editor/mock.ts +++ b/test/editor/mock.ts @@ -18,7 +18,7 @@ import { Room, MatrixClient } from "matrix-js-sdk/src/matrix"; import AutocompleteWrapperModel from "../../src/editor/autocomplete"; import { PartCreator } from "../../src/editor/parts"; -import { Caret } from "../../src/editor/caret"; +import DocumentPosition from "../../src/editor/position"; class MockAutoComplete { public _updateCallback; @@ -79,11 +79,11 @@ export function createPartCreator(completions = []) { } export function createRenderer() { - const render = (c: Caret) => { + const render = (c: DocumentPosition) => { render.caret = c; render.count += 1; }; render.count = 0; - render.caret = null as Caret; + render.caret = null as DocumentPosition; return render; } diff --git a/test/editor/operations-test.ts b/test/editor/operations-test.ts index ea409a53f89..6af732e5bd5 100644 --- a/test/editor/operations-test.ts +++ b/test/editor/operations-test.ts @@ -25,7 +25,6 @@ import { } from "../../src/editor/operations"; import { Formatting } from "../../src/components/views/rooms/MessageComposerFormatBar"; import { longestBacktickSequence } from '../../src/editor/deserialize'; -import DocumentPosition from "../../src/editor/position"; const SERIALIZED_NEWLINE = { "text": "\n", "type": "newline" }; @@ -93,7 +92,7 @@ describe("editor/operations: formatting operations", () => { expect(range.parts[0].text).toBe(input); formatRangeAsLink(range, text); - expect((renderer.caret).offset).toBe(4 + expectation.indexOf("|")); + expect(renderer.caret.offset).toBe(4 + expectation.indexOf("|")); expect(model.parts[0].text).toBe("foo " + expectation.replace("|", "") + " bar"); }); });