Fix regression around pasting links (#8537)

* Fix regression around pasting links

* Add tests

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";
@@ -92,7 +92,7 @@ function selectionEquals(a: Partial<Selection>, b: Selection): boolean {
 interface IProps {
     model: EditorModel;
     room: Room;
-    threadId: string;
+    threadId?: string;
     placeholder?: string;
     label?: string;
     initialCaret?: DocumentOffset;
@@ -333,28 +333,29 @@ export default class BasicMessageEditor extends React.Component<IProps, IState>
 
     private onPaste = (event: ClipboardEvent<HTMLDivElement>): 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;
         }
 
         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;
+            parts = serializedTextParts.map(p => partCreator.deserializePart(p));
         } 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);
         }

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" && (<HTMLElement>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,

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);

test/components/views/rooms/BasicMessageComposer-test.tsx

@@ -0,0 +1,65 @@
+/*
+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).toHaveLength(1);
+        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((
+        <BasicMessageComposer model={model} room={room} />
+    ));
+}

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 DocumentPosition from "../../src/editor/position";
 
 class MockAutoComplete {
     public _updateCallback;
@@ -78,11 +79,11 @@ export function createPartCreator(completions = []) {
 }
 
 export function createRenderer() {
-    const render = (c) => {
+    const render = (c: DocumentPosition) => {
         render.caret = c;
         render.count += 1;
     };
     render.count = 0;
-    render.caret = null;
+    render.caret = null as DocumentPosition;
     return render;
 }

test/editor/operations-test.ts

@@ -17,21 +17,88 @@ 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';
 
 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|", ""],
+        ])("converts %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);