From 893a5c971fd43186637f50cb3725eefa09b6d365 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20L=C3=B6thberg?= Date: Fri, 2 Dec 2016 19:58:35 +0100 Subject: [PATCH 1/9] Fix escaping markdown by rendering plaintext MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We still need to parse "plaintext" messages through the markdown renderer so that escappes are rendered properly. Fixes vector-im/riot-web#2870. Signed-off-by: Johannes Löthberg --- src/Markdown.js | 32 ++++++++++++------- .../views/rooms/MessageComposerInput.js | 8 +++-- 2 files changed, 25 insertions(+), 15 deletions(-) diff --git a/src/Markdown.js b/src/Markdown.js index 18c888b5414..2eb84b9041d 100644 --- a/src/Markdown.js +++ b/src/Markdown.js @@ -56,23 +56,31 @@ export default class Markdown { return is_plain; } - toHTML() { + render(html) { const parser = new commonmark.Parser(); const renderer = new commonmark.HtmlRenderer({safe: true}); const real_paragraph = renderer.paragraph; - renderer.paragraph = function(node, entering) { - // If there is only one top level node, just return the - // bare text: it's a single line of text and so should be - // 'inline', rather than unnecessarily wrapped in its own - // p tag. If, however, we have multiple nodes, each gets - // its own p tag to keep them as separate paragraphs. - var par = node; - while (par.parent) { - par = par.parent + if (html) { + renderer.paragraph = function(node, entering) { + // If there is only one top level node, just return the + // bare text: it's a single line of text and so should be + // 'inline', rather than unnecessarily wrapped in its own + // p tag. If, however, we have multiple nodes, each gets + // its own p tag to keep them as separate paragraphs. + var par = node; + while (par.parent) { + par = par.parent + } + if (par.firstChild != par.lastChild) { + real_paragraph.call(this, node, entering); + } } - if (par.firstChild != par.lastChild) { - real_paragraph.call(this, node, entering); + } else { + renderer.paragraph = function(node, entering) { + if (entering) { + this.lit('\n\n'); + } } } diff --git a/src/components/views/rooms/MessageComposerInput.js b/src/components/views/rooms/MessageComposerInput.js index 37d937d6f52..5e8df592da3 100644 --- a/src/components/views/rooms/MessageComposerInput.js +++ b/src/components/views/rooms/MessageComposerInput.js @@ -401,7 +401,7 @@ export default class MessageComposerInput extends React.Component { let contentState = null; if (enabled) { const md = new Markdown(this.state.editorState.getCurrentContent().getPlainText()); - contentState = RichText.HTMLtoContentState(md.toHTML()); + contentState = RichText.HTMLtoContentState(md.render(true)); } else { let markdown = stateToMarkdown(this.state.editorState.getCurrentContent()); if (markdown[markdown.length - 1] === '\n') { @@ -523,8 +523,10 @@ export default class MessageComposerInput extends React.Component { ); } else { const md = new Markdown(contentText); - if (!md.isPlainText()) { - contentHTML = md.toHTML(); + if (md.isPlainText()) { + contentText = md.render(false); + } else { + contentHTML = md.render(true); } } From 35d70f0b35aeefa4af43379078b63922f0f70afe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20L=C3=B6thberg?= Date: Tue, 17 Jan 2017 20:32:06 +0100 Subject: [PATCH 2/9] markdown: Only add \n\n on multiple paragraphs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Johannes Löthberg --- src/Markdown.js | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/Markdown.js b/src/Markdown.js index 2eb84b9041d..17723e42f89 100644 --- a/src/Markdown.js +++ b/src/Markdown.js @@ -78,8 +78,20 @@ export default class Markdown { } } else { renderer.paragraph = function(node, entering) { - if (entering) { - this.lit('\n\n'); + // If there is only one top level node, just return the + // bare text: it's a single line of text and so should be + // 'inline', rather than unnecessarily wrapped in its own + // p tag. If, however, we have multiple nodes, each gets + // its own p tag to keep them as separate paragraphs. + var par = node; + while (par.parent) { + node = par; + par = par.parent; + } + if (node != par.lastChild) { + if (!entering) { + this.lit('\n\n'); + } } } } From c819b433a247ca502daf600c3bc763f138262f5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20L=C3=B6thberg?= Date: Tue, 17 Jan 2017 20:37:27 +0100 Subject: [PATCH 3/9] Make old message composer use new markdown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Johannes Löthberg --- src/components/views/rooms/MessageComposerInputOld.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/components/views/rooms/MessageComposerInputOld.js b/src/components/views/rooms/MessageComposerInputOld.js index 28e3186c50c..3b0100278b2 100644 --- a/src/components/views/rooms/MessageComposerInputOld.js +++ b/src/components/views/rooms/MessageComposerInputOld.js @@ -325,12 +325,13 @@ module.exports = React.createClass({ } if (send_markdown) { - const htmlText = mdown.toHTML(); + const htmlText = mdown.render(true); sendMessagePromise = isEmote ? MatrixClientPeg.get().sendHtmlEmote(this.props.room.roomId, contentText, htmlText) : MatrixClientPeg.get().sendHtmlMessage(this.props.room.roomId, contentText, htmlText); } else { + const contentText = mdown.render(false); sendMessagePromise = isEmote ? MatrixClientPeg.get().sendEmoteMessage(this.props.room.roomId, contentText) : MatrixClientPeg.get().sendTextMessage(this.props.room.roomId, contentText); From 49d60ff879aa677e5befb10c831b0d89e9952d0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20L=C3=B6thberg?= Date: Tue, 17 Jan 2017 21:04:12 +0100 Subject: [PATCH 4/9] Markdown: softbreak is not HTML MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Johannes Löthberg --- src/Markdown.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Markdown.js b/src/Markdown.js index 17723e42f89..ad7ec5ef0cd 100644 --- a/src/Markdown.js +++ b/src/Markdown.js @@ -48,6 +48,7 @@ export default class Markdown { } // text and paragraph are just text dummy_renderer.text = function(t) { return t; } + dummy_renderer.softbreak = function(t) { return t; } dummy_renderer.paragraph = function(t) { return t; } const dummy_parser = new commonmark.Parser(); From 2e3bdcf5c697009b0d7b6b82ab5f902ec49115de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20L=C3=B6thberg?= Date: Tue, 17 Jan 2017 22:20:05 +0100 Subject: [PATCH 5/9] Markdown: Don't XML escape the output when not HTML MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Johannes Löthberg --- src/Markdown.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Markdown.js b/src/Markdown.js index ad7ec5ef0cd..e6f5f59f018 100644 --- a/src/Markdown.js +++ b/src/Markdown.js @@ -78,6 +78,10 @@ export default class Markdown { } } } else { + renderer.out = function(s) { + this.lit(s); + } + renderer.paragraph = function(node, entering) { // If there is only one top level node, just return the // bare text: it's a single line of text and so should be From 6d2e521421d03032a3e7c5001bec42545c469512 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20L=C3=B6thberg?= Date: Wed, 18 Jan 2017 14:25:11 +0100 Subject: [PATCH 6/9] Markdown: Add comment about out function override MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Johannes Löthberg --- src/Markdown.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Markdown.js b/src/Markdown.js index e6f5f59f018..35ae42f770e 100644 --- a/src/Markdown.js +++ b/src/Markdown.js @@ -78,6 +78,9 @@ export default class Markdown { } } } else { + // The default `out` function only sends the input through an XML + // escaping function, which causes messages to be entity encoded, + // which we don't want in this case. renderer.out = function(s) { this.lit(s); } From 30bd01cdf2a2548cf536c7edc72a8950d1238175 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20L=C3=B6thberg?= Date: Wed, 18 Jan 2017 19:29:11 +0100 Subject: [PATCH 7/9] Markdown: Split up render function into toHTML/toPlaintext MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Johannes Löthberg --- src/Markdown.js | 100 ++++++++++-------- .../views/rooms/MessageComposerInput.js | 6 +- .../views/rooms/MessageComposerInputOld.js | 4 +- 3 files changed, 62 insertions(+), 48 deletions(-) diff --git a/src/Markdown.js b/src/Markdown.js index 35ae42f770e..80d1aa43356 100644 --- a/src/Markdown.js +++ b/src/Markdown.js @@ -23,7 +23,9 @@ import commonmark from 'commonmark'; */ export default class Markdown { constructor(input) { - this.input = input + this.input = input; + this.parser = new commonmark.Parser(); + this.renderer = new commonmark.HtmlRenderer({safe: false}); } isPlainText() { @@ -57,54 +59,66 @@ export default class Markdown { return is_plain; } - render(html) { - const parser = new commonmark.Parser(); - - const renderer = new commonmark.HtmlRenderer({safe: true}); - const real_paragraph = renderer.paragraph; - if (html) { - renderer.paragraph = function(node, entering) { - // If there is only one top level node, just return the - // bare text: it's a single line of text and so should be - // 'inline', rather than unnecessarily wrapped in its own - // p tag. If, however, we have multiple nodes, each gets - // its own p tag to keep them as separate paragraphs. - var par = node; - while (par.parent) { - par = par.parent - } - if (par.firstChild != par.lastChild) { - real_paragraph.call(this, node, entering); - } + toHTML(html) { + const real_paragraph = this.renderer.paragraph; + + this.renderer.paragraph = function(node, entering) { + // If there is only one top level node, just return the + // bare text: it's a single line of text and so should be + // 'inline', rather than unnecessarily wrapped in its own + // p tag. If, however, we have multiple nodes, each gets + // its own p tag to keep them as separate paragraphs. + var par = node; + while (par.parent) { + par = par.parent } - } else { - // The default `out` function only sends the input through an XML - // escaping function, which causes messages to be entity encoded, - // which we don't want in this case. - renderer.out = function(s) { - this.lit(s); + if (par.firstChild != par.lastChild) { + real_paragraph.call(this, node, entering); } + } - renderer.paragraph = function(node, entering) { - // If there is only one top level node, just return the - // bare text: it's a single line of text and so should be - // 'inline', rather than unnecessarily wrapped in its own - // p tag. If, however, we have multiple nodes, each gets - // its own p tag to keep them as separate paragraphs. - var par = node; - while (par.parent) { - node = par; - par = par.parent; - } - if (node != par.lastChild) { - if (!entering) { - this.lit('\n\n'); - } + var parsed = this.parser.parse(this.input); + var rendered = this.renderer.render(parsed); + + this.renderer.paragraph = real_paragraph; + + return rendered; + } + + toPlaintext() { + const real_paragraph = this.renderer.paragraph; + + // The default `out` function only sends the input through an XML + // escaping function, which causes messages to be entity encoded, + // which we don't want in this case. + this.renderer.out = function(s) { + // The `lit` function adds a string literal to the output buffer. + this.lit(s); + } + + this.renderer.paragraph = function(node, entering) { + // If there is only one top level node, just return the + // bare text: it's a single line of text and so should be + // 'inline', rather than unnecessarily wrapped in its own + // p tag. If, however, we have multiple nodes, each gets + // its own p tag to keep them as separate paragraphs. + var par = node; + while (par.parent) { + node = par; + par = par.parent; + } + if (node != par.lastChild) { + if (!entering) { + this.lit('\n\n'); } } } - var parsed = parser.parse(this.input); - return renderer.render(parsed); + var parsed = this.parser.parse(this.input); + var rendered = this.renderer.render(parsed); + + this.renderer.paragraph = real_paragraph; + + return rendered; } } diff --git a/src/components/views/rooms/MessageComposerInput.js b/src/components/views/rooms/MessageComposerInput.js index 5e8df592da3..41b27c13949 100644 --- a/src/components/views/rooms/MessageComposerInput.js +++ b/src/components/views/rooms/MessageComposerInput.js @@ -401,7 +401,7 @@ export default class MessageComposerInput extends React.Component { let contentState = null; if (enabled) { const md = new Markdown(this.state.editorState.getCurrentContent().getPlainText()); - contentState = RichText.HTMLtoContentState(md.render(true)); + contentState = RichText.HTMLtoContentState(md.toHTML()); } else { let markdown = stateToMarkdown(this.state.editorState.getCurrentContent()); if (markdown[markdown.length - 1] === '\n') { @@ -524,9 +524,9 @@ export default class MessageComposerInput extends React.Component { } else { const md = new Markdown(contentText); if (md.isPlainText()) { - contentText = md.render(false); + contentText = md.toPlaintext(); } else { - contentHTML = md.render(true); + contentHTML = md.toHTML(true); } } diff --git a/src/components/views/rooms/MessageComposerInputOld.js b/src/components/views/rooms/MessageComposerInputOld.js index 3b0100278b2..91abd5a2a8f 100644 --- a/src/components/views/rooms/MessageComposerInputOld.js +++ b/src/components/views/rooms/MessageComposerInputOld.js @@ -325,13 +325,13 @@ module.exports = React.createClass({ } if (send_markdown) { - const htmlText = mdown.render(true); + const htmlText = mdown.toHTML(); sendMessagePromise = isEmote ? MatrixClientPeg.get().sendHtmlEmote(this.props.room.roomId, contentText, htmlText) : MatrixClientPeg.get().sendHtmlMessage(this.props.room.roomId, contentText, htmlText); } else { - const contentText = mdown.render(false); + const contentText = mdown.toPlaintext(false); sendMessagePromise = isEmote ? MatrixClientPeg.get().sendEmoteMessage(this.props.room.roomId, contentText) : MatrixClientPeg.get().sendTextMessage(this.props.room.roomId, contentText); From 14ead373e2e8c6d1ed612000ddbee668b84f8989 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20L=C3=B6thberg?= Date: Wed, 18 Jan 2017 20:54:34 +0100 Subject: [PATCH 8/9] Add markdown test-cases MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Johannes Löthberg --- .../views/rooms/MessageComposerInput-test.js | 81 +++++++++++++++++++ 1 file changed, 81 insertions(+) diff --git a/test/components/views/rooms/MessageComposerInput-test.js b/test/components/views/rooms/MessageComposerInput-test.js index 8d33e0ead30..ca2bbba2ebb 100644 --- a/test/components/views/rooms/MessageComposerInput-test.js +++ b/test/components/views/rooms/MessageComposerInput-test.js @@ -158,4 +158,85 @@ describe('MessageComposerInput', () => { expect(['__', '**']).toContain(spy.args[0][1]); }); + it('should not entity-encode " in Markdown mode', () => { + const spy = sinon.spy(client, 'sendTextMessage'); + mci.enableRichtext(false); + addTextToDraft('"'); + mci.handleReturn(sinon.stub()); + + expect(spy.calledOnce).toEqual(true); + expect(spy.args[0][1]).toEqual('"'); + }); + + it('should escape characters without other markup in Markdown mode', () => { + const spy = sinon.spy(client, 'sendTextMessage'); + mci.enableRichtext(false); + addTextToDraft('\\*escaped\\*'); + mci.handleReturn(sinon.stub()); + + expect(spy.calledOnce).toEqual(true); + expect(spy.args[0][1]).toEqual('*escaped*'); + }); + + it('should escape characters with other markup in Markdown mode', () => { + const spy = sinon.spy(client, 'sendHtmlMessage'); + mci.enableRichtext(false); + addTextToDraft('\\*escaped\\* *italic*'); + mci.handleReturn(sinon.stub()); + + expect(spy.calledOnce).toEqual(true); + expect(spy.args[0][1]).toEqual('\\*escaped\\* *italic*'); + expect(spy.args[0][2]).toEqual('*escaped* italic'); + }); + + it('should not convert -_- into a horizontal rule in Markdown mode', () => { + const spy = sinon.spy(client, 'sendTextMessage'); + mci.enableRichtext(false); + addTextToDraft('-_-'); + mci.handleReturn(sinon.stub()); + + expect(spy.calledOnce).toEqual(true); + expect(spy.args[0][1]).toEqual('-_-'); + }); + + it('should not strip tags in Markdown mode', () => { + const spy = sinon.spy(client, 'sendHtmlMessage'); + mci.enableRichtext(false); + addTextToDraft('striked-out'); + mci.handleReturn(sinon.stub()); + + expect(spy.calledOnce).toEqual(true); + expect(spy.args[0][1]).toEqual('striked-out'); + expect(spy.args[0][2]).toEqual('striked-out'); + }); + + it('should not strike-through ~~~ in Markdown mode', () => { + const spy = sinon.spy(client, 'sendTextMessage'); + mci.enableRichtext(false); + addTextToDraft('~~~striked-out~~~'); + mci.handleReturn(sinon.stub()); + + expect(spy.calledOnce).toEqual(true); + expect(spy.args[0][1]).toEqual('~~~striked-out~~~'); + }); + + it('should not mark single unmarkedup paragraphs as HTML in Markdown mode', () => { + const spy = sinon.spy(client, 'sendTextMessage'); + mci.enableRichtext(false); + addTextToDraft('Lorem ipsum dolor sit amet, consectetur adipiscing elit.'); + mci.handleReturn(sinon.stub()); + + expect(spy.calledOnce).toEqual(true); + expect(spy.args[0][1]).toEqual('Lorem ipsum dolor sit amet, consectetur adipiscing elit.'); + }); + + it('should not mark two unmarkedup paragraphs as HTML in Markdown mode', () => { + const spy = sinon.spy(client, 'sendTextMessage'); + mci.enableRichtext(false); + addTextToDraft('Lorem ipsum dolor sit amet, consectetur adipiscing elit.\n\nFusce congue sapien sed neque molestie volutpat.'); + mci.handleReturn(sinon.stub()); + + expect(spy.calledOnce).toEqual(true); + expect(spy.args[0][1]).toEqual('Lorem ipsum dolor sit amet, consectetur adipiscing elit.\n\nFusce congue sapien sed neque molestie volutpat.'); + }); }); From 9c1c657a1edf157ec6039bf12b0f8cc848bca5f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20L=C3=B6thberg?= Date: Thu, 19 Jan 2017 11:55:36 +0100 Subject: [PATCH 9/9] Markdown: delete remaining pre-split relics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Johannes Löthberg --- src/Markdown.js | 2 +- src/components/views/rooms/MessageComposerInput.js | 2 +- src/components/views/rooms/MessageComposerInputOld.js | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Markdown.js b/src/Markdown.js index 80d1aa43356..3506e3cb591 100644 --- a/src/Markdown.js +++ b/src/Markdown.js @@ -59,7 +59,7 @@ export default class Markdown { return is_plain; } - toHTML(html) { + toHTML() { const real_paragraph = this.renderer.paragraph; this.renderer.paragraph = function(node, entering) { diff --git a/src/components/views/rooms/MessageComposerInput.js b/src/components/views/rooms/MessageComposerInput.js index 41b27c13949..b6af5a9f091 100644 --- a/src/components/views/rooms/MessageComposerInput.js +++ b/src/components/views/rooms/MessageComposerInput.js @@ -526,7 +526,7 @@ export default class MessageComposerInput extends React.Component { if (md.isPlainText()) { contentText = md.toPlaintext(); } else { - contentHTML = md.toHTML(true); + contentHTML = md.toHTML(); } } diff --git a/src/components/views/rooms/MessageComposerInputOld.js b/src/components/views/rooms/MessageComposerInputOld.js index 91abd5a2a8f..ed4533737f8 100644 --- a/src/components/views/rooms/MessageComposerInputOld.js +++ b/src/components/views/rooms/MessageComposerInputOld.js @@ -331,7 +331,7 @@ module.exports = React.createClass({ MatrixClientPeg.get().sendHtmlMessage(this.props.room.roomId, contentText, htmlText); } else { - const contentText = mdown.toPlaintext(false); + const contentText = mdown.toPlaintext(); sendMessagePromise = isEmote ? MatrixClientPeg.get().sendEmoteMessage(this.props.room.roomId, contentText) : MatrixClientPeg.get().sendTextMessage(this.props.room.roomId, contentText);