Merge pull request #2070 from matrix-org/t3chguy/slate_cont2

Even More Slate Fixes :D

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 <br /> for the last <p>
+            if (i !== contentDiv.children.length - 1) {
+                contentHTML += '<br />';
+            }
+        } 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.
@@ -141,31 +168,7 @@ export function isUrlPermitted(inputUrl) {
     }
 }
 
-const sanitizeHtmlParams = {
+const transformTags = { // custom to matrix
-    allowedTags: [
-        'font', // custom to matrix for IRC-style font coloring
-        'del', // for markdown
-        'h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'blockquote', 'p', 'a', 'ul', 'ol', 'sup', 'sub',
-        'nl', 'li', 'b', 'i', 'u', 'strong', 'em', 'strike', 'code', 'hr', 'br', 'div',
-        'table', 'thead', 'caption', 'tbody', 'tr', 'th', 'td', 'pre', 'span', 'img',
-    ],
-    allowedAttributes: {
-        // custom ones first:
-        font: ['color', 'data-mx-bg-color', 'data-mx-color', 'style'], // custom to matrix
-        span: ['data-mx-bg-color', 'data-mx-color', 'style'], // custom to matrix
-        a: ['href', 'name', 'target', 'rel'], // remote target: custom to matrix
-        img: ['src', 'width', 'height', 'alt', 'title'],
-        ol: ['start'],
-        code: ['class'], // We don't actually allow all classes, we filter them in transformTags
-    },
-    // Lots of these won't come up by default because we don't allow them
-    selfClosing: ['img', 'br', 'hr', 'area', 'base', 'basefont', 'input', 'link', 'meta'],
-    // URL schemes we permit
-    allowedSchemes: PERMITTED_URL_SCHEMES,
-
-    allowProtocolRelative: false,
-
-    transformTags: { // custom to matrix
     // add blank targets to all hyperlinks except vector URLs
     'a': function(tagName, attribs) {
         if (attribs.href) {
@@ -198,7 +201,7 @@ const sanitizeHtmlParams = {
             }
         }
         attribs.rel = 'noopener'; // https://mathiasbynens.github.io/rel-noopener/
-            return { tagName: tagName, attribs: attribs };
+        return { tagName, attribs };
     },
     'img': function(tagName, attribs) {
         // Strip out imgs that aren't `mxc` here instead of using allowedSchemesByTag
@@ -212,7 +215,7 @@ const sanitizeHtmlParams = {
             attribs.width || 800,
             attribs.height || 600,
         );
-            return { tagName: tagName, attribs: attribs };
+        return { tagName, attribs };
     },
     'code': function(tagName, attribs) {
         if (typeof attribs.class !== 'undefined') {
@@ -222,10 +225,7 @@ const sanitizeHtmlParams = {
             });
             attribs.class = classes.join(' ');
         }
-            return {
+        return { tagName, attribs };
-                tagName: tagName,
-                attribs: attribs,
-            };
     },
     '*': function(tagName, attribs) {
         // Delete any style previously assigned, style is an allowedTag for font and span
@@ -257,9 +257,41 @@ const sanitizeHtmlParams = {
             attribs.style = style;
         }
 
-            return { tagName: tagName, attribs: attribs };
+        return { tagName, attribs };
     },
+};
+
+const sanitizeHtmlParams = {
+    allowedTags: [
+        'font', // custom to matrix for IRC-style font coloring
+        'del', // for markdown
+        'h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'blockquote', 'p', 'a', 'ul', 'ol', 'sup', 'sub',
+        'nl', 'li', 'b', 'i', 'u', 'strong', 'em', 'strike', 'code', 'hr', 'br', 'div',
+        'table', 'thead', 'caption', 'tbody', 'tr', 'th', 'td', 'pre', 'span', 'img',
+    ],
+    allowedAttributes: {
+        // custom ones first:
+        font: ['color', 'data-mx-bg-color', 'data-mx-color', 'style'], // custom to matrix
+        span: ['data-mx-bg-color', 'data-mx-color', 'style'], // custom to matrix
+        a: ['href', 'name', 'target', 'rel'], // remote target: custom to matrix
+        img: ['src', 'width', 'height', 'alt', 'title'],
+        ol: ['start'],
+        code: ['class'], // We don't actually allow all classes, we filter them in transformTags
     },
+    // Lots of these won't come up by default because we don't allow them
+    selfClosing: ['img', 'br', 'hr', 'area', 'base', 'basefont', 'input', 'link', 'meta'],
+    // URL schemes we permit
+    allowedSchemes: PERMITTED_URL_SCHEMES,
+
+    allowProtocolRelative: false,
+    transformTags,
+};
+
+// 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 +417,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 +425,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 +441,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.
+            // XXX: hacky bodge to temporarily apply a textFilter to the sanitizeParams structure.
-            sanitizeHtmlParams.textFilter = function(safeText) {
+            sanitizeParams.textFilter = function(safeText) {
                 return highlighter.applyHighlights(safeText, safeHighlights).join('');
             };
         }
@@ -422,13 +460,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 +477,7 @@ export function bodyToHtml(content, highlights, opts={}) {
             safeBody = unicodeToImage(safeBody);
         }
     } finally {
-        delete sanitizeHtmlParams.textFilter;
+        delete sanitizeParams.textFilter;
     }
 
     if (opts.returnString) {

src/Markdown.js

@@ -111,7 +111,7 @@ export default class Markdown {
         // you can nest them.
         //
         // Let's try sending with <p/>s anyway for now, though.
-/*        
+
         const real_paragraph = renderer.paragraph;
 
         renderer.paragraph = function(node, entering) {
@@ -124,7 +124,7 @@ export default class Markdown {
                 real_paragraph.call(this, node, entering);
             }
         };
-*/        
+
 
         renderer.html_inline = html_if_tag_allowed;
 

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.
+            // ...or create a new one. and explicitly focus it otherwise tab in-out issues
-            return Plain.deserialize('', { defaultBlock: DEFAULT_NODE });
+            const base = Plain.deserialize('', { defaultBlock: DEFAULT_NODE });
+            return base.change().focus().value;
         }
     }
 
@@ -372,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,
                 });
@@ -502,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();
         }
@@ -732,6 +735,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);
             }
         }
@@ -974,17 +978,28 @@ export default class MessageComposerInput extends React.Component {
             case 'files':
                 return this.props.onFilesPasted(transfer.files);
             case '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);
-                if (this.state.isRichTextEnabled) {
+                    return change
-                    return change.insertFragment(fragment.document);
+                        .setOperationFlag("skip", false)
+                        .setOperationFlag("merge", false)
+                        .insertFragment(fragment.document);
                 } else {
-                    return change.insertText(this.md.serialize(fragment));
+                    // 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(transfer.text);
                 }
             }
             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);
         }
     };
 
@@ -1087,8 +1102,7 @@ export default class MessageComposerInput extends React.Component {
             if (contentText === '') return true;
 
             if (shouldSendHTML) {
-                // FIXME: should we strip out the surrounding <p></p>?
+                contentHTML = HtmlUtils.processHtmlForSending(this.html.serialize(editorState));
-                contentHTML = this.html.serialize(editorState); // HtmlUtils.processHtmlForSending();
             }
         } else {
             const sourceWithPills = this.plainWithMdPills.serialize(editorState);
@@ -1537,7 +1551,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;
         }