From 8bb08b1b75c6f38f472269fc452800eeec079c27 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 18 Jul 2018 10:10:11 +0100 Subject: [PATCH 1/8] fix focus on new editorState as it didn't have focus so broke when alt tab Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/views/rooms/MessageComposerInput.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/components/views/rooms/MessageComposerInput.js b/src/components/views/rooms/MessageComposerInput.js index 1850601d8a..c5aafaa587 100644 --- a/src/components/views/rooms/MessageComposerInput.js +++ b/src/components/views/rooms/MessageComposerInput.js @@ -330,8 +330,9 @@ export default class MessageComposerInput extends React.Component { } return editorState; } else { - // ...or create a new one. - return Plain.deserialize('', { defaultBlock: DEFAULT_NODE }); + // ...or create a new one. and explicitly focus it otherwise tab in-out issues + const base = Plain.deserialize('', { defaultBlock: DEFAULT_NODE }); + return base.change().focus().value; } } From 19e5dc579981fadb53c28f88ef4c2b771a1403e2 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 18 Jul 2018 10:10:42 +0100 Subject: [PATCH 2/8] do less rewriting for composer quote to prevent breaking pills Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/HtmlUtils.js | 213 +++++++++--------- .../views/rooms/MessageComposerInput.js | 1 + 2 files changed, 113 insertions(+), 101 deletions(-) diff --git a/src/HtmlUtils.js b/src/HtmlUtils.js index 09ce1187d5..e07c6eb72b 100644 --- a/src/HtmlUtils.js +++ b/src/HtmlUtils.js @@ -141,6 +141,99 @@ export function isUrlPermitted(inputUrl) { } } +const transformTags = { // custom to matrix + // add blank targets to all hyperlinks except vector URLs + 'a': function(tagName, attribs) { + if (attribs.href) { + attribs.target = '_blank'; // by default + + let m; + // FIXME: horrible duplication with linkify-matrix + m = attribs.href.match(linkifyMatrix.VECTOR_URL_PATTERN); + if (m) { + attribs.href = m[1]; + delete attribs.target; + } else { + m = attribs.href.match(linkifyMatrix.MATRIXTO_URL_PATTERN); + if (m) { + const entity = m[1]; + switch (entity[0]) { + case '@': + attribs.href = '#/user/' + entity; + break; + case '+': + attribs.href = '#/group/' + entity; + break; + case '#': + case '!': + attribs.href = '#/room/' + entity; + break; + } + delete attribs.target; + } + } + } + attribs.rel = 'noopener'; // https://mathiasbynens.github.io/rel-noopener/ + return { tagName, attribs }; + }, + 'img': function(tagName, attribs) { + // Strip out imgs that aren't `mxc` here instead of using allowedSchemesByTag + // because transformTags is used _before_ we filter by allowedSchemesByTag and + // we don't want to allow images with `https?` `src`s. + if (!attribs.src || !attribs.src.startsWith('mxc://')) { + return { tagName, attribs: {}}; + } + attribs.src = MatrixClientPeg.get().mxcUrlToHttp( + attribs.src, + attribs.width || 800, + attribs.height || 600, + ); + return { tagName, attribs }; + }, + 'code': function(tagName, attribs) { + if (typeof attribs.class !== 'undefined') { + // Filter out all classes other than ones starting with language- for syntax highlighting. + const classes = attribs.class.split(/\s+/).filter(function(cl) { + return cl.startsWith('language-'); + }); + attribs.class = classes.join(' '); + } + return { tagName, attribs }; + }, + '*': function(tagName, attribs) { + // Delete any style previously assigned, style is an allowedTag for font and span + // because attributes are stripped after transforming + delete attribs.style; + + // Sanitise and transform data-mx-color and data-mx-bg-color to their CSS + // equivalents + const customCSSMapper = { + 'data-mx-color': 'color', + 'data-mx-bg-color': 'background-color', + // $customAttributeKey: $cssAttributeKey + }; + + let style = ""; + Object.keys(customCSSMapper).forEach((customAttributeKey) => { + const cssAttributeKey = customCSSMapper[customAttributeKey]; + const customAttributeValue = attribs[customAttributeKey]; + if (customAttributeValue && + typeof customAttributeValue === 'string' && + COLOR_REGEX.test(customAttributeValue) + ) { + style += cssAttributeKey + ":" + customAttributeValue + ";"; + delete attribs[customAttributeKey]; + } + }); + + if (style) { + attribs.style = style; + } + + return { tagName, attribs }; + }, +}; + const sanitizeHtmlParams = { allowedTags: [ 'font', // custom to matrix for IRC-style font coloring @@ -164,102 +257,14 @@ const sanitizeHtmlParams = { allowedSchemes: PERMITTED_URL_SCHEMES, allowProtocolRelative: false, + transformTags, +}; - transformTags: { // custom to matrix - // add blank targets to all hyperlinks except vector URLs - 'a': function(tagName, attribs) { - if (attribs.href) { - attribs.target = '_blank'; // by default - - let m; - // FIXME: horrible duplication with linkify-matrix - m = attribs.href.match(linkifyMatrix.VECTOR_URL_PATTERN); - if (m) { - attribs.href = m[1]; - delete attribs.target; - } else { - m = attribs.href.match(linkifyMatrix.MATRIXTO_URL_PATTERN); - if (m) { - const entity = m[1]; - switch (entity[0]) { - case '@': - attribs.href = '#/user/' + entity; - break; - case '+': - attribs.href = '#/group/' + entity; - break; - case '#': - case '!': - attribs.href = '#/room/' + entity; - break; - } - delete attribs.target; - } - } - } - attribs.rel = 'noopener'; // https://mathiasbynens.github.io/rel-noopener/ - return { tagName: tagName, attribs: attribs }; - }, - 'img': function(tagName, attribs) { - // Strip out imgs that aren't `mxc` here instead of using allowedSchemesByTag - // because transformTags is used _before_ we filter by allowedSchemesByTag and - // we don't want to allow images with `https?` `src`s. - if (!attribs.src || !attribs.src.startsWith('mxc://')) { - return { tagName, attribs: {}}; - } - attribs.src = MatrixClientPeg.get().mxcUrlToHttp( - attribs.src, - attribs.width || 800, - attribs.height || 600, - ); - return { tagName: tagName, attribs: attribs }; - }, - 'code': function(tagName, attribs) { - if (typeof attribs.class !== 'undefined') { - // Filter out all classes other than ones starting with language- for syntax highlighting. - const classes = attribs.class.split(/\s+/).filter(function(cl) { - return cl.startsWith('language-'); - }); - attribs.class = classes.join(' '); - } - return { - tagName: tagName, - attribs: attribs, - }; - }, - '*': function(tagName, attribs) { - // Delete any style previously assigned, style is an allowedTag for font and span - // because attributes are stripped after transforming - delete attribs.style; - - // Sanitise and transform data-mx-color and data-mx-bg-color to their CSS - // equivalents - const customCSSMapper = { - 'data-mx-color': 'color', - 'data-mx-bg-color': 'background-color', - // $customAttributeKey: $cssAttributeKey - }; - - let style = ""; - Object.keys(customCSSMapper).forEach((customAttributeKey) => { - const cssAttributeKey = customCSSMapper[customAttributeKey]; - const customAttributeValue = attribs[customAttributeKey]; - if (customAttributeValue && - typeof customAttributeValue === 'string' && - COLOR_REGEX.test(customAttributeValue) - ) { - style += cssAttributeKey + ":" + customAttributeValue + ";"; - delete attribs[customAttributeKey]; - } - }); - - if (style) { - attribs.style = style; - } - - return { tagName: tagName, attribs: attribs }; - }, - }, +// this is the same as the above except with less rewriting +const composerSanitizeHtmlParams = Object.assign({}, sanitizeHtmlParams); +composerSanitizeHtmlParams.transformTags = { + 'code': transformTags['code'], + '*': transformTags['*'], }; class BaseHighlighter { @@ -385,6 +390,7 @@ class TextHighlighter extends BaseHighlighter { * opts.stripReplyFallback: optional argument specifying the event is a reply and so fallback needs removing * opts.returnString: return an HTML string rather than JSX elements * opts.emojiOne: optional param to do emojiOne (default true) + * opts.forComposerQuote: optional param to lessen the url rewriting done by sanitization, for quoting into composer */ export function bodyToHtml(content, highlights, opts={}) { const isHtmlMessage = content.format === "org.matrix.custom.html" && content.formatted_body; @@ -392,6 +398,11 @@ export function bodyToHtml(content, highlights, opts={}) { const doEmojiOne = opts.emojiOne === undefined ? true : opts.emojiOne; let bodyHasEmoji = false; + let sanitizeParams = sanitizeHtmlParams; + if (opts.forComposerQuote) { + sanitizeParams = composerSanitizeHtmlParams; + } + let strippedBody; let safeBody; let isDisplayedWithHtml; @@ -403,10 +414,10 @@ export function bodyToHtml(content, highlights, opts={}) { if (highlights && highlights.length > 0) { const highlighter = new HtmlHighlighter("mx_EventTile_searchHighlight", opts.highlightLink); const safeHighlights = highlights.map(function(highlight) { - return sanitizeHtml(highlight, sanitizeHtmlParams); + return sanitizeHtml(highlight, sanitizeParams); }); - // XXX: hacky bodge to temporarily apply a textFilter to the sanitizeHtmlParams structure. - sanitizeHtmlParams.textFilter = function(safeText) { + // XXX: hacky bodge to temporarily apply a textFilter to the sanitizeParams structure. + sanitizeParams.textFilter = function(safeText) { return highlighter.applyHighlights(safeText, safeHighlights).join(''); }; } @@ -422,13 +433,13 @@ export function bodyToHtml(content, highlights, opts={}) { // Only generate safeBody if the message was sent as org.matrix.custom.html if (isHtmlMessage) { isDisplayedWithHtml = true; - safeBody = sanitizeHtml(formattedBody, sanitizeHtmlParams); + safeBody = sanitizeHtml(formattedBody, sanitizeParams); } else { // ... or if there are emoji, which we insert as HTML alongside the // escaped plaintext body. if (bodyHasEmoji) { isDisplayedWithHtml = true; - safeBody = sanitizeHtml(escape(strippedBody), sanitizeHtmlParams); + safeBody = sanitizeHtml(escape(strippedBody), sanitizeParams); } } @@ -439,7 +450,7 @@ export function bodyToHtml(content, highlights, opts={}) { safeBody = unicodeToImage(safeBody); } } finally { - delete sanitizeHtmlParams.textFilter; + delete sanitizeParams.textFilter; } if (opts.returnString) { diff --git a/src/components/views/rooms/MessageComposerInput.js b/src/components/views/rooms/MessageComposerInput.js index c5aafaa587..4153311bc6 100644 --- a/src/components/views/rooms/MessageComposerInput.js +++ b/src/components/views/rooms/MessageComposerInput.js @@ -373,6 +373,7 @@ export default class MessageComposerInput extends React.Component { break; case 'quote': { const html = HtmlUtils.bodyToHtml(payload.event.getContent(), null, { + forComposerQuote: true, returnString: true, emojiOne: false, }); From f5856270cc260068258e45fbe135815d949c71f3 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 18 Jul 2018 10:48:54 +0100 Subject: [PATCH 3/8] undo removal of stripping

