From e8495f637f1bf068dc0fa0395f65384d3edd0ad1 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Fri, 21 Jul 2017 16:38:31 +0100 Subject: [PATCH 1/6] Strip MD mentions from the `body` of sent messages Because previously we just sent the display name and MD links are not very readable. --- src/components/views/rooms/MessageComposerInput.js | 10 +++++++++- src/linkify-matrix.js | 2 ++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/components/views/rooms/MessageComposerInput.js b/src/components/views/rooms/MessageComposerInput.js index 2c011f3770..e489bdd8d7 100644 --- a/src/components/views/rooms/MessageComposerInput.js +++ b/src/components/views/rooms/MessageComposerInput.js @@ -45,8 +45,9 @@ import ComposerHistoryManager from '../../../ComposerHistoryManager'; import MessageComposerStore from '../../../stores/MessageComposerStore'; import { getDisplayAliasForRoom } from '../../../Rooms'; -import {MATRIXTO_URL_PATTERN} from '../../../linkify-matrix'; +import {MATRIXTO_URL_PATTERN, MATRIXTO_MD_LINK_PATTERN} from '../../../linkify-matrix'; const REGEX_MATRIXTO = new RegExp(MATRIXTO_URL_PATTERN); +const REGEX_MATRIXTO_MARKDOWN_GLOBAL = new RegExp(MATRIXTO_MD_LINK_PATTERN, 'g'); import {asciiRegexp, shortnameToUnicode, emojioneList, asciiList, mapUnicodeToShort} from 'emojione'; const EMOJI_SHORTNAMES = Object.keys(emojioneList); @@ -778,6 +779,13 @@ export default class MessageComposerInput extends React.Component { sendTextFn = this.client.sendEmoteMessage; } + // Strip MD user mentions to preserve plaintext mention behaviour + // (MD links are very verbose and ugly) + contentText = contentText.replace(REGEX_MATRIXTO_MARKDOWN_GLOBAL, + (markdownLink, text, resource, prefix) => { + return prefix === '@' ? text : markdownLink; + }); + let sendMessagePromise; if (contentHTML) { sendMessagePromise = sendHtmlFn.call( diff --git a/src/linkify-matrix.js b/src/linkify-matrix.js index 01512a771a..e395b7986e 100644 --- a/src/linkify-matrix.js +++ b/src/linkify-matrix.js @@ -168,6 +168,8 @@ matrixLinkify.VECTOR_URL_PATTERN = "^(?:https?:\/\/)?(?:" + ")(#.*)"; matrixLinkify.MATRIXTO_URL_PATTERN = "^(?:https?:\/\/)?(?:www\\.)?matrix\\.to/#/((#|@|!).*)"; +matrixLinkify.MATRIXTO_MD_LINK_PATTERN = + '\\[([^\\]]*)\\]\\((?:https?:\/\/)?(?:www\\.)?matrix\\.to/#/((#|@|!)[^\\)]*)\\)'; matrixLinkify.MATRIXTO_BASE_URL= "https://matrix.to"; matrixLinkify.options = { From 397201a74d16f91c30d0c903524350fd1e8a42b3 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Mon, 24 Jul 2017 09:41:46 +0100 Subject: [PATCH 2/6] Remove subjective comment --- src/components/views/rooms/MessageComposerInput.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/components/views/rooms/MessageComposerInput.js b/src/components/views/rooms/MessageComposerInput.js index 0dab6ed9c0..10414ecbeb 100644 --- a/src/components/views/rooms/MessageComposerInput.js +++ b/src/components/views/rooms/MessageComposerInput.js @@ -732,7 +732,6 @@ export default class MessageComposerInput extends React.Component { } // Strip MD user mentions to preserve plaintext mention behaviour - // (MD links are very verbose and ugly) contentText = contentText.replace(REGEX_MATRIXTO_MARKDOWN_GLOBAL, (markdownLink, text, resource, prefix) => { return prefix === '@' ? text : markdownLink; From 6945fa54eac5c3f9d92a38a956d889bf30c3c15e Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Mon, 24 Jul 2017 14:41:13 +0100 Subject: [PATCH 3/6] Reimplement so that only tab-completed mentions are stripped Instead of blindly stripping all MD mentions, only strip those that were tab-completed. We do this by adding the `isCompleted` flag to the Entity data. --- src/components/views/elements/Pill.js | 1 - .../views/rooms/MessageComposerInput.js | 34 ++++++++++++++++--- 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/src/components/views/elements/Pill.js b/src/components/views/elements/Pill.js index 39987228e5..89607f4eb8 100644 --- a/src/components/views/elements/Pill.js +++ b/src/components/views/elements/Pill.js @@ -92,7 +92,6 @@ export default React.createClass({ avatar = ; } } - const classes = classNames({ "mx_UserPill": isUserPill, "mx_RoomPill": isRoomPill, diff --git a/src/components/views/rooms/MessageComposerInput.js b/src/components/views/rooms/MessageComposerInput.js index 10414ecbeb..2863b13243 100644 --- a/src/components/views/rooms/MessageComposerInput.js +++ b/src/components/views/rooms/MessageComposerInput.js @@ -731,10 +731,33 @@ export default class MessageComposerInput extends React.Component { sendTextFn = this.client.sendEmoteMessage; } - // Strip MD user mentions to preserve plaintext mention behaviour + // Strip MD user (tab-completed) mentions to preserve plaintext mention behaviour contentText = contentText.replace(REGEX_MATRIXTO_MARKDOWN_GLOBAL, - (markdownLink, text, resource, prefix) => { - return prefix === '@' ? text : markdownLink; + (markdownLink, text, resource, prefix, offset) => { + // Calculate the offset relative to the current block that the offset is in + let sum = 0; + const blocks = contentState.getBlocksAsArray(); + let block; + for (let i = 0; i < blocks.length; i++) { + block = blocks[i]; + sum += block.getLength(); + if (sum > offset) { + sum -= block.getLength(); + break; + } + } + offset -= sum; + + const entityKey = block.getEntityAt(offset); + const entity = entityKey ? Entity.get(entityKey) : null; + if (entity && entity.getData().isCompletion && prefix === '@') { + // This is a completed mention, so do not insert MD link, just text + return text; + } else { + // This is either a MD link that was typed into the composer or another + // type of pill (e.g. room pill) + return markdownLink; + } }); let sendMessagePromise; @@ -900,7 +923,10 @@ export default class MessageComposerInput extends React.Component { let entityKey; let mdCompletion; if (href) { - entityKey = Entity.create('LINK', 'IMMUTABLE', {url: href}); + entityKey = Entity.create('LINK', 'IMMUTABLE', { + url: href, + isCompletion: true, + }); if (!this.state.isRichtextEnabled) { mdCompletion = `[${completion}](${href})`; } From f7145941fda926a7a67b078d8b8de10960cbee57 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Mon, 24 Jul 2017 14:42:20 +0100 Subject: [PATCH 4/6] Add tests to assert correct MD mention link stripping Tab-completed @Mentions should only be sent as display names in the `body` of the event. The HTML should be unaffected, and always sent as an anchor tag. --- .../views/rooms/MessageComposerInput-test.js | 66 +++++++++++++++++++ test/test-utils.js | 7 +- 2 files changed, 72 insertions(+), 1 deletion(-) diff --git a/test/components/views/rooms/MessageComposerInput-test.js b/test/components/views/rooms/MessageComposerInput-test.js index fe379afcff..e5d98e26c6 100644 --- a/test/components/views/rooms/MessageComposerInput-test.js +++ b/test/components/views/rooms/MessageComposerInput-test.js @@ -9,6 +9,7 @@ import sdk from 'matrix-react-sdk'; import UserSettingsStore from '../../../../src/UserSettingsStore'; const MessageComposerInput = sdk.getComponent('views.rooms.MessageComposerInput'); import MatrixClientPeg from '../../../../src/MatrixClientPeg'; +import RoomMember from 'matrix-js-sdk'; function addTextToDraft(text) { const components = document.getElementsByClassName('public-DraftEditor-content'); @@ -31,6 +32,7 @@ describe('MessageComposerInput', () => { testUtils.beforeEach(this); sandbox = testUtils.stubClient(sandbox); client = MatrixClientPeg.get(); + client.credentials = {userId: '@me:domain.com'}; parentDiv = document.createElement('div'); document.body.appendChild(parentDiv); @@ -236,4 +238,68 @@ describe('MessageComposerInput', () => { 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.'); }); + + it('should strip tab-completed mentions so that only the display name is sent in the plain body in Markdown mode', () => { + // Sending a HTML message because we have entities in the composer (because of completions) + const spy = sinon.spy(client, 'sendHtmlMessage'); + mci.enableRichtext(false); + mci.setDisplayedCompletion({ + completion: 'Some Member', + selection: mci.state.editorState.getSelection(), + href: `https://matrix.to/#/@some_member:domain.bla`, + }); + + mci.handleReturn(sinon.stub()); + + expect(spy.args[0][1]).toEqual( + 'Some Member', + 'the plaintext body should only include the display name', + ); + expect(spy.args[0][2]).toEqual( + 'Some Member', + 'the html body should contain an anchor tag with a matrix.to href and display name text', + ); + }); + + it('should strip tab-completed mentions so that only the display name is sent in the plain body in RTE mode', () => { + // Sending a HTML message because we have entities in the composer (because of completions) + const spy = sinon.spy(client, 'sendHtmlMessage'); + mci.enableRichtext(true); + mci.setDisplayedCompletion({ + completion: 'Some Member', + selection: mci.state.editorState.getSelection(), + href: `https://matrix.to/#/@some_member:domain.bla`, + }); + + mci.handleReturn(sinon.stub()); + + expect(spy.args[0][1]).toEqual('Some Member'); + expect(spy.args[0][2]).toEqual('Some Member'); + }); + + it('should not strip non-tab-completed mentions when manually typing MD', () => { + // Sending a HTML message because we have entities in the composer (because of completions) + const spy = sinon.spy(client, 'sendHtmlMessage'); + // Markdown mode enabled + mci.enableRichtext(false); + addTextToDraft('[My Not-Tab-Completed Mention](https://matrix.to/#/@some_member:domain.bla)'); + + mci.handleReturn(sinon.stub()); + + expect(spy.args[0][1]).toEqual('[My Not-Tab-Completed Mention](https://matrix.to/#/@some_member:domain.bla)'); + expect(spy.args[0][2]).toEqual('My Not-Tab-Completed Mention'); + }); + + it('should not strip arbitrary typed (i.e. not tab-completed) MD links', () => { + // Sending a HTML message because we have entities in the composer (because of completions) + const spy = sinon.spy(client, 'sendHtmlMessage'); + // Markdown mode enabled + mci.enableRichtext(false); + addTextToDraft('[Click here](https://some.lovely.url)'); + + mci.handleReturn(sinon.stub()); + + expect(spy.args[0][1]).toEqual('[Click here](https://some.lovely.url)'); + expect(spy.args[0][2]).toEqual('Click here'); + }); }); diff --git a/test/test-utils.js b/test/test-utils.js index 23f16a2e4c..06d3c03c49 100644 --- a/test/test-utils.js +++ b/test/test-utils.js @@ -238,7 +238,12 @@ export function mkStubRoom(roomId = null) { return { roomId, getReceiptsForEvent: sinon.stub().returns([]), - getMember: sinon.stub().returns({}), + getMember: sinon.stub().returns({ + userId: '@member:domain.bla', + name: 'Member', + roomId: roomId, + getAvatarUrl: () => 'mxc://avatar.url/image.png', + }), getJoinedMembers: sinon.stub().returns([]), getPendingEvents: () => [], getLiveTimeline: () => stubTimeline, From dc09ad8db4b74a7f7bf414530a95c73e0e6972e6 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Mon, 24 Jul 2017 14:50:44 +0100 Subject: [PATCH 5/6] Re-add NL --- src/components/views/elements/Pill.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/components/views/elements/Pill.js b/src/components/views/elements/Pill.js index 89607f4eb8..39987228e5 100644 --- a/src/components/views/elements/Pill.js +++ b/src/components/views/elements/Pill.js @@ -92,6 +92,7 @@ export default React.createClass({ avatar = ; } } + const classes = classNames({ "mx_UserPill": isUserPill, "mx_RoomPill": isRoomPill, From 727b1ca545a94b1734a1cf7a0e27a592425688c6 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Mon, 24 Jul 2017 15:07:33 +0100 Subject: [PATCH 6/6] Remove disambiguation from user mentions, add tooltip for mxid Also add a tooltip for room pills = room alias/ID Requires: https://github.com/matrix-org/matrix-js-sdk/pull/513 --- src/components/views/elements/Pill.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/components/views/elements/Pill.js b/src/components/views/elements/Pill.js index 39987228e5..294474a41a 100644 --- a/src/components/views/elements/Pill.js +++ b/src/components/views/elements/Pill.js @@ -78,7 +78,7 @@ export default React.createClass({ new RoomMember(null, resource); if (member) { userId = member.userId; - linkText = member.name; + linkText = member.rawDisplayName; avatar = ; } } else if (isRoomPill) { @@ -101,11 +101,11 @@ export default React.createClass({ if ((isUserPill || isRoomPill) && avatar) { return this.props.inMessage ? - + {avatar} {linkText} : - + {avatar} {linkText} ;