From 709b2eed7c802789334769d176790bfe5c6d9705 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 19 Dec 2019 15:10:54 +0000 Subject: [PATCH 01/21] Add bunch of null-guards and similar to fix React Errors/complaints --- src/components/views/elements/LazyRenderList.js | 10 ++++++++-- src/components/views/elements/Pill.js | 2 +- src/components/views/elements/SettingsFlag.js | 5 +++-- src/components/views/elements/TagTile.js | 2 ++ src/components/views/groups/GroupPublicityToggle.js | 4 ++-- 5 files changed, 16 insertions(+), 7 deletions(-) diff --git a/src/components/views/elements/LazyRenderList.js b/src/components/views/elements/LazyRenderList.js index 0fc0ef6733..7572dced0b 100644 --- a/src/components/views/elements/LazyRenderList.js +++ b/src/components/views/elements/LazyRenderList.js @@ -56,14 +56,20 @@ class ItemRange { } export default class LazyRenderList extends React.Component { + constructor(props) { + super(props); + + this.state = {}; + } + static getDerivedStateFromProps(props, state) { const range = LazyRenderList.getVisibleRangeFromProps(props); const intersectRange = range.expand(props.overflowMargin); const renderRange = range.expand(props.overflowItems); - const listHasChangedSize = !!state && renderRange.totalSize() !== state.renderRange.totalSize(); + const listHasChangedSize = !!state.renderRange && renderRange.totalSize() !== state.renderRange.totalSize(); // only update render Range if the list has shrunk/grown and we need to adjust padding OR // if the new range + overflowMargin isn't contained by the old anymore - if (listHasChangedSize || !state || !state.renderRange.contains(intersectRange)) { + if (listHasChangedSize || !state.renderRange || !state.renderRange.contains(intersectRange)) { return {renderRange}; } return null; diff --git a/src/components/views/elements/Pill.js b/src/components/views/elements/Pill.js index 12830488b1..b821276e92 100644 --- a/src/components/views/elements/Pill.js +++ b/src/components/views/elements/Pill.js @@ -127,7 +127,7 @@ const Pill = createReactClass({ } break; case Pill.TYPE_USER_MENTION: { - const localMember = nextProps.room.getMember(resourceId); + const localMember = nextProps.room ? nextProps.room.getMember(resourceId) : null; member = localMember; if (!localMember) { member = new RoomMember(null, resourceId); diff --git a/src/components/views/elements/SettingsFlag.js b/src/components/views/elements/SettingsFlag.js index a3a6d18d33..d62f3182fc 100644 --- a/src/components/views/elements/SettingsFlag.js +++ b/src/components/views/elements/SettingsFlag.js @@ -34,12 +34,13 @@ module.exports = createReactClass({ getInitialState: function() { return { - value: SettingsStore.getValueAt( + // convert to Boolean to protect against null-capable "tri-state" Settings e.g fallbackICEServerAllowed + value: Boolean(SettingsStore.getValueAt( this.props.level, this.props.name, this.props.roomId, this.props.isExplicit, - ), + )), }; }, diff --git a/src/components/views/elements/TagTile.js b/src/components/views/elements/TagTile.js index 767980f0a0..e31c1c524e 100644 --- a/src/components/views/elements/TagTile.js +++ b/src/components/views/elements/TagTile.js @@ -56,6 +56,8 @@ export default createReactClass({ hover: false, // The profile data of the group if this.props.tag is a group ID profile: null, + // Whether or not the context menu is open + menuDisplayed: false, }; }, diff --git a/src/components/views/groups/GroupPublicityToggle.js b/src/components/views/groups/GroupPublicityToggle.js index bacf54382a..176bd9f988 100644 --- a/src/components/views/groups/GroupPublicityToggle.js +++ b/src/components/views/groups/GroupPublicityToggle.js @@ -32,7 +32,7 @@ export default createReactClass({ return { busy: false, ready: false, - isGroupPublicised: null, + isGroupPublicised: false, // assume false as expects a boolean }; }, @@ -43,7 +43,7 @@ export default createReactClass({ _initGroupStore: function(groupId) { this._groupStoreToken = GroupStore.registerListener(groupId, () => { this.setState({ - isGroupPublicised: GroupStore.getGroupPublicity(groupId), + isGroupPublicised: Boolean(GroupStore.getGroupPublicity(groupId)), ready: GroupStore.isStateReady(groupId, GroupStore.STATE_KEY.Summary), }); }); From 4489b5a21aa7550d1bfeee1b4f3960ebc4698cb7 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Sat, 28 Dec 2019 20:05:55 +0000 Subject: [PATCH 02/21] Escape HTML in og:description and render any html &-encoded entities --- src/components/views/rooms/LinkPreviewWidget.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/components/views/rooms/LinkPreviewWidget.js b/src/components/views/rooms/LinkPreviewWidget.js index ee63cd1bb7..06c0201af8 100644 --- a/src/components/views/rooms/LinkPreviewWidget.js +++ b/src/components/views/rooms/LinkPreviewWidget.js @@ -128,15 +128,15 @@ module.exports = createReactClass({ } const AccessibleButton = sdk.getComponent('elements.AccessibleButton'); + // Escape to prevent any HTML injections, we can't replace & as the description may contain & encoded html entities + const safeDescription = (p["og:description"] || "").replace("<", "<").replace(">", ">"); return (
{ img }
{ p["og:title"] }
{ p["og:site_name"] ? (" - " + p["og:site_name"]) : null }
-
- { p["og:description"] } -
+
Date: Fri, 3 Jan 2020 19:41:06 -0700 Subject: [PATCH 03/21] Add suggestions for which users to invite to chat Fixes https://github.com/vector-im/riot-web/issues/11198 Note this doesn't implement the entire algorithm in 11198 because it feels too complicated at this stage. Instead, the idea is to review the suggestions closer to when the whole dialog is complete and fix them then: https://github.com/vector-im/riot-web/issues/11769 Algorithm for picking members is largely based on https://github.com/matrix-org/matrix-react-sdk/commit/db5218e19a26b650d4af37de81f516c68ab7e9b8 --- .../views/dialogs/DMInviteDialog.js | 91 ++++++++++++++++--- src/i18n/strings/en_EN.json | 3 +- 2 files changed, 81 insertions(+), 13 deletions(-) diff --git a/src/components/views/dialogs/DMInviteDialog.js b/src/components/views/dialogs/DMInviteDialog.js index ff498e3e75..bdeae6bc3e 100644 --- a/src/components/views/dialogs/DMInviteDialog.js +++ b/src/components/views/dialogs/DMInviteDialog.js @@ -23,6 +23,7 @@ import {makeUserPermalink} from "../../../utils/permalinks/Permalinks"; import DMRoomMap from "../../../utils/DMRoomMap"; import {RoomMember} from "matrix-js-sdk/lib/matrix"; import * as humanize from "humanize"; +import SdkConfig from "../../../SdkConfig"; // TODO: [TravisR] Make this generic for all kinds of invites @@ -36,10 +37,6 @@ class DMRoomTile extends React.PureComponent { onToggle: PropTypes.func.isRequired, }; - constructor() { - super(); - } - _onClick = (e) => { // Stop the browser from highlighting text e.preventDefault(); @@ -84,6 +81,8 @@ export default class DMInviteDialog extends React.PureComponent { filterText: "", recents: this._buildRecents(), numRecentsShown: INITIAL_ROOMS_SHOWN, + suggestions: this._buildSuggestions(), + numSuggestionsShown: INITIAL_ROOMS_SHOWN, }; } @@ -109,6 +108,59 @@ export default class DMInviteDialog extends React.PureComponent { return recents; } + _buildSuggestions(): {userId: string, user: RoomMember} { + const maxConsideredMembers = 200; + const client = MatrixClientPeg.get(); + const excludedUserIds = [client.getUserId(), SdkConfig.get()['welcomeUserId']]; + const joinedRooms = client.getRooms() + .filter(r => r.getMyMembership() === 'join') + .filter(r => r.getJoinedMemberCount() <= maxConsideredMembers); + const memberRooms = joinedRooms.reduce((members, room) => { + const joinedMembers = room.getJoinedMembers().filter(u => !excludedUserIds.includes(u.userId)); + for (const member of joinedMembers) { + if (!members[member.userId]) { + members[member.userId] = { + member: member, + // Track the room size of the 'picked' member so we can use the profile of + // the smallest room (likely a DM). + pickedMemberRoomSize: room.getJoinedMemberCount(), + rooms: [], + }; + } + + members[member.userId].rooms.push(room); + + if (room.getJoinedMemberCount() < members[member.userId].pickedMemberRoomSize) { + members[member.userId].member = member; + members[member.userId].pickedMemberRoomSize = room.getJoinedMemberCount(); + } + } + return members; + }, {/* userId => {member, rooms[]} */}); + const memberScores = Object.values(memberRooms).reduce((scores, entry) => { + const numMembersTotal = entry.rooms.reduce((c, r) => c + r.getJoinedMemberCount(), 0); + const maxRange = maxConsideredMembers * entry.rooms.length; + scores[entry.member.userId] = { + member: entry.member, + numRooms: entry.rooms.length, + score: Math.max(0, Math.pow(1 - (numMembersTotal / maxRange), 5)), + }; + return scores; + }, {/* userId => {member, numRooms, score} */}); + const members = Object.values(memberScores); + members.sort((a, b) => { + if (a.score === b.score) { + if (a.numRooms === b.numRooms) { + return a.member.userId.localeCompare(b.member.userId); + } + + return b.numRooms - a.numRooms; + } + return b.score - a.score; + }); + return members.map(m => ({userId: m.userId, user: m.member})); + } + _startDm = () => { this.props.onFinished(this.state.targets); }; @@ -125,6 +177,10 @@ export default class DMInviteDialog extends React.PureComponent { this.setState({numRecentsShown: this.state.numRecentsShown + INCREMENT_ROOMS_SHOWN}); }; + _showMoreSuggestions = () => { + this.setState({numSuggestionsShown: this.state.numSuggestionsShown + INCREMENT_ROOMS_SHOWN}); + }; + _toggleMember = (userId) => { const targets = this.state.targets.map(t => t); // cheap clone for mutation const idx = targets.indexOf(userId); @@ -133,29 +189,39 @@ export default class DMInviteDialog extends React.PureComponent { this.setState({targets}); }; - _renderRecents() { - if (!this.state.recents || this.state.recents.length === 0) return null; + _renderSection(kind: "recents"|"suggestions") { + const sourceMembers = kind === 'recents' ? this.state.recents : this.state.suggestions; + let showNum = kind === 'recents' ? this.state.numRecentsShown : this.state.numSuggestionsShown; + const showMoreFn = kind === 'recents' ? this._showMoreRecents.bind(this) : this._showMoreSuggestions.bind(this); + const lastActive = (m) => kind === 'recents' ? m.lastActive : null; + const sectionName = kind === 'recents' ? _t("Recent Conversations") : _t("Suggestions"); + + if (!sourceMembers || sourceMembers.length === 0) return null; + + // If we're going to hide one member behind 'show more', just use up the space of the button + // with the member's tile instead. + if (showNum === sourceMembers.length - 1) showNum++; // .slice() will return an incomplete array but won't error on us if we go too far - const toRender = this.state.recents.slice(0, this.state.numRecentsShown); - const hasMore = toRender.length < this.state.recents.length; + const toRender = sourceMembers.slice(0, showNum); + const hasMore = toRender.length < sourceMembers.length; const AccessibleButton = sdk.getComponent("elements.AccessibleButton"); let showMore = null; if (hasMore) { showMore = ( - + {_t("Show more")} ); } const tiles = toRender.map(r => ( - + )); return (
-

{_t("Recent Conversations")}

+

{sectionName}

{tiles} {showMore}
@@ -209,7 +275,8 @@ export default class DMInviteDialog extends React.PureComponent { {_t("Go")}
- {this._renderRecents()} + {this._renderSection('recents')} + {this._renderSection('suggestions')}
); diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index cac3f2f619..4666a1fe9d 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -1431,8 +1431,9 @@ "View Servers in Room": "View Servers in Room", "Toolbox": "Toolbox", "Developer Tools": "Developer Tools", - "Show more": "Show more", "Recent Conversations": "Recent Conversations", + "Suggestions": "Suggestions", + "Show more": "Show more", "Direct Messages": "Direct Messages", "If you can't find someone, ask them for their username, or share your username (%(userId)s) or profile link.": "If you can't find someone, ask them for their username, or share your username (%(userId)s) or profile link.", "Go": "Go", From df25a988104a978b23cc3aa45ae739c97cfe31ef Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 3 Jan 2020 20:35:30 -0700 Subject: [PATCH 04/21] Implement basic filtering for invite targets Part of https://github.com/vector-im/riot-web/issues/11200 --- res/css/views/dialogs/_DMInviteDialog.scss | 4 + src/HtmlUtils.js | 5 ++ .../views/dialogs/DMInviteDialog.js | 75 +++++++++++++++++-- 3 files changed, 78 insertions(+), 6 deletions(-) diff --git a/res/css/views/dialogs/_DMInviteDialog.scss b/res/css/views/dialogs/_DMInviteDialog.scss index 1153ecb0d4..364c796f16 100644 --- a/res/css/views/dialogs/_DMInviteDialog.scss +++ b/res/css/views/dialogs/_DMInviteDialog.scss @@ -77,5 +77,9 @@ limitations under the License. float: right; line-height: 36px; // Height of the avatar to keep the time vertically aligned } + + .mx_DMInviteDialog_roomTile_highlight { + font-weight: 900; + } } diff --git a/src/HtmlUtils.js b/src/HtmlUtils.js index 7cdff26a21..ce677e6c68 100644 --- a/src/HtmlUtils.js +++ b/src/HtmlUtils.js @@ -528,3 +528,8 @@ export function checkBlockNode(node) { return false; } } + +export function htmlEntitiesEncode(str: string) { + // Source: https://stackoverflow.com/a/18750001/7037379 + return str.replace(/[\u00A0-\u9999<>&]/gim, i => `&#${i.charCodeAt(0)};`); +} diff --git a/src/components/views/dialogs/DMInviteDialog.js b/src/components/views/dialogs/DMInviteDialog.js index bdeae6bc3e..ba3221d632 100644 --- a/src/components/views/dialogs/DMInviteDialog.js +++ b/src/components/views/dialogs/DMInviteDialog.js @@ -24,6 +24,7 @@ import DMRoomMap from "../../../utils/DMRoomMap"; import {RoomMember} from "matrix-js-sdk/lib/matrix"; import * as humanize from "humanize"; import SdkConfig from "../../../SdkConfig"; +import {htmlEntitiesEncode} from "../../../HtmlUtils"; // TODO: [TravisR] Make this generic for all kinds of invites @@ -35,6 +36,7 @@ class DMRoomTile extends React.PureComponent { member: PropTypes.object.isRequired, lastActiveTs: PropTypes.number, onToggle: PropTypes.func.isRequired, + highlightWord: PropTypes.string, }; _onClick = (e) => { @@ -45,6 +47,44 @@ class DMRoomTile extends React.PureComponent { this.props.onToggle(this.props.member.userId); }; + _highlightName(str: string) { + if (!this.props.highlightWord) return str; + + // First encode the thing to avoid injection + str = htmlEntitiesEncode(str); + + // We convert things to lowercase for index searching, but pull substrings from + // the submitted text to preserve case. + const lowerStr = str.toLowerCase(); + const filterStr = this.props.highlightWord.toLowerCase(); + + const result = []; + + let i = 0; + let ii; + while ((ii = lowerStr.indexOf(filterStr, i)) >= 0) { + // Push any text we missed (first bit/middle of text) + if (ii > i) { + // Push any text we aren't highlighting (middle of text match) + result.push({str.substring(i, ii)}); + } + + i = ii; // copy over ii only if we have a match (to preserve i for end-of-text matching) + + // Highlight the word the user entered + const substr = str.substring(i, filterStr.length + i); + result.push({substr}); + i += substr.length; + } + + // Push any text we missed (end of text) + if (i < (str.length - 1)) { + result.push({str.substring(i)}); + } + + return result; + } + render() { const MemberAvatar = sdk.getComponent("views.avatars.MemberAvatar"); @@ -59,8 +99,8 @@ class DMRoomTile extends React.PureComponent { return (
- {this.props.member.name} - {this.props.member.userId} + {this._highlightName(this.props.member.name)} + {this._highlightName(this.props.member.userId)} {timestamp}
); @@ -158,7 +198,7 @@ export default class DMInviteDialog extends React.PureComponent { } return b.score - a.score; }); - return members.map(m => ({userId: m.userId, user: m.member})); + return members.map(m => ({userId: m.member.userId, user: m.member})); } _startDm = () => { @@ -190,14 +230,32 @@ export default class DMInviteDialog extends React.PureComponent { }; _renderSection(kind: "recents"|"suggestions") { - const sourceMembers = kind === 'recents' ? this.state.recents : this.state.suggestions; + let sourceMembers = kind === 'recents' ? this.state.recents : this.state.suggestions; let showNum = kind === 'recents' ? this.state.numRecentsShown : this.state.numSuggestionsShown; const showMoreFn = kind === 'recents' ? this._showMoreRecents.bind(this) : this._showMoreSuggestions.bind(this); const lastActive = (m) => kind === 'recents' ? m.lastActive : null; const sectionName = kind === 'recents' ? _t("Recent Conversations") : _t("Suggestions"); + // Hide the section if there's nothing to filter by if (!sourceMembers || sourceMembers.length === 0) return null; + // Do some simple filtering on the input before going much further. If we get no results, say so. + if (this.state.filterText) { + const filterBy = this.state.filterText.toLowerCase(); + sourceMembers = sourceMembers + .filter(m => m.user.name.toLowerCase().includes(filterBy) || m.userId.toLowerCase().includes(filterBy)); + + if (sourceMembers.length === 0) { + return ( +
+

{sectionName}

+

{_t("No results")}

+
+ ); + } + } + + // If we're going to hide one member behind 'show more', just use up the space of the button // with the member's tile instead. if (showNum === sourceMembers.length - 1) showNum++; @@ -217,7 +275,13 @@ export default class DMInviteDialog extends React.PureComponent { } const tiles = toRender.map(r => ( - + )); return (
@@ -241,7 +305,6 @@ export default class DMInviteDialog extends React.PureComponent { id="inviteTargets" value={this.state.filterText} onChange={this._updateFilter} - placeholder="TODO: Implement filtering/searching (vector-im/riot-web#11199)" />
); From 8b4c1e3dec79f699951d87fb11eb29541637c943 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 3 Jan 2020 21:17:48 -0700 Subject: [PATCH 05/21] Support searching in the user directory for invite targets Part of https://github.com/vector-im/riot-web/issues/11200 --- .../views/dialogs/DMInviteDialog.js | 84 ++++++++++++++++++- 1 file changed, 81 insertions(+), 3 deletions(-) diff --git a/src/components/views/dialogs/DMInviteDialog.js b/src/components/views/dialogs/DMInviteDialog.js index ba3221d632..aec64919a0 100644 --- a/src/components/views/dialogs/DMInviteDialog.js +++ b/src/components/views/dialogs/DMInviteDialog.js @@ -25,14 +25,41 @@ import {RoomMember} from "matrix-js-sdk/lib/matrix"; import * as humanize from "humanize"; import SdkConfig from "../../../SdkConfig"; import {htmlEntitiesEncode} from "../../../HtmlUtils"; +import {getHttpUriForMxc} from "matrix-js-sdk/lib/content-repo"; // TODO: [TravisR] Make this generic for all kinds of invites const INITIAL_ROOMS_SHOWN = 3; // Number of rooms to show at first const INCREMENT_ROOMS_SHOWN = 5; // Number of rooms to add when 'show more' is clicked +class DirectoryMember { + _userId: string; + _displayName: string; + _avatarUrl: string; + + constructor(userDirResult: {user_id: string, display_name: string, avatar_url: string}) { + this._userId = userDirResult.user_id; + this._displayName = userDirResult.display_name; + this._avatarUrl = userDirResult.avatar_url; + } + + // These next members are to implement the contract expected by DMRoomTile + get name(): string { + return this._displayName || this._userId; + } + + get userId(): string { + return this._userId; + } + + getMxcAvatarUrl(): string { + return this._avatarUrl; + } +} + class DMRoomTile extends React.PureComponent { static propTypes = { + // Has properties to match RoomMember: userId (str), name (str), getMxcAvatarUrl(): string member: PropTypes.object.isRequired, lastActiveTs: PropTypes.number, onToggle: PropTypes.func.isRequired, @@ -86,7 +113,7 @@ class DMRoomTile extends React.PureComponent { } render() { - const MemberAvatar = sdk.getComponent("views.avatars.MemberAvatar"); + const BaseAvatar = sdk.getComponent("views.avatars.BaseAvatar"); let timestamp = null; if (this.props.lastActiveTs) { @@ -96,9 +123,20 @@ class DMRoomTile extends React.PureComponent { timestamp = {humanTs}; } + const avatarSize = 36; + const avatarUrl = getHttpUriForMxc( + MatrixClientPeg.get().getHomeserverUrl(), this.props.member.getMxcAvatarUrl(), + avatarSize, avatarSize, "crop"); + return (
- + {this._highlightName(this.props.member.name)} {this._highlightName(this.props.member.userId)} {timestamp} @@ -113,6 +151,8 @@ export default class DMInviteDialog extends React.PureComponent { onFinished: PropTypes.func.isRequired, }; + _debounceTimer: number = null; + constructor() { super(); @@ -123,6 +163,7 @@ export default class DMInviteDialog extends React.PureComponent { numRecentsShown: INITIAL_ROOMS_SHOWN, suggestions: this._buildSuggestions(), numSuggestionsShown: INITIAL_ROOMS_SHOWN, + serverResultsMixin: [], // { user: DirectoryMember, userId: string }[], like recents and suggestions }; } @@ -210,7 +251,35 @@ export default class DMInviteDialog extends React.PureComponent { }; _updateFilter = (e) => { - this.setState({filterText: e.target.value}); + const term = e.target.value; + this.setState({filterText: term}); + + // Debounce server lookups to reduce spam. We don't clear the existing server + // results because they might still be vaguely accurate, likewise for races which + // could happen here. + if (this._debounceTimer) { + clearTimeout(this._debounceTimer); + } + this._debounceTimer = setTimeout(() => { + MatrixClientPeg.get().searchUserDirectory({term}).then(r => { + if (term !== this.state.filterText) { + // Discard the results - we were probably too slow on the server-side to make + // these results useful. This is a race we want to avoid because we could overwrite + // more accurate results. + return; + } + this.setState({ + serverResultsMixin: r.results.map(u => ({ + userId: u.user_id, + user: new DirectoryMember(u), + })), + }); + }).catch(e => { + console.error("Error searching user directory:"); + console.error(e); + this.setState({serverResultsMixin: []}); // clear results because it's moderately fatal + }); + }, 150); // 150ms debounce (human reaction time + some) }; _showMoreRecents = () => { @@ -236,6 +305,15 @@ export default class DMInviteDialog extends React.PureComponent { const lastActive = (m) => kind === 'recents' ? m.lastActive : null; const sectionName = kind === 'recents' ? _t("Recent Conversations") : _t("Suggestions"); + // Mix in the server results if we have any, but only if we're searching + if (this.state.filterText && this.state.serverResultsMixin && kind === 'suggestions') { + // only pick out the server results that aren't already covered though + const uniqueServerResults = this.state.serverResultsMixin + .filter(u => !sourceMembers.some(m => m.userId === u.userId)); + + sourceMembers = sourceMembers.concat(uniqueServerResults); + } + // Hide the section if there's nothing to filter by if (!sourceMembers || sourceMembers.length === 0) return null; From 295dcbfe487ebb9bb4928d029a052f28cb6a079c Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Sat, 4 Jan 2020 13:34:05 +0000 Subject: [PATCH 06/21] Fix ability to remove avatars Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/views/room_settings/RoomProfileSettings.js | 2 ++ src/components/views/settings/ProfileSettings.js | 2 ++ 2 files changed, 4 insertions(+) diff --git a/src/components/views/room_settings/RoomProfileSettings.js b/src/components/views/room_settings/RoomProfileSettings.js index 2093a31a28..abcdce23f8 100644 --- a/src/components/views/room_settings/RoomProfileSettings.js +++ b/src/components/views/room_settings/RoomProfileSettings.js @@ -98,6 +98,8 @@ export default class RoomProfileSettings extends React.Component { newState.avatarUrl = client.mxcUrlToHttp(uri, 96, 96, 'crop', false); newState.originalAvatarUrl = newState.avatarUrl; newState.avatarFile = null; + } else if (this.state.originalAvatarUrl !== this.state.avatarUrl) { + await client.sendStateEvent(this.props.roomId, 'm.room.avatar', {url: undefined}, ''); } if (this.state.originalTopic !== this.state.topic) { diff --git a/src/components/views/settings/ProfileSettings.js b/src/components/views/settings/ProfileSettings.js index 480b414911..72ff7e3d15 100644 --- a/src/components/views/settings/ProfileSettings.js +++ b/src/components/views/settings/ProfileSettings.js @@ -88,6 +88,8 @@ export default class ProfileSettings extends React.Component { newState.avatarUrl = client.mxcUrlToHttp(uri, 96, 96, 'crop', false); newState.originalAvatarUrl = newState.avatarUrl; newState.avatarFile = null; + } else if (this.state.originalAvatarUrl !== this.state.avatarUrl) { + await client.setAvatarUrl(""); // use empty string as Synapse 500's on undefined } this.setState(newState); From cf071c5ac659898f19865411bfb3e9c874bc2da8 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Sun, 5 Jan 2020 21:23:34 +0000 Subject: [PATCH 07/21] Deduplicate recent emoji Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/views/emojipicker/EmojiPicker.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/components/views/emojipicker/EmojiPicker.js b/src/components/views/emojipicker/EmojiPicker.js index 4d49b25100..0325dfd807 100644 --- a/src/components/views/emojipicker/EmojiPicker.js +++ b/src/components/views/emojipicker/EmojiPicker.js @@ -47,10 +47,10 @@ class EmojiPicker extends React.Component { viewportHeight: 280, }; - // Convert recent emoji characters to emoji data, removing unknowns. - this.recentlyUsed = recent.get() + // Convert recent emoji characters to emoji data, removing unknowns and duplicates + this.recentlyUsed = Array.from(new Set(recent.get() .map(unicode => getEmojiFromUnicode(unicode)) - .filter(data => !!data); + .filter(data => !!data))); this.memoizedDataByCategory = { recent: this.recentlyUsed, ...DATA_BY_CATEGORY, From ba2f4aa973db060afd62807b837e9ee266b245ab Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Sun, 5 Jan 2020 21:26:32 +0000 Subject: [PATCH 08/21] tidy Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/views/emojipicker/EmojiPicker.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/components/views/emojipicker/EmojiPicker.js b/src/components/views/emojipicker/EmojiPicker.js index 0325dfd807..a9cf2a4732 100644 --- a/src/components/views/emojipicker/EmojiPicker.js +++ b/src/components/views/emojipicker/EmojiPicker.js @@ -48,9 +48,7 @@ class EmojiPicker extends React.Component { }; // Convert recent emoji characters to emoji data, removing unknowns and duplicates - this.recentlyUsed = Array.from(new Set(recent.get() - .map(unicode => getEmojiFromUnicode(unicode)) - .filter(data => !!data))); + this.recentlyUsed = Array.from(new Set(recent.get().map(getEmojiFromUnicode).filter(Boolean))); this.memoizedDataByCategory = { recent: this.recentlyUsed, ...DATA_BY_CATEGORY, From b6098ddb47e138a3aca2a37d444788db08afa0dc Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Sun, 5 Jan 2020 21:58:36 +0000 Subject: [PATCH 09/21] Delegate all room alias validation to the RoomAliasField validator Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- .../views/room_settings/AliasSettings.js | 79 ++++++++++--------- 1 file changed, 43 insertions(+), 36 deletions(-) diff --git a/src/components/views/room_settings/AliasSettings.js b/src/components/views/room_settings/AliasSettings.js index aa9d46c9d5..8e75680a89 100644 --- a/src/components/views/room_settings/AliasSettings.js +++ b/src/components/views/room_settings/AliasSettings.js @@ -17,7 +17,7 @@ limitations under the License. import EditableItemList from "../elements/EditableItemList"; -const React = require('react'); +import React, {createRef} from 'react'; import PropTypes from 'prop-types'; const MatrixClientPeg = require('../../../MatrixClientPeg'); const sdk = require("../../../index"); @@ -28,22 +28,41 @@ import AccessibleButton from "../elements/AccessibleButton"; const Modal = require("../../../Modal"); class EditableAliasesList extends EditableItemList { + constructor(props) { + super(props); + + this._aliasField = createRef(); + } + + _onAliasAdded = async () => { + await this._aliasField.current.validate({ allowEmpty: false }); + + if (this._aliasField.current.isValid) { + if (this.props.onItemAdded) this.props.onItemAdded(this.props.newItem); + return; + } + + this._aliasField.current.focus(); + this._aliasField.current.validate({ allowEmpty: false, focused: true }); + }; + _renderNewItemField() { const RoomAliasField = sdk.getComponent('views.elements.RoomAliasField'); const onChange = (alias) => this._onNewItemChanged({target: {value: alias}}); return (
- + { _t("Add") } @@ -99,11 +118,6 @@ export default class AliasSettings extends React.Component { return dict; } - isAliasValid(alias) { - // XXX: FIXME https://github.com/matrix-org/matrix-doc/issues/668 - return (alias.match(/^#([^/:,]+?):(.+)$/) && encodeURI(alias) === alias); - } - changeCanonicalAlias(alias) { if (!this.props.canSetCanonicalAlias) return; @@ -139,38 +153,31 @@ export default class AliasSettings extends React.Component { const localDomain = MatrixClientPeg.get().getDomain(); if (!alias.includes(':')) alias += ':' + localDomain; - if (this.isAliasValid(alias) && alias.endsWith(localDomain)) { - MatrixClientPeg.get().createAlias(alias, this.props.roomId).then(() => { - const localAliases = this.state.domainToAliases[localDomain] || []; - const domainAliases = Object.assign({}, this.state.domainToAliases); - domainAliases[localDomain] = [...localAliases, alias]; - this.setState({ - domainToAliases: domainAliases, - // Reset the add field - newAlias: "", - }); + MatrixClientPeg.get().createAlias(alias, this.props.roomId).then(() => { + const localAliases = this.state.domainToAliases[localDomain] || []; + const domainAliases = Object.assign({}, this.state.domainToAliases); + domainAliases[localDomain] = [...localAliases, alias]; - if (!this.state.canonicalAlias) { - this.changeCanonicalAlias(alias); - } - }).catch((err) => { - console.error(err); - Modal.createTrackedDialog('Error creating alias', '', ErrorDialog, { - title: _t("Error creating alias"), - description: _t( - "There was an error creating that alias. It may not be allowed by the server " + - "or a temporary failure occurred.", - ), - }); + this.setState({ + domainToAliases: domainAliases, + // Reset the add field + newAlias: "", }); - } else { - const ErrorDialog = sdk.getComponent("dialogs.ErrorDialog"); - Modal.createTrackedDialog('Invalid alias format', '', ErrorDialog, { - title: _t('Invalid alias format'), - description: _t('\'%(alias)s\' is not a valid format for an alias', { alias: alias }), + + if (!this.state.canonicalAlias) { + this.changeCanonicalAlias(alias); + } + }).catch((err) => { + console.error(err); + Modal.createTrackedDialog('Error creating alias', '', ErrorDialog, { + title: _t("Error creating alias"), + description: _t( + "There was an error creating that alias. It may not be allowed by the server " + + "or a temporary failure occurred.", + ), }); - } + }); }; onLocalAliasDeleted = (index) => { From 4a836349ab5dbfc0c83f7e7983a64ced93604594 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Sun, 5 Jan 2020 22:01:59 +0000 Subject: [PATCH 10/21] regenerate i18n Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/i18n/strings/en_EN.json | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index cac3f2f619..9cae17157a 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -1081,8 +1081,6 @@ "There was an error updating the room's main address. It may not be allowed by the server or a temporary failure occurred.": "There was an error updating the room's main address. It may not be allowed by the server or a temporary failure occurred.", "Error creating alias": "Error creating alias", "There was an error creating that alias. It may not be allowed by the server or a temporary failure occurred.": "There was an error creating that alias. It may not be allowed by the server or a temporary failure occurred.", - "Invalid alias format": "Invalid alias format", - "'%(alias)s' is not a valid format for an alias": "'%(alias)s' is not a valid format for an alias", "Error removing alias": "Error removing alias", "There was an error removing that alias. It may no longer exist or a temporary error occurred.": "There was an error removing that alias. It may no longer exist or a temporary error occurred.", "Main address": "Main address", From 378a82e6fb760657b5f77c1d13cad81c298af0da Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Sun, 5 Jan 2020 22:22:09 +0000 Subject: [PATCH 11/21] Use html-entities instead Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- package.json | 3 ++- src/components/views/rooms/LinkPreviewWidget.js | 10 ++++++---- yarn.lock | 5 +++++ 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/package.json b/package.json index 7ef14e6635..7e2c242b3a 100644 --- a/package.json +++ b/package.json @@ -74,7 +74,6 @@ "file-saver": "^1.3.3", "filesize": "3.5.6", "flux": "2.1.1", - "react-focus-lock": "^2.2.1", "focus-visible": "^5.0.2", "fuse.js": "^2.2.0", "gemini-scrollbar": "github:matrix-org/gemini-scrollbar#91e1e566", @@ -82,6 +81,7 @@ "glob": "^5.0.14", "glob-to-regexp": "^0.4.1", "highlight.js": "^9.15.8", + "html-entities": "^1.2.1", "is-ip": "^2.0.0", "isomorphic-fetch": "^2.2.1", "linkifyjs": "^2.1.6", @@ -99,6 +99,7 @@ "react-addons-css-transition-group": "15.6.2", "react-beautiful-dnd": "^4.0.1", "react-dom": "^16.9.0", + "react-focus-lock": "^2.2.1", "react-gemini-scrollbar": "github:matrix-org/react-gemini-scrollbar#9cf17f63b7c0b0ec5f31df27da0f82f7238dc594", "resize-observer-polyfill": "^1.5.0", "sanitize-html": "^1.18.4", diff --git a/src/components/views/rooms/LinkPreviewWidget.js b/src/components/views/rooms/LinkPreviewWidget.js index 06c0201af8..4822848233 100644 --- a/src/components/views/rooms/LinkPreviewWidget.js +++ b/src/components/views/rooms/LinkPreviewWidget.js @@ -18,7 +18,9 @@ limitations under the License. import React, {createRef} from 'react'; import PropTypes from 'prop-types'; import createReactClass from 'create-react-class'; -import { linkifyElement } from '../../../HtmlUtils'; +import { AllHtmlEntities } from 'html-entities'; + +import {linkifyElement} from '../../../HtmlUtils'; import SettingsStore from "../../../settings/SettingsStore"; import { _t } from "../../../languageHandler"; @@ -128,15 +130,15 @@ module.exports = createReactClass({ } const AccessibleButton = sdk.getComponent('elements.AccessibleButton'); - // Escape to prevent any HTML injections, we can't replace & as the description may contain & encoded html entities - const safeDescription = (p["og:description"] || "").replace("<", "<").replace(">", ">"); return (
{ img }
{ p["og:site_name"] ? (" - " + p["og:site_name"]) : null }
-
+
+ { AllHtmlEntities.decode(p["og:description"] || "") } +
Date: Mon, 6 Jan 2020 00:18:24 +0000 Subject: [PATCH 12/21] Add comment and delint Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/views/rooms/LinkPreviewWidget.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/components/views/rooms/LinkPreviewWidget.js b/src/components/views/rooms/LinkPreviewWidget.js index 4822848233..3b5545e0e0 100644 --- a/src/components/views/rooms/LinkPreviewWidget.js +++ b/src/components/views/rooms/LinkPreviewWidget.js @@ -19,7 +19,6 @@ import React, {createRef} from 'react'; import PropTypes from 'prop-types'; import createReactClass from 'create-react-class'; import { AllHtmlEntities } from 'html-entities'; - import {linkifyElement} from '../../../HtmlUtils'; import SettingsStore from "../../../settings/SettingsStore"; import { _t } from "../../../languageHandler"; @@ -129,6 +128,10 @@ module.exports = createReactClass({
; } + // The description includes &-encoded HTML entities, we decode those as React treats the thing as an + // opaque string. This does not allow any HTML to be injected into the DOM. + const description = AllHtmlEntities.decode(p["og:description"] || ""); + const AccessibleButton = sdk.getComponent('elements.AccessibleButton'); return (
@@ -137,7 +140,7 @@ module.exports = createReactClass({
{ p["og:site_name"] ? (" - " + p["og:site_name"]) : null }
- { AllHtmlEntities.decode(p["og:description"] || "") } + { description }
From ac1d9b03bfbf1358ec3e092dc5a35c2e9c4598ad Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 6 Jan 2020 09:35:14 +0000 Subject: [PATCH 13/21] undo one of the "fixes" Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/views/elements/SettingsFlag.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/components/views/elements/SettingsFlag.js b/src/components/views/elements/SettingsFlag.js index d62f3182fc..a3a6d18d33 100644 --- a/src/components/views/elements/SettingsFlag.js +++ b/src/components/views/elements/SettingsFlag.js @@ -34,13 +34,12 @@ module.exports = createReactClass({ getInitialState: function() { return { - // convert to Boolean to protect against null-capable "tri-state" Settings e.g fallbackICEServerAllowed - value: Boolean(SettingsStore.getValueAt( + value: SettingsStore.getValueAt( this.props.level, this.props.name, this.props.roomId, this.props.isExplicit, - )), + ), }; }, From d5eb4ff310d2cd328f8795bd6eea37b90a4db86a Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 6 Jan 2020 13:02:32 +0000 Subject: [PATCH 14/21] Update src/components/views/settings/ProfileSettings.js Co-Authored-By: J. Ryan Stinnett --- src/components/views/settings/ProfileSettings.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/settings/ProfileSettings.js b/src/components/views/settings/ProfileSettings.js index 72ff7e3d15..b56321c1c9 100644 --- a/src/components/views/settings/ProfileSettings.js +++ b/src/components/views/settings/ProfileSettings.js @@ -89,7 +89,7 @@ export default class ProfileSettings extends React.Component { newState.originalAvatarUrl = newState.avatarUrl; newState.avatarFile = null; } else if (this.state.originalAvatarUrl !== this.state.avatarUrl) { - await client.setAvatarUrl(""); // use empty string as Synapse 500's on undefined + await client.setAvatarUrl(""); // use empty string as Synapse 500s on undefined } this.setState(newState); From bef824e84ee36769712fc3c48171032056875df0 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 6 Jan 2020 12:21:59 -0700 Subject: [PATCH 15/21] Remove harmful html entities encoding and other style nits React will take care of this for us. It's harmful because simple characters get converted to something illegible. --- src/HtmlUtils.js | 5 ----- src/components/views/dialogs/DMInviteDialog.js | 12 ++++-------- 2 files changed, 4 insertions(+), 13 deletions(-) diff --git a/src/HtmlUtils.js b/src/HtmlUtils.js index ce677e6c68..7cdff26a21 100644 --- a/src/HtmlUtils.js +++ b/src/HtmlUtils.js @@ -528,8 +528,3 @@ export function checkBlockNode(node) { return false; } } - -export function htmlEntitiesEncode(str: string) { - // Source: https://stackoverflow.com/a/18750001/7037379 - return str.replace(/[\u00A0-\u9999<>&]/gim, i => `&#${i.charCodeAt(0)};`); -} diff --git a/src/components/views/dialogs/DMInviteDialog.js b/src/components/views/dialogs/DMInviteDialog.js index aec64919a0..bb3e38a304 100644 --- a/src/components/views/dialogs/DMInviteDialog.js +++ b/src/components/views/dialogs/DMInviteDialog.js @@ -24,7 +24,6 @@ import DMRoomMap from "../../../utils/DMRoomMap"; import {RoomMember} from "matrix-js-sdk/lib/matrix"; import * as humanize from "humanize"; import SdkConfig from "../../../SdkConfig"; -import {htmlEntitiesEncode} from "../../../HtmlUtils"; import {getHttpUriForMxc} from "matrix-js-sdk/lib/content-repo"; // TODO: [TravisR] Make this generic for all kinds of invites @@ -77,11 +76,9 @@ class DMRoomTile extends React.PureComponent { _highlightName(str: string) { if (!this.props.highlightWord) return str; - // First encode the thing to avoid injection - str = htmlEntitiesEncode(str); - // We convert things to lowercase for index searching, but pull substrings from - // the submitted text to preserve case. + // the submitted text to preserve case. Note: we don't need to htmlEntities the + // string because React will safely encode the text for us. const lowerStr = str.toLowerCase(); const filterStr = this.props.highlightWord.toLowerCase(); @@ -92,8 +89,8 @@ class DMRoomTile extends React.PureComponent { while ((ii = lowerStr.indexOf(filterStr, i)) >= 0) { // Push any text we missed (first bit/middle of text) if (ii > i) { - // Push any text we aren't highlighting (middle of text match) - result.push({str.substring(i, ii)}); + // Push any text we aren't highlighting (middle of text match, or beginning of text) + result.push({str.substring(i, ii)}); } i = ii; // copy over ii only if we have a match (to preserve i for end-of-text matching) @@ -333,7 +330,6 @@ export default class DMInviteDialog extends React.PureComponent { } } - // If we're going to hide one member behind 'show more', just use up the space of the button // with the member's tile instead. if (showNum === sourceMembers.length - 1) showNum++; From 6c410e602787ef041f85fb6aa62ca6ae5bae3b4f Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 6 Jan 2020 12:23:35 -0700 Subject: [PATCH 16/21] Fix comment style --- src/components/views/dialogs/DMInviteDialog.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/components/views/dialogs/DMInviteDialog.js b/src/components/views/dialogs/DMInviteDialog.js index bdeae6bc3e..2854c914f8 100644 --- a/src/components/views/dialogs/DMInviteDialog.js +++ b/src/components/views/dialogs/DMInviteDialog.js @@ -115,6 +115,8 @@ export default class DMInviteDialog extends React.PureComponent { const joinedRooms = client.getRooms() .filter(r => r.getMyMembership() === 'join') .filter(r => r.getJoinedMemberCount() <= maxConsideredMembers); + + // Generates { userId: {member, rooms[]} } const memberRooms = joinedRooms.reduce((members, room) => { const joinedMembers = room.getJoinedMembers().filter(u => !excludedUserIds.includes(u.userId)); for (const member of joinedMembers) { @@ -136,7 +138,9 @@ export default class DMInviteDialog extends React.PureComponent { } } return members; - }, {/* userId => {member, rooms[]} */}); + }, {}); + + // Generates { userId: {member, numRooms, score} } const memberScores = Object.values(memberRooms).reduce((scores, entry) => { const numMembersTotal = entry.rooms.reduce((c, r) => c + r.getJoinedMemberCount(), 0); const maxRange = maxConsideredMembers * entry.rooms.length; @@ -146,7 +150,8 @@ export default class DMInviteDialog extends React.PureComponent { score: Math.max(0, Math.pow(1 - (numMembersTotal / maxRange), 5)), }; return scores; - }, {/* userId => {member, numRooms, score} */}); + }, {}); + const members = Object.values(memberScores); members.sort((a, b) => { if (a.score === b.score) { From 7b91d2056b2098e7a12e574a8495503d23570c15 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 6 Jan 2020 23:50:42 +0000 Subject: [PATCH 17/21] Attempt to fix e2e tests Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- test/end-to-end-tests/src/usecases/room-settings.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/end-to-end-tests/src/usecases/room-settings.js b/test/end-to-end-tests/src/usecases/room-settings.js index 5b425f14b7..8e9e267d45 100644 --- a/test/end-to-end-tests/src/usecases/room-settings.js +++ b/test/end-to-end-tests/src/usecases/room-settings.js @@ -55,6 +55,7 @@ module.exports = async function changeRoomSettings(session, settings) { await session.replaceInputText(aliasField, settings.alias.substring(1, settings.alias.lastIndexOf(":"))); const addButton = await session.query(".mx_RoomSettingsDialog .mx_AliasSettings .mx_AccessibleButton"); await addButton.click(); + await session.delay(10); // delay to give time for the validator to run and check the alias session.log.done(); } From a62c260f794f2f89815045458b05a656bc5962dc Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 7 Jan 2020 12:58:24 +0000 Subject: [PATCH 18/21] Fix userinfo for users not in the room Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/views/right_panel/UserInfo.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/views/right_panel/UserInfo.js b/src/components/views/right_panel/UserInfo.js index b55991eda8..9c90953114 100644 --- a/src/components/views/right_panel/UserInfo.js +++ b/src/components/views/right_panel/UserInfo.js @@ -1000,8 +1000,8 @@ const UserInfo = ({user, groupId, roomId, onClose}) => { // Load room if we are given a room id and memoize it const room = useMemo(() => roomId ? cli.getRoom(roomId) : null, [cli, roomId]); - // fetch latest room member if we have a room, so we don't show historical information - const member = useMemo(() => room ? room.getMember(user.userId) : user, [room, user]); + // fetch latest room member if we have a room, so we don't show historical information, falling back to user + const member = useMemo(() => room ? (room.getMember(user.userId) || user) : user, [room, user]); // only display the devices list if our client supports E2E const _enableDevices = cli.isCryptoEnabled(); From 40b23d6aa4cb6ad465c4ec1c9c2b55bd2ebe3cc9 Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 7 Jan 2020 18:48:55 +0000 Subject: [PATCH 19/21] Strip all variation selectors on emoji ...when inserting into or looking up in the unicode to emoji map. This broke with emojibase 4.2.0 which changed the type of a whole load of emojis to 'text' when previously they were 'emoji'. This caused them to get the 'text' variant of the unicode string which has the text variation selector (15) appended instead of the emoji variation selector (16). We were only stripping the emoji selector, so upgrading to 4.2.0 caused riot to fail to find the heart in the unicode map, which therefore prevented the app from starting. --- src/emoji.js | 39 ++++++++++++++++++++++++++++----------- 1 file changed, 28 insertions(+), 11 deletions(-) diff --git a/src/emoji.js b/src/emoji.js index 7b7a9c1bfe..8b3c4c9fe4 100644 --- a/src/emoji.js +++ b/src/emoji.js @@ -16,14 +16,12 @@ limitations under the License. import EMOJIBASE from 'emojibase-data/en/compact.json'; -export const VARIATION_SELECTOR = String.fromCharCode(0xFE0F); - // The unicode is stored without the variant selector const UNICODE_TO_EMOJI = new Map(); // not exported as gets for it are handled by getEmojiFromUnicode export const EMOTICON_TO_EMOJI = new Map(); export const SHORTCODE_TO_EMOJI = new Map(); -export const getEmojiFromUnicode = unicode => UNICODE_TO_EMOJI.get(unicode.replace(VARIATION_SELECTOR, "")); +export const getEmojiFromUnicode = unicode => UNICODE_TO_EMOJI.get(stripVariation(unicode)); const EMOJIBASE_GROUP_ID_TO_CATEGORY = [ "people", // smileys @@ -51,13 +49,6 @@ export const DATA_BY_CATEGORY = { // Store various mappings from unicode/emoticon/shortcode to the Emoji objects EMOJIBASE.forEach(emoji => { - if (emoji.unicode.includes(VARIATION_SELECTOR)) { - // Clone data into variation-less version - emoji = Object.assign({}, emoji, { - unicode: emoji.unicode.replace(VARIATION_SELECTOR, ""), - }); - } - const categoryId = EMOJIBASE_GROUP_ID_TO_CATEGORY[emoji.group]; if (DATA_BY_CATEGORY.hasOwnProperty(categoryId)) { DATA_BY_CATEGORY[categoryId].push(emoji); @@ -66,7 +57,13 @@ EMOJIBASE.forEach(emoji => { emoji.filterString = `${emoji.annotation}\n${emoji.shortcodes.join('\n')}}\n${emoji.emoticon || ''}`.toLowerCase(); // Add mapping from unicode to Emoji object - UNICODE_TO_EMOJI.set(emoji.unicode, emoji); + // The 'unicode' field that we use in emojibase has either + // VS15 or VS16 appended to any characters that can take + // variation selectors. Which one it appends depends + // on whether emojibase considers their type to be 'text' or + // 'emoji'. We therefore strip any variation chars from strings + // both when building the map and when looking up. + UNICODE_TO_EMOJI.set(stripVariation(emoji.unicode), emoji); if (emoji.emoticon) { // Add mapping from emoticon to Emoji object @@ -80,3 +77,23 @@ EMOJIBASE.forEach(emoji => { }); } }); + +/** + * Strips variation selectors from a string + * NB. Skin tone modifers are not variation selectors: + * this function does not touch them. (Should it?) + * + * @param {string} str string to strip + * @returns {string} stripped string + */ +function stripVariation(str) { + let ret = ''; + for (let i = 0; i < str.length; ++i) { + const charCode = str.charCodeAt(i); + // append to output only if it's outside the variation selector range + if (charCode < 0xFE00 && charCode > 0xFE0F) { + ret += str.charAt(i); + } + } + return ret; +} From 2d410c91acdc41fa7bcd985787429e40f8188055 Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 7 Jan 2020 19:57:17 +0000 Subject: [PATCH 20/21] Use a regex because it's simpler and works and my loop did not because I meant 'or', not 'and' --- src/emoji.js | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/src/emoji.js b/src/emoji.js index 8b3c4c9fe4..d62630ae08 100644 --- a/src/emoji.js +++ b/src/emoji.js @@ -87,13 +87,5 @@ EMOJIBASE.forEach(emoji => { * @returns {string} stripped string */ function stripVariation(str) { - let ret = ''; - for (let i = 0; i < str.length; ++i) { - const charCode = str.charCodeAt(i); - // append to output only if it's outside the variation selector range - if (charCode < 0xFE00 && charCode > 0xFE0F) { - ret += str.charAt(i); - } - } - return ret; + return str.replace("\uFE00-\uFE0F", ""); } From 45ef57d86f9d2a5523468c2a8c7ee87fd158bcaa Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 7 Jan 2020 20:41:19 +0000 Subject: [PATCH 21/21] Use a regex that actually works --- src/emoji.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/emoji.js b/src/emoji.js index d62630ae08..125864e381 100644 --- a/src/emoji.js +++ b/src/emoji.js @@ -87,5 +87,5 @@ EMOJIBASE.forEach(emoji => { * @returns {string} stripped string */ function stripVariation(str) { - return str.replace("\uFE00-\uFE0F", ""); + return str.replace(/[\uFE00-\uFE0F]/, ""); }