s as it breaks HTML `/me`s Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/HtmlUtils.js | 27 +++++++++++++++++++ src/Markdown.js | 8 +++--- .../views/rooms/MessageComposerInput.js | 3 +-- 3 files changed, 32 insertions(+), 6 deletions(-) diff --git a/src/HtmlUtils.js b/src/HtmlUtils.js index e07c6eb72b..b6a2bd0acb 100644 --- a/src/HtmlUtils.js +++ b/src/HtmlUtils.js @@ -112,6 +112,33 @@ export function charactersToImageNode(alt, useSvg, ...unicode) { />; } +export function processHtmlForSending(html: string): string { + const contentDiv = document.createElement('div'); + contentDiv.innerHTML = html; + + if (contentDiv.children.length === 0) { + return contentDiv.innerHTML; + } + + let contentHTML = ""; + for (let i=0; i < contentDiv.children.length; i++) { + const element = contentDiv.children[i]; + if (element.tagName.toLowerCase() === 'p') { + contentHTML += element.innerHTML; + // Don't add a
for the last

+ if (i !== contentDiv.children.length - 1) { + contentHTML += '
'; + } + } else { + const temp = document.createElement('div'); + temp.appendChild(element.cloneNode(true)); + contentHTML += temp.innerHTML; + } + } + + return contentHTML; +} + /* * Given an untrusted HTML string, return a React node with an sanitized version * of that HTML. diff --git a/src/Markdown.js b/src/Markdown.js index dc0d5962fd..acfea52100 100644 --- a/src/Markdown.js +++ b/src/Markdown.js @@ -111,7 +111,7 @@ export default class Markdown { // you can nest them. // // Let's try sending with

s anyway for now, though. -/* + const real_paragraph = renderer.paragraph; renderer.paragraph = function(node, entering) { @@ -124,10 +124,10 @@ export default class Markdown { real_paragraph.call(this, node, entering); } }; -*/ + renderer.html_inline = html_if_tag_allowed; - + renderer.html_block = function(node) { /* // as with `paragraph`, we only insert line breaks @@ -138,7 +138,7 @@ export default class Markdown { html_if_tag_allowed.call(this, node); /* if (isMultiLine) this.cr(); -*/ +*/ }; return renderer.render(this.parsed); diff --git a/src/components/views/rooms/MessageComposerInput.js b/src/components/views/rooms/MessageComposerInput.js index 4153311bc6..88b8280a4c 100644 --- a/src/components/views/rooms/MessageComposerInput.js +++ b/src/components/views/rooms/MessageComposerInput.js @@ -1089,8 +1089,7 @@ export default class MessageComposerInput extends React.Component { if (contentText === '') return true; if (shouldSendHTML) { - // FIXME: should we strip out the surrounding

? - contentHTML = this.html.serialize(editorState); // HtmlUtils.processHtmlForSending(); + contentHTML = HtmlUtils.processHtmlForSending(this.html.serialize(editorState)); } } else { const sourceWithPills = this.plainWithMdPills.serialize(editorState); From 1a9de3fabed01d54279b2b996e41aa46765c9daa Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 18 Jul 2018 10:58:27 +0100 Subject: [PATCH 4/8] fix undo on pasting plaintext content Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/views/rooms/MessageComposerInput.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/components/views/rooms/MessageComposerInput.js b/src/components/views/rooms/MessageComposerInput.js index 88b8280a4c..c20f6fb52c 100644 --- a/src/components/views/rooms/MessageComposerInput.js +++ b/src/components/views/rooms/MessageComposerInput.js @@ -986,7 +986,11 @@ export default class MessageComposerInput extends React.Component { } } case 'text': - return change.insertText(transfer.text); + // don't skip/merge so that multiple consecutive pastes can be undone individually + return change + .setOperationFlag("skip", false) + .setOperationFlag("merge", false) + .insertText(transfer.text); } }; @@ -1538,7 +1542,7 @@ export default class MessageComposerInput extends React.Component { let {placeholder} = this.props; // XXX: workaround for placeholder being shown when there is a formatting block e.g blockquote but no text - if (isEmpty && this.state.editorState.startBlock.type !== DEFAULT_NODE) { + if (isEmpty && this.state.editorState.startBlock && this.state.editorState.startBlock.type !== DEFAULT_NODE) { placeholder = undefined; } From 3e956514b3c9bd3c9a3879e065db4cd4f08fe104 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 18 Jul 2018 11:00:45 +0100 Subject: [PATCH 5/8] also prevent merge/skip on rich text pastes Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/views/rooms/MessageComposerInput.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/components/views/rooms/MessageComposerInput.js b/src/components/views/rooms/MessageComposerInput.js index c20f6fb52c..03ca16c72c 100644 --- a/src/components/views/rooms/MessageComposerInput.js +++ b/src/components/views/rooms/MessageComposerInput.js @@ -980,9 +980,15 @@ export default class MessageComposerInput extends React.Component { // that we will silently discard nested blocks (e.g. nested lists) :( const fragment = this.html.deserialize(transfer.html); if (this.state.isRichTextEnabled) { - return change.insertFragment(fragment.document); + return change + .setOperationFlag("skip", false) + .setOperationFlag("merge", false) + .insertFragment(fragment.document); } else { - return change.insertText(this.md.serialize(fragment)); + return change + .setOperationFlag("skip", false) + .setOperationFlag("merge", false) + .insertText(this.md.serialize(fragment)); } } case 'text': From 6bb88c0548474b7189069d943a0ff17f4b87b52d Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 18 Jul 2018 11:28:48 +0100 Subject: [PATCH 6/8] attempt to fix clash of Cmd-M on Mac. Should fix vector-im/riot-web#7047 Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/views/rooms/MessageComposerInput.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/components/views/rooms/MessageComposerInput.js b/src/components/views/rooms/MessageComposerInput.js index 03ca16c72c..16551f8446 100644 --- a/src/components/views/rooms/MessageComposerInput.js +++ b/src/components/views/rooms/MessageComposerInput.js @@ -734,6 +734,7 @@ export default class MessageComposerInput extends React.Component { }[ev.keyCode]; if (ctrlCmdCommand) { + ev.preventDefault(); // to prevent clashing with Mac's minimize window return this.handleKeyCommand(ctrlCmdCommand); } } From 88dddb628dcd72e895261f786ae9c000d8484346 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 18 Jul 2018 11:50:13 +0100 Subject: [PATCH 7/8] in MD mode forgo any Magic Rich Pasting conversion as its confusing Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/views/rooms/MessageComposerInput.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/components/views/rooms/MessageComposerInput.js b/src/components/views/rooms/MessageComposerInput.js index 16551f8446..af342a0b99 100644 --- a/src/components/views/rooms/MessageComposerInput.js +++ b/src/components/views/rooms/MessageComposerInput.js @@ -977,19 +977,20 @@ export default class MessageComposerInput extends React.Component { case 'files': return this.props.onFilesPasted(transfer.files); case 'html': { - // FIXME: https://github.com/ianstormtaylor/slate/issues/1497 means - // that we will silently discard nested blocks (e.g. nested lists) :( - const fragment = this.html.deserialize(transfer.html); if (this.state.isRichTextEnabled) { + // FIXME: https://github.com/ianstormtaylor/slate/issues/1497 means + // that we will silently discard nested blocks (e.g. nested lists) :( + const fragment = this.html.deserialize(transfer.html); return change .setOperationFlag("skip", false) .setOperationFlag("merge", false) .insertFragment(fragment.document); } else { + // in MD mode we don't want the rich content pasted as the magic was annoying people so paste plain return change .setOperationFlag("skip", false) .setOperationFlag("merge", false) - .insertText(this.md.serialize(fragment)); + .insertText(transfer.text); } } case 'text': From 855f8871b810d20e88b255f693a9d783ee545c25 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 18 Jul 2018 16:50:07 +0100 Subject: [PATCH 8/8] replace heuristic for the time being as it failed with inlines like pills. Fixes vector-im/riot-web#7059 Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/views/rooms/MessageComposerInput.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/components/views/rooms/MessageComposerInput.js b/src/components/views/rooms/MessageComposerInput.js index af342a0b99..a6ed136cd5 100644 --- a/src/components/views/rooms/MessageComposerInput.js +++ b/src/components/views/rooms/MessageComposerInput.js @@ -504,8 +504,9 @@ export default class MessageComposerInput extends React.Component { // when in autocomplete mode and selection changes hide the autocomplete. // Selection changes when we enter text so use a heuristic to compare documents without doing it recursively if (this.autocomplete.state.completionList.length > 0 && !this.autocomplete.state.hide && - this.state.editorState.document.text === editorState.document.text && - !rangeEquals(this.state.editorState.selection, editorState.selection)) + !rangeEquals(this.state.editorState.selection, editorState.selection) && + // XXX: the heuristic failed when inlines like pills weren't taken into account. This is inideal + this.state.editorState.document.toJSON() === editorState.document.toJSON()) { this.autocomplete.hide(); }