correctly send pills in messages

src/Markdown.js

@@ -133,7 +133,10 @@ export default class Markdown {
      * Render the markdown message to plain text. That is, essentially
      * just remove any backslashes escaping what would otherwise be
      * markdown syntax
-     * (to fix https://github.com/vector-im/riot-web/issues/2870)
+     * (to fix https://github.com/vector-im/riot-web/issues/2870).
+     *
+     * N.B. this does **NOT** render arbitrary MD to plain text - only MD
+     * which has no formatting.  Otherwise it emits HTML(!).
      */
     toPlaintext() {
         const renderer = new commonmark.HtmlRenderer({safe: false});
@@ -161,6 +164,14 @@ export default class Markdown {
             if (is_multi_line(node) && node.next) this.lit('\n\n');
         };
 
+        // convert MD links into console-friendly ' < http://foo >' style links
+        // ...except given this function never gets called with links, it's useless.
+        // renderer.link = function(node, entering) {
+        //     if (!entering) {
+        //         this.lit(` < ${node.destination} >`);
+        //     }
+        // };
+
         return renderer.render(this.parsed);
     }
 }

src/autocomplete/NotifProvider.js

@@ -40,6 +40,7 @@ export default class NotifProvider extends AutocompleteProvider {
         if (command && command[0] && '@room'.startsWith(command[0]) && command[0].length > 1) {
             return [{
                 completion: '@room',
+                completionId: '@room',
                 suffix: ' ',
                 component: (
                     <PillCompletion initialComponent={<RoomAvatar width={24} height={24} room={this.room} />} title="@room" description={_t("Notify the whole room")} />

src/autocomplete/PlainWithPillsSerializer.js

@@ -0,0 +1,89 @@
+/*
+Copyright 2018 New Vector Ltd
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+// Based originally on slate-plain-serializer
+
+import { Block } from 'slate';
+
+/**
+ * Plain text serializer, which converts a Slate `value` to a plain text string,
+ * serializing pills into various different formats as required.
+ *
+ * @type {PlainWithPillsSerializer}
+ */
+
+class PlainWithPillsSerializer {
+
+    /*
+     * @param {String} options.pillFormat - either 'md', 'plain', 'id'
+     */
+    constructor(options = {}) {
+        let {
+            pillFormat = 'plain',
+        } = options;
+        this.pillFormat = pillFormat;
+    }
+
+    /**
+     * Serialize a Slate `value` to a plain text string,
+     * serializing pills as either MD links, plain text representations or
+     * ID representations as required.
+     *
+     * @param {Value} value
+     * @return {String}
+     */
+    serialize = value => {
+        return this._serializeNode(value.document)
+    }
+
+    /**
+     * Serialize a `node` to plain text.
+     *
+     * @param {Node} node
+     * @return {String}
+     */
+    _serializeNode = node => {
+        if (
+            node.object == 'document' ||
+            (node.object == 'block' && Block.isBlockList(node.nodes))
+        ) {
+            return node.nodes.map(this._serializeNode).join('\n');
+        } else if (node.type == 'pill') {
+            switch (this.pillFormat) {
+                case 'plain':
+                    return node.text;
+                case 'md':
+                    return `[${ node.text }](${ node.data.get('url') })`;
+                case 'id':
+                    return node.data.completionId || node.text;
+            }
+        }
+        else if (node.nodes) {
+            return node.nodes.map(this._serializeNode).join('');
+        }
+        else {
+            return node.text;
+        }
+    }
+}
+
+/**
+ * Export.
+ *
+ * @type {PlainWithPillsSerializer}
+ */
+
+export default PlainWithPillsSerializer

src/autocomplete/RoomProvider.js

@@ -78,6 +78,7 @@ export default class RoomProvider extends AutocompleteProvider {
                 const displayAlias = getDisplayAliasForRoom(room.room) || room.roomId;
                 return {
                     completion: displayAlias,
+                    completionId: displayAlias,
                     suffix: ' ',
                     href: makeRoomPermalink(displayAlias),
                     component: (

src/autocomplete/UserProvider.js

@@ -113,6 +113,7 @@ export default class UserProvider extends AutocompleteProvider {
                     // Length of completion should equal length of text in decorator. draft-js
                     // relies on the length of the entity === length of the text in the decoration.
                     completion: user.rawDisplayName.replace(' (IRC)', ''),
+                    completionId: user.userId,
                     suffix: range.start === 0 ? ': ' : ' ',
                     href: makeUserPermalink(user.userId),
                     component: (

src/components/views/rooms/Autocomplete.js

@@ -263,7 +263,6 @@ export default class Autocomplete extends React.Component {
                 const componentPosition = position;
                 position++;
 
-                const onMouseMove = () => this.setSelection(componentPosition);
                 const onClick = () => {
                     this.setSelection(componentPosition);
                     this.onCompletionClicked();
@@ -273,7 +272,6 @@ export default class Autocomplete extends React.Component {
                     key: i,
                     ref: `completion${position - 1}`,
                     className,
-                    onMouseMove,
                     onClick,
                 });
             });

src/components/views/rooms/MessageComposerInput.js

@@ -25,6 +25,7 @@ import { Value, Document, Event, Inline, Text, Range, Node } from 'slate';
 import Html from 'slate-html-serializer';
 import { Markdown as Md } from 'slate-md-serializer';
 import Plain from 'slate-plain-serializer';
+import PlainWithPillsSerializer from "../../../autocomplete/PlainWithPillsSerializer";
 
 // import {Editor, EditorState, RichUtils, CompositeDecorator, Modifier,
 //     getDefaultKeyBinding, KeyBindingUtil, ContentState, ContentBlock, SelectionState,
@@ -157,7 +158,7 @@ export default class MessageComposerInput extends React.Component {
             // the currently displayed editor state (note: this is always what is modified on input)
             editorState: this.createEditorState(
                 isRichtextEnabled,
-                MessageComposerStore.getContentState(this.props.room.roomId),
+                MessageComposerStore.getEditorState(this.props.room.roomId),
             ),
 
             // the original editor state, before we started tabbing through completions
@@ -172,6 +173,10 @@ export default class MessageComposerInput extends React.Component {
         };
 
         this.client = MatrixClientPeg.get();
+
+        this.plainWithMdPills    = new PlainWithPillsSerializer({ pillFormat: 'md' });
+        this.plainWithIdPills    = new PlainWithPillsSerializer({ pillFormat: 'id' });
+        this.plainWithPlainPills = new PlainWithPillsSerializer({ pillFormat: 'plain' });
     }
 
 /*
@@ -686,30 +691,27 @@ export default class MessageComposerInput extends React.Component {
             return false;
         }
 */
-        const contentState = this.state.editorState;
+        const editorState = this.state.editorState;
 
-        let contentText = Plain.serialize(contentState);
+        let contentText;
         let contentHTML;
 
-        if (contentText === '') return true;
+        // only look for commands if the first block contains simple unformatted text
+        // i.e. no pills or rich-text formatting.
+        let cmd, commandText;
+        const firstChild = editorState.document.nodes.get(0);
+        const firstGrandChild = firstChild && firstChild.nodes.get(0);
+        if (firstChild && firstGrandChild &&
+            firstChild.object === 'block' && firstGrandChild.object === 'text' &&
+            firstGrandChild.text[0] === '/' && firstGrandChild.text[1] !== '/')
+        {
+            commandText = this.plainWithIdPills.serialize(editorState);
+            cmd = SlashCommands.processInput(this.props.room.roomId, commandText);
+        }
 
-/*
-        // Strip MD user (tab-completed) mentions to preserve plaintext mention behaviour.
-        // We have to do this now as opposed to after calculating the contentText for MD
-        // mode because entity positions may not be maintained when using
-        // md.toPlaintext().
-        // Unfortunately this means we lose mentions in history when in MD mode. This
-        // would be fixed if history was stored as contentState.
-        contentText = this.removeMDLinks(contentState, ['@']);
-
-        // Some commands (/join) require pills to be replaced with their text content
-        const commandText = this.removeMDLinks(contentState, ['#']);
-*/
-        const commandText = contentText;
-        const cmd = SlashCommands.processInput(this.props.room.roomId, commandText);
         if (cmd) {
             if (!cmd.error) {
-                this.historyManager.save(contentState, this.state.isRichtextEnabled ? 'rich' : 'markdown');
+                this.historyManager.save(editorState, this.state.isRichtextEnabled ? 'rich' : 'markdown');
                 this.setState({
                     editorState: this.createEditorState(),
                 });
@@ -774,46 +776,31 @@ export default class MessageComposerInput extends React.Component {
                 shouldSendHTML = hasLink;
             }
 */
+            contentText = this.plainWithPlainPills.serialize(editorState);
+            if (contentText === '') return true;
+
             let shouldSendHTML = true;
             if (shouldSendHTML) {
                 contentHTML = HtmlUtils.processHtmlForSending(
-                    RichText.editorStateToHTML(contentState),
+                    RichText.editorStateToHTML(editorState),
                 );
             }
         } else {
+            const sourceWithPills = this.plainWithMdPills.serialize(editorState);
+            if (sourceWithPills === '') return true;
 
-            // Use the original contentState because `contentText` has had mentions
-            // stripped and these need to end up in contentHTML.
+            const mdWithPills = new Markdown(sourceWithPills);
 
-/*
-            // Replace all Entities of type `LINK` with markdown link equivalents.
-            // TODO: move this into `Markdown` and do the same conversion in the other
-            // two places (toggling from MD->RT mode and loading MD history into RT mode)
-            // but this can only be done when history includes Entities.
-            const pt = contentState.getBlocksAsArray().map((block) => {
-                let blockText = block.getText();
-                let offset = 0;
-                this.findPillEntities(contentState, block, (start, end) => {
-                    const entity = contentState.getEntity(block.getEntityAt(start));
-                    if (entity.getType() !== 'LINK') {
-                        return;
-                    }
-                    const text = blockText.slice(offset + start, offset + end);
-                    const url = entity.getData().url;
-                    const mdLink = `[${text}](${url})`;
-                    blockText = blockText.slice(0, offset + start) + mdLink + blockText.slice(offset + end);
-                    offset += mdLink.length - text.length;
-                });
-                return blockText;
-            }).join('\n');
-*/
-            const md = new Markdown(contentText);
             // if contains no HTML and we're not quoting (needing HTML)
-            if (md.isPlainText() && !mustSendHTML) {
-                contentText = md.toPlaintext();
+            if (mdWithPills.isPlainText() && !mustSendHTML) {
+                // N.B. toPlainText is only usable here because we know that the MD
+                // didn't contain any formatting in the first place...
+                contentText = mdWithPills.toPlaintext();
             } else {
-                contentText = md.toPlaintext();
-                contentHTML = md.toHTML();
+                // to avoid ugliness clients which can't parse HTML we don't send pills
+                // in the plaintext body.
+                contentText = this.plainWithPlainPills.serialize(editorState);
+                contentHTML = mdWithPills.toHTML();
             }
         }
 
@@ -821,11 +808,11 @@ export default class MessageComposerInput extends React.Component {
         let sendTextFn = ContentHelpers.makeTextMessage;
 
         this.historyManager.save(
-            contentState,
+            editorState,
             this.state.isRichtextEnabled ? 'rich' : 'markdown',
         );
 
-        if (contentText.startsWith('/me')) {
+        if (commandText && commandText.startsWith('/me')) {
             if (replyingToEv) {
                 const ErrorDialog = sdk.getComponent("dialogs.ErrorDialog");
                 Modal.createTrackedDialog('Emote Reply Fail', '', ErrorDialog, {
@@ -842,14 +829,16 @@ export default class MessageComposerInput extends React.Component {
             sendTextFn = ContentHelpers.makeEmoteMessage;
         }
 
-
-        let content = contentHTML ? sendHtmlFn(contentText, contentHTML) : sendTextFn(contentText);
+        let content = contentHTML ?
+                      sendHtmlFn(contentText, contentHTML) :
+                      sendTextFn(contentText);
 
         if (replyingToEv) {
             const replyContent = ReplyThread.makeReplyMixIn(replyingToEv);
             content = Object.assign(replyContent, content);
 
-            // Part of Replies fallback support - prepend the text we're sending with the text we're replying to
+            // Part of Replies fallback support - prepend the text we're sending
+            // with the text we're replying to
             const nestedReply = ReplyThread.getNestedReplyText(replyingToEv);
             if (nestedReply) {
                 if (content.formatted_body) {
@@ -1009,20 +998,26 @@ export default class MessageComposerInput extends React.Component {
             return false;
         }
 
-        const {range = null, completion = '', href = null, suffix = ''} = displayedCompletion;
+        const {
+            range = null,
+            completion = '',
+            completionId = '',
+            href = null,
+            suffix = ''
+        } = displayedCompletion;
 
         let inline;
         if (href) {
             inline = Inline.create({
                 type: 'pill',
                 data: { url: href },
-                nodes: [Text.create(completion)],
+                nodes: [Text.create(completionId || completion)],
             });
         } else if (completion === '@room') {
             inline = Inline.create({
                 type: 'pill',
                 data: { type: Pill.TYPE_AT_ROOM_MENTION },
-                nodes: [Text.create(completion)],
+                nodes: [Text.create(completionId || completion)],
             });
         }
 

src/stores/MessageComposerStore.js

@@ -44,7 +44,7 @@ class MessageComposerStore extends Store {
     __onDispatch(payload) {
         switch (payload.action) {
             case 'editor_state':
-                this._contentState(payload);
+                this._editorState(payload);
                 break;
             case 'on_logged_out':
                 this.reset();
@@ -52,7 +52,7 @@ class MessageComposerStore extends Store {
         }
     }
 
-    _contentState(payload) {
+    _editorState(payload) {
         const editorStateMap = this._state.editorStateMap;
         editorStateMap[payload.room_id] = payload.editor_state;
         localStorage.setItem('editor_state', JSON.stringify(editorStateMap));
@@ -61,7 +61,7 @@ class MessageComposerStore extends Store {
         });
     }
 
-    getContentState(roomId) {
+    getEditorState(roomId) {
         return this._state.editorStateMap[roomId];
     }