From ecc04d36de824117402bedcbe016d9e38336d957 Mon Sep 17 00:00:00 2001 From: Evan Hahn <69474926+EvanHahn-Signal@users.noreply.github.com> Date: Mon, 8 Mar 2021 16:31:19 -0600 Subject: [PATCH] Disallow group names longer than 32 extended graphemes --- components/GroupTitleInput.scss | 23 ++++ stylesheets/_modules.scss | 21 ---- stylesheets/manifest.scss | 3 +- ts/Intl.d.ts | 27 +++++ ts/components/GroupTitleInput.stories.tsx | 32 ++++++ ts/components/GroupTitleInput.tsx | 100 ++++++++++++++++++ ts/components/Tooltip.tsx | 18 +--- .../LeftPaneSetGroupMetadataHelper.tsx | 11 +- ts/test-both/util/grapheme_test.ts | 49 +++++++++ ts/util/grapheme.ts | 13 +++ ts/util/lint/exceptions.json | 29 ++++- ts/util/multiRef.ts | 22 ++++ 12 files changed, 302 insertions(+), 46 deletions(-) create mode 100644 components/GroupTitleInput.scss create mode 100644 ts/Intl.d.ts create mode 100644 ts/components/GroupTitleInput.stories.tsx create mode 100644 ts/components/GroupTitleInput.tsx create mode 100644 ts/test-both/util/grapheme_test.ts create mode 100644 ts/util/grapheme.ts create mode 100644 ts/util/multiRef.ts diff --git a/components/GroupTitleInput.scss b/components/GroupTitleInput.scss new file mode 100644 index 000000000..0f93e4312 --- /dev/null +++ b/components/GroupTitleInput.scss @@ -0,0 +1,23 @@ +// Copyright 2021 Signal Messenger, LLC +// SPDX-License-Identifier: AGPL-3.0-only + +.module-GroupTitleInput { + margin: 16px; + @include font-body-1; + padding: 8px 12px; + border-radius: 6px; + border: 2px solid $color-gray-15; + background: $color-white; + color: $color-black; + + &:focus { + outline: none; + + @include light-theme { + border-color: $ultramarine-ui-light; + } + @include dark-theme { + border-color: $ultramarine-ui-dark; + } + } +} diff --git a/stylesheets/_modules.scss b/stylesheets/_modules.scss index d355ec9ba..1c552f709 100644 --- a/stylesheets/_modules.scss +++ b/stylesheets/_modules.scss @@ -7433,27 +7433,6 @@ button.module-image__border-overlay:focus { } } -.module-left-pane__compose-input { - margin: 16px; - @include font-body-1; - padding: 8px 12px; - border-radius: 6px; - border: 2px solid $color-gray-15; - background: $color-white; - color: $color-black; - - &:focus { - outline: none; - - @include light-theme { - border-color: $ultramarine-ui-light; - } - @include dark-theme { - border-color: $ultramarine-ui-dark; - } - } -} - .module-left-pane__list--measure { flex-grow: 1; flex-shrink: 1; diff --git a/stylesheets/manifest.scss b/stylesheets/manifest.scss index 5d54d61ac..6316ea70a 100644 --- a/stylesheets/manifest.scss +++ b/stylesheets/manifest.scss @@ -32,5 +32,6 @@ @import './components/Button.scss'; @import './components/ContactPill.scss'; @import './components/ContactPills.scss'; -@import './components/GroupDialog.scss'; @import './components/ConversationHeader.scss'; +@import './components/GroupDialog.scss'; +@import './components/GroupTitleInput.scss'; diff --git a/ts/Intl.d.ts b/ts/Intl.d.ts new file mode 100644 index 000000000..0461dfc02 --- /dev/null +++ b/ts/Intl.d.ts @@ -0,0 +1,27 @@ +// Copyright 2021 Signal Messenger, LLC +// SPDX-License-Identifier: AGPL-3.0-only + +declare namespace Intl { + type SegmenterOptions = { + granularity?: 'grapheme' | 'word' | 'sentence'; + }; + + type SegmentData = { + index: number; + input: string; + segment: string; + }; + + interface Segments { + containing(index: number): SegmentData; + + [Symbol.iterator](): Iterator; + } + + // `Intl.Segmenter` is not yet in TypeScript's type definitions, so we add it. + class Segmenter { + constructor(locale?: string, options?: SegmenterOptions); + + segment(str: string): Segments; + } +} diff --git a/ts/components/GroupTitleInput.stories.tsx b/ts/components/GroupTitleInput.stories.tsx new file mode 100644 index 000000000..e6e0a43e2 --- /dev/null +++ b/ts/components/GroupTitleInput.stories.tsx @@ -0,0 +1,32 @@ +// Copyright 2021 Signal Messenger, LLC +// SPDX-License-Identifier: AGPL-3.0-only + +import React, { useState } from 'react'; + +import { storiesOf } from '@storybook/react'; + +import { setup as setupI18n } from '../../js/modules/i18n'; +import enMessages from '../../_locales/en/messages.json'; + +import { GroupTitleInput } from './GroupTitleInput'; + +const i18n = setupI18n('en', enMessages); + +const story = storiesOf('Components/GroupTitleInput', module); + +const Wrapper = ({ disabled }: { disabled?: boolean }) => { + const [value, setValue] = useState(''); + + return ( + + ); +}; + +story.add('Default', () => ); + +story.add('Disabled', () => ); diff --git a/ts/components/GroupTitleInput.tsx b/ts/components/GroupTitleInput.tsx new file mode 100644 index 000000000..d4df24fcc --- /dev/null +++ b/ts/components/GroupTitleInput.tsx @@ -0,0 +1,100 @@ +// Copyright 2021 Signal Messenger, LLC +// SPDX-License-Identifier: AGPL-3.0-only + +import React, { forwardRef, useRef, ClipboardEvent } from 'react'; + +import { LocalizerType } from '../types/Util'; +import { multiRef } from '../util/multiRef'; +import * as grapheme from '../util/grapheme'; + +const MAX_GRAPHEME_COUNT = 32; + +type PropsType = { + disabled?: boolean; + i18n: LocalizerType; + onChangeValue: (value: string) => void; + value: string; +}; + +/** + * Group titles must have fewer than MAX_GRAPHEME_COUNT glyphs. Ideally, we'd use the + * `maxLength` property on inputs, but that doesn't account for glyphs that are more than + * one UTF-16 code units. For example: `'💩💩'.length === 4`. + * + * This component effectively implements a "max grapheme length" on an input. + * + * At a high level, this component handles two methods of input: + * + * - `onChange`. *Before* the value is changed (in `onKeyDown`), we save the value and the + * cursor position. Then, in `onChange`, we see if the new value is too long. If it is, + * we revert the value and selection. Otherwise, we fire `onChangeValue`. + * + * - `onPaste`. If you're pasting something that will fit, we fall back to normal browser + * behavior, which calls `onChange`. If you're pasting something that won't fit, it's a + * noop. + */ +export const GroupTitleInput = forwardRef( + ({ i18n, disabled = false, onChangeValue, value }, ref) => { + const innerRef = useRef(null); + const valueOnKeydownRef = useRef(value); + const selectionStartOnKeydownRef = useRef(value.length); + + return ( + { + const inputEl = innerRef.current; + if (!inputEl) { + return; + } + + valueOnKeydownRef.current = inputEl.value; + selectionStartOnKeydownRef.current = inputEl.selectionStart || 0; + }} + onChange={() => { + const inputEl = innerRef.current; + if (!inputEl) { + return; + } + + const newValue = inputEl.value; + if (grapheme.count(newValue) <= MAX_GRAPHEME_COUNT) { + onChangeValue(newValue); + } else { + inputEl.value = valueOnKeydownRef.current; + inputEl.selectionStart = selectionStartOnKeydownRef.current; + inputEl.selectionEnd = selectionStartOnKeydownRef.current; + } + }} + onPaste={(event: ClipboardEvent) => { + const inputEl = innerRef.current; + if (!inputEl) { + return; + } + + const selectionStart = inputEl.selectionStart || 0; + const selectionEnd = + inputEl.selectionEnd || inputEl.selectionStart || 0; + const textBeforeSelection = value.slice(0, selectionStart); + const textAfterSelection = value.slice(selectionEnd); + + const pastedText = event.clipboardData.getData('Text'); + + const newGraphemeCount = + grapheme.count(textBeforeSelection) + + grapheme.count(pastedText) + + grapheme.count(textAfterSelection); + + if (newGraphemeCount > MAX_GRAPHEME_COUNT) { + event.preventDefault(); + } + }} + placeholder={i18n('setGroupMetadata__group-name-placeholder')} + ref={multiRef(ref, innerRef)} + type="text" + value={value} + /> + ); + } +); diff --git a/ts/components/Tooltip.tsx b/ts/components/Tooltip.tsx index f87e6566b..d3e67b112 100644 --- a/ts/components/Tooltip.tsx +++ b/ts/components/Tooltip.tsx @@ -6,6 +6,7 @@ import classNames from 'classnames'; import { noop } from 'lodash'; import { Manager, Reference, Popper } from 'react-popper'; import { Theme, themeClassName } from '../util/theme'; +import { multiRef } from '../util/multiRef'; type EventWrapperPropsType = { children: React.ReactNode; @@ -50,22 +51,7 @@ const TooltipEventWrapper = React.forwardRef< { - wrapperRef.current = el; - - // This is a simplified version of [what React does][0] to set a ref. - // [0]: https://github.com/facebook/react/blob/29b7b775f2ecf878eaf605be959d959030598b07/packages/react-reconciler/src/ReactFiberCommitWork.js#L661-L677 - if (typeof ref === 'function') { - ref(el); - } else if (ref) { - // I believe the types for `ref` are wrong in this case, as `ref.current` should - // not be `readonly`. That's why we do this cast. See [the React source][1]. - // [1]: https://github.com/facebook/react/blob/29b7b775f2ecf878eaf605be959d959030598b07/packages/shared/ReactTypes.js#L78-L80 - // eslint-disable-next-line no-param-reassign - (ref as React.MutableRefObject).current = el; - } - }} + ref={multiRef(ref, wrapperRef)} > {children} diff --git a/ts/components/leftPane/LeftPaneSetGroupMetadataHelper.tsx b/ts/components/leftPane/LeftPaneSetGroupMetadataHelper.tsx index 9b3740e50..5ec513ebd 100644 --- a/ts/components/leftPane/LeftPaneSetGroupMetadataHelper.tsx +++ b/ts/components/leftPane/LeftPaneSetGroupMetadataHelper.tsx @@ -11,6 +11,7 @@ import { AvatarInput } from '../AvatarInput'; import { Alert } from '../Alert'; import { Spinner } from '../Spinner'; import { Button } from '../Button'; +import { GroupTitleInput } from '../GroupTitleInput'; export type LeftPaneSetGroupMetadataPropsType = { groupAvatar: undefined | ArrayBuffer; @@ -113,15 +114,11 @@ export class LeftPaneSetGroupMetadataHelper extends LeftPaneHelper< onChange={setComposeGroupAvatar} value={this.groupAvatar} /> - { - setComposeGroupName(event.target.value); - }} - placeholder={i18n('setGroupMetadata__group-name-placeholder')} + i18n={i18n} + onChangeValue={setComposeGroupName} ref={focusRef} - type="text" value={this.groupName} /> diff --git a/ts/test-both/util/grapheme_test.ts b/ts/test-both/util/grapheme_test.ts new file mode 100644 index 000000000..3f84f760c --- /dev/null +++ b/ts/test-both/util/grapheme_test.ts @@ -0,0 +1,49 @@ +// Copyright 2021 Signal Messenger, LLC +// SPDX-License-Identifier: AGPL-3.0-only + +import { assert } from 'chai'; + +import { count } from '../../util/grapheme'; + +describe('grapheme utilities', () => { + describe('count', () => { + it('returns the number of extended graphemes in a string (not necessarily the length)', () => { + // These tests modified [from iOS][0]. + // [0]: https://github.com/signalapp/Signal-iOS/blob/800930110b0386a4c351716c001940a3e8fac942/Signal/test/util/DisplayableTextFilterTest.swift#L40-L71 + + // Plain text + assert.strictEqual(count(''), 0); + assert.strictEqual(count('boring text'), 11); + assert.strictEqual(count('Bokmål'), 6); + + // Emojis + assert.strictEqual(count('💩💩💩'), 3); + assert.strictEqual(count('👩‍❤️‍👩'), 1); + assert.strictEqual(count('🇹🇹🌼🇹🇹🌼🇹🇹'), 5); + assert.strictEqual(count('🇹🇹'), 1); + assert.strictEqual(count('🇹🇹 '), 2); + assert.strictEqual(count('👌🏽👌🏾👌🏿'), 3); + assert.strictEqual(count('😍'), 1); + assert.strictEqual(count('👩🏽'), 1); + assert.strictEqual(count('👾🙇💁🙅🙆🙋🙎🙍'), 8); + assert.strictEqual(count('🐵🙈🙉🙊'), 4); + assert.strictEqual(count('❤️💔💌💕💞💓💗💖💘💝💟💜💛💚💙'), 15); + assert.strictEqual(count('✋🏿💪🏿👐🏿🙌🏿👏🏿🙏🏿'), 6); + assert.strictEqual(count('🚾🆒🆓🆕🆖🆗🆙🏧'), 8); + assert.strictEqual(count('0️⃣1️⃣2️⃣3️⃣4️⃣5️⃣6️⃣7️⃣8️⃣9️⃣🔟'), 11); + assert.strictEqual(count('🇺🇸🇷🇺🇦🇫🇦🇲'), 4); + assert.strictEqual(count('🇺🇸🇷🇺🇸 🇦🇫🇦🇲🇸'), 7); + assert.strictEqual(count('🇺🇸🇷🇺🇸🇦🇫🇦🇲'), 5); + assert.strictEqual(count('🇺🇸🇷🇺🇸🇦'), 3); + assert.strictEqual(count('１２３'), 3); + + // Normal diacritic usage + assert.strictEqual(count('Příliš žluťoučký kůň úpěl ďábelské ódy.'), 39); + + // Excessive diacritics + assert.strictEqual(count('Z͑ͫ̓ͪ̂ͫ̽͏̴̙̤̞͉͚̯̞̠͍A̴̵̜̰͔ͫ͗͢L̠ͨͧͩ͘G̴̻͈͍͔̹̑͗̎̅͛́Ǫ̵̹̻̝̳͂̌̌͘'), 5); + assert.strictEqual(count('H҉̸̧͘͠A͢͞V̛̛I̴̸N͏̕͏G҉̵͜͏͢ ̧̧́T̶̛͘͡R̸̵̨̢̀O̷̡U͡҉B̶̛͢͞L̸̸͘͢͟É̸ ̸̛͘͏R͟È͠͞A̸͝Ḑ̕͘͜I̵͘҉͜͞N̷̡̢͠G̴͘͠ ͟͞T͏̢́͡È̀X̕҉̢̀T̢͠?̕͏̢͘͢'), 28); + assert.strictEqual(count('L̷̳͔̲͝Ģ̵̮̯̤̩̙͍̬̟͉̹̘̹͍͈̮̦̰̣͟͝O̶̴̮̻̮̗͘͡!̴̷̟͓͓'), 4); + }); + }); +}); diff --git a/ts/util/grapheme.ts b/ts/util/grapheme.ts new file mode 100644 index 000000000..46aac86bf --- /dev/null +++ b/ts/util/grapheme.ts @@ -0,0 +1,13 @@ +// Copyright 2021 Signal Messenger, LLC +// SPDX-License-Identifier: AGPL-3.0-only + +export function count(str: string): number { + const segments = new Intl.Segmenter().segment(str); + const iterator = segments[Symbol.iterator](); + + let result = -1; + for (let done = false; !done; result += 1) { + done = Boolean(iterator.next().done); + } + return result; +} diff --git a/ts/util/lint/exceptions.json b/ts/util/lint/exceptions.json index dcabd4d5c..2f6a29f6c 100644 --- a/ts/util/lint/exceptions.json +++ b/ts/util/lint/exceptions.json @@ -14730,6 +14730,33 @@ "updated": "2020-11-17T23:29:38.698Z", "reasonDetail": "Doesn't touch the DOM." }, + { + "rule": "React-useRef", + "path": "ts/components/GroupTitleInput.js", + "line": " const innerRef = react_1.useRef(null);", + "lineNumber": 47, + "reasonCategory": "usageTrusted", + "updated": "2021-03-05T16:51:54.214Z", + "reasonDetail": "Used to handle an element. Only updates the value and selection state." + }, + { + "rule": "React-useRef", + "path": "ts/components/GroupTitleInput.js", + "line": " const valueOnKeydownRef = react_1.useRef(value);", + "lineNumber": 48, + "reasonCategory": "usageTrusted", + "updated": "2021-03-05T16:51:54.214Z", + "reasonDetail": "Only stores a string." + }, + { + "rule": "React-useRef", + "path": "ts/components/GroupTitleInput.js", + "line": " const selectionStartOnKeydownRef = react_1.useRef(value.length);", + "lineNumber": 49, + "reasonCategory": "usageTrusted", + "updated": "2021-03-05T16:51:54.214Z", + "reasonDetail": "Only stores a number." + }, { "rule": "jQuery-$(", "path": "ts/components/Intl.js", @@ -14830,7 +14857,7 @@ "rule": "React-useRef", "path": "ts/components/Tooltip.js", "line": " const wrapperRef = react_1.default.useRef(null);", - "lineNumber": 19, + "lineNumber": 20, "reasonCategory": "usageTrusted", "updated": "2020-12-04T00:11:08.128Z", "reasonDetail": "Used to add (and remove) event listeners." diff --git a/ts/util/multiRef.ts b/ts/util/multiRef.ts new file mode 100644 index 000000000..76768d021 --- /dev/null +++ b/ts/util/multiRef.ts @@ -0,0 +1,22 @@ +// Copyright 2021 Signal Messenger, LLC +// SPDX-License-Identifier: AGPL-3.0-only + +import { Ref } from 'react'; + +export function multiRef(...refs: Array>): (topLevelRef: T) => void { + return (el: T) => { + refs.forEach(ref => { + // This is a simplified version of [what React does][0] to set a ref. + // [0]: https://github.com/facebook/react/blob/29b7b775f2ecf878eaf605be959d959030598b07/packages/react-reconciler/src/ReactFiberCommitWork.js#L661-L677 + if (typeof ref === 'function') { + ref(el); + } else if (ref) { + // I believe the types for `ref` are wrong in this case, as `ref.current` should + // not be `readonly`. That's why we do this cast. See [the React source][1]. + // [1]: https://github.com/facebook/react/blob/29b7b775f2ecf878eaf605be959d959030598b07/packages/shared/ReactTypes.js#L78-L80 + // eslint-disable-next-line no-param-reassign + (ref as React.MutableRefObject).current = el; + } + }); + }; +}