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

Commit

Permalink
Treat lists with a single empty item as plain text, not Markdown. (#6833
Browse files Browse the repository at this point in the history
)

* Treat lists with a single empty item as plain text, not Markdown.

This allows users to send simple messages such as `-`, `+`, `*` and
`2021.` without Element Web mangling them into a nonsensical empty list.
As soon as any non-whitespace content is added to the item, it switches
back to treating it as a list of one item.

Fixes vector-im/element-web#7631.

* Fix type errors.

* Lint.

---------

Co-authored-by: Michael Weimann <michaelw@matrix.org>
Co-authored-by: Johannes Marbach <johannesm@element.io>
  • Loading branch information
3 people authored Aug 17, 2023
1 parent 33ec714 commit 5aefd46
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 1 deletion.
23 changes: 22 additions & 1 deletion src/Markdown.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,10 @@ const innerNodeLiteral = (node: commonmark.Node): string => {
return literal;
};

const emptyItemWithNoSiblings = (node: commonmark.Node): boolean => {
return !node.prev && !node.next && !node.firstChild;
};

/**
* Class that wraps commonmark, adding the ability to see whether
* a given message actually uses any markdown syntax or whether
Expand Down Expand Up @@ -242,13 +246,30 @@ export default class Markdown {

public isPlainText(): boolean {
const walker = this.parsed.walker();

let ev: commonmark.NodeWalkingStep | null;

while ((ev = walker.next())) {
const node = ev.node;

if (TEXT_NODES.indexOf(node.type) > -1) {
// definitely text
continue;
} else if (node.type == "list" || node.type == "item") {
// Special handling for inputs like `+`, `*`, `-` and `2021.` which
// would otherwise be treated as a list of a single empty item.
// See https://github.com/vector-im/element-web/issues/7631
if (node.type == "list" && node.firstChild && emptyItemWithNoSiblings(node.firstChild)) {
// A list with a single empty item is treated as plain text.
continue;
}

if (node.type == "item" && emptyItemWithNoSiblings(node)) {
// An empty list item with no sibling items is treated as plain text.
continue;
}

// Everything else is actual lists and therefore not plaintext.
return false;
} else if (node.type == "html_inline" || node.type == "html_block") {
// if it's an allowed html tag, we need to render it and therefore
// we will need to use HTML. If it's not allowed, it's not HTML since
Expand Down
23 changes: 23 additions & 0 deletions test/editor/serialize-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,29 @@ describe("editor/serialize", function () {
const html = htmlSerializeIfNeeded(model, {});
expect(html).toBe("*hello* world &lt; hey world!");
});

it("lists with a single empty item are not considered markdown", function () {
const pc = createPartCreator();

const model1 = new EditorModel([pc.plain("-")], pc);
const html1 = htmlSerializeIfNeeded(model1, {});
expect(html1).toBe(undefined);

const model2 = new EditorModel([pc.plain("* ")], pc);
const html2 = htmlSerializeIfNeeded(model2, {});
expect(html2).toBe(undefined);

const model3 = new EditorModel([pc.plain("2021.")], pc);
const html3 = htmlSerializeIfNeeded(model3, {});
expect(html3).toBe(undefined);
});

it("lists with a single non-empty item are still markdown", function () {
const pc = createPartCreator();
const model = new EditorModel([pc.plain("2021. foo")], pc);
const html = htmlSerializeIfNeeded(model, {});
expect(html).toBe('<ol start="2021">\n<li>foo</li>\n</ol>\n');
});
});

describe("with plaintext", function () {
Expand Down

0 comments on commit 5aefd46

Please sign in to comment.