Merge pull request #1162 from matrix-org/luke/fix-rte-always-send-html

Only send HTML when using RTE when necessary

src/HtmlUtils.js

@@ -85,7 +85,6 @@ export function charactersToImageNode(alt, useSvg, ...unicode) {
 
 
 export function processHtmlForSending(html: string): string {
-
     const contentDiv = document.createElement('div');
     contentDiv.innerHTML = html;
 
@@ -94,10 +93,14 @@ export function processHtmlForSending(html: string): string {
     }
 
     let contentHTML = "";
-    for (let i=0; i<contentDiv.children.length; i++) {
+    for (let i=0; i < contentDiv.children.length; i++) {
         const element = contentDiv.children[i];
         if (element.tagName.toLowerCase() === 'p') {
-            contentHTML += element.innerHTML + '<br />';
+            contentHTML += element.innerHTML;
+            // Don't add a <br /> for the last <p>
+            if (i !== contentDiv.children.length - 1) {
+                contentHTML += '<br />';
+            }
         } else if (element.tagName.toLowerCase() === 'pre') {
             // Replace "<br>\n" with "\n" within `<pre>` tags because the <br> is
             // redundant. This is a workaround for a bug in draft-js-export-html:

src/components/views/rooms/MessageComposerInput.js

@@ -512,9 +512,30 @@ export default class MessageComposerInput extends React.Component {
         }
 
         if (this.state.isRichtextEnabled) {
-            contentHTML = HtmlUtils.processHtmlForSending(
+            // We should only send HTML if any block is styled or contains inline style
-                RichText.contentStateToHTML(contentState),
+            let shouldSendHTML = false;
-            );
+            const blocks = contentState.getBlocksAsArray();
+            if (blocks.some((block) => block.getType() !== 'unstyled')) {
+                shouldSendHTML = true;
+            } else {
+                const characterLists = blocks.map((block) => block.getCharacterList());
+                // For each block of characters, determine if any inline styles are applied
+                // and if yes, send HTML
+                characterLists.forEach((characters) => {
+                    const numberOfStylesForCharacters = characters.map(
+                        (character) => character.getStyle().toArray().length,
+                    ).toArray();
+                    // If any character has more than 0 inline styles applied, send HTML
+                    if (numberOfStylesForCharacters.some((styles) => styles > 0)) {
+                        shouldSendHTML = true;
+                    }
+                });
+            }
+            if (shouldSendHTML) {
+                contentHTML = HtmlUtils.processHtmlForSending(
+                    RichText.contentStateToHTML(contentState),
+                );
+            }
         } else {
             const md = new Markdown(contentText);
             if (md.isPlainText()) {
@@ -536,8 +557,8 @@ export default class MessageComposerInput extends React.Component {
         }
 
         this.historyManager.addItem(
-            this.state.isRichtextEnabled ? contentHTML : contentState.getPlainText(),
+            contentHTML ? contentHTML : contentText,
-            this.state.isRichtextEnabled ? 'html' : 'markdown');
+            contentHTML ? 'html' : 'markdown');
 
         let sendMessagePromise;
         if (contentHTML) {

test/components/views/rooms/MessageComposerInput-test.js

@@ -99,7 +99,7 @@ describe('MessageComposerInput', () => {
     });
 
     it('should not change content unnecessarily on Markdown -> RTE conversion', () => {
-        const spy = sinon.spy(client, 'sendHtmlMessage');
+        const spy = sinon.spy(client, 'sendTextMessage');
         mci.enableRichtext(false);
         addTextToDraft('a');
         mci.handleKeyCommand('toggle-mode');
@@ -109,8 +109,8 @@ describe('MessageComposerInput', () => {
         expect(spy.args[0][1]).toEqual('a');
     });
 
-    it('should send emoji messages in rich text', () => {
+    it('should send emoji messages when rich text is enabled', () => {
-        const spy = sinon.spy(client, 'sendHtmlMessage');
+        const spy = sinon.spy(client, 'sendTextMessage');
         mci.enableRichtext(true);
         addTextToDraft('☹');
         mci.handleReturn(sinon.stub());
@@ -118,7 +118,7 @@ describe('MessageComposerInput', () => {
         expect(spy.calledOnce).toEqual(true, 'should send message');
     });
 
-    it('should send emoji messages in Markdown', () => {
+    it('should send emoji messages when Markdown is enabled', () => {
         const spy = sinon.spy(client, 'sendTextMessage');
         mci.enableRichtext(false);
         addTextToDraft('☹');