Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Fix regression around pasting links #8537

Merged
merged 4 commits into from
May 9, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 12 additions & 11 deletions src/components/views/rooms/BasicMessageComposer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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)) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this range.length > 0 is the main fix

formatRangeAsLink(range, plainText);
} else {
replaceRangeAndMoveCaret(range, parts);
}
Expand Down
12 changes: 7 additions & 5 deletions src/editor/dom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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)) {
Expand All @@ -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,
Expand Down
6 changes: 2 additions & 4 deletions src/editor/operations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
64 changes: 64 additions & 0 deletions test/components/views/rooms/BasicMessageComposer-test.tsx
Original file line number Diff line number Diff line change
@@ -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");
t3chguy marked this conversation as resolved.
Show resolved Hide resolved
});
});

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} />
));
}
5 changes: 3 additions & 2 deletions test/editor/mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
82 changes: 75 additions & 7 deletions test/editor/operations-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
t3chguy marked this conversation as resolved.
Show resolved Hide resolved
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((<DocumentPosition>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);
Expand Down