MM-66937 Fix broken IME handling in Find Channels modal (#35264) (#35798)

* MM-66937 Add E2E tests for bug

* MM-66937 Remove delayInputUpdate on that input to fix the bug

* Remove delayInputUpdate prop from QuickInput and SuggestionBox

* Run prettier

* Inline updateInputFromProps and remove eslint-disable that's no longer needed

* Fix snapshots

e2e-tests/playwright/lib/src/ime.ts

@@ -0,0 +1,184 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See LICENSE.txt for license information.
+
+import type {Page} from '@playwright/test';
+
+/**
+ * This is Korean for "Test if Hangul is typed well".
+ *
+ * When testing manually, if you want to type this on a US English Qwerty keyboard with your OS language set to Korean,
+ * this can be typed as "gksrmfdl wkf dlqfurehlsmswl xptmxm"
+ */
+export const koreanTestPhrase = '한글이 잘 입력되는지 테스트';
+
+/**
+ * Simulates typing a phrase containing Korean Hangul characters using an Input Method Editor that composes characters
+ * from multiple keypresses as the user types. This isn't completely realistic because it's missing keyboard events, but
+ * it's sufficient to reproduce composition bugs like MM-66937.
+ *
+ * This finishes by ending composition on the final character typed, so this can't be used to test anything involving
+ * the character that's actively being composed (such as autocompleting partial characters).
+ *
+ * Note: This only works on Chrome-based browsers because it relies on the Chrome Devtools Protocol (CDP).
+ */
+export async function typeKoreanWithIme(page: Page, text: string) {
+    const client = await page.context().newCDPSession(page);
+
+    for (const decomposed of decomposeKorean(text)) {
+        if (decomposed.jama) {
+            // # Type the individual jamo
+
+            // The first one is typed as-is
+            await client.send('Input.imeSetComposition', {
+                selectionStart: -1,
+                selectionEnd: -1,
+                text: decomposed.jama[0],
+            });
+
+            // When you type the second one, the IME combines the two into the resulting character. Instead of reversing
+            // the math, we can do that by concatenating them and then normalizing the Unicode.
+            await client.send('Input.imeSetComposition', {
+                selectionStart: -1,
+                selectionEnd: -1,
+                text: (decomposed.jama[0] + decomposed.jama[1]).normalize('NFKD'),
+            });
+
+            // For the third one, we can't normalize the Unicode because there are some initial and final jama which
+            // look identical and normalize to the same value, so just use the original character
+            await client.send('Input.imeSetComposition', {
+                selectionStart: -1,
+                selectionEnd: -1,
+                text: decomposed.character,
+            });
+
+            // # End composition by inserting the complete character into the textbox
+            // Technically, this doesn't actually happen until the user types something else or clicks on the textbox,
+            // but it's cleaner to do now since we don't currently support searching for partially composed characters.
+            await client.send('Input.insertText', {
+                text: decomposed.character,
+            });
+        } else {
+            // # Insert the character
+            await client.send('Input.insertText', {
+                text: decomposed.character,
+            });
+        }
+    }
+
+    await client.detach();
+}
+
+function decomposeKorean(text: string): Array<{character: string; jama?: string[]}> {
+    // Adapted from https://useless-factor.blogspot.com/2007/08/unicode-implementers-guide-part-3.html and
+    // https://web.archive.org/web/20190512031142/http://www.programminginkorean.com/programming/hangul-in-unicode/composing-syllables-in-unicode/
+
+    // All Korean Hangul characters/syllables are in this range of Unicode
+    const hangulStart = 0xac00;
+    const hangulEnd = 0xd7a3;
+
+    // Hangul characters are made up of an initial consonant, a medial vowel, and an optional final vowel
+    const initial = [
+        'ㄱ',
+        'ㄲ',
+        'ㄴ',
+        'ㄷ',
+        'ㄸ',
+        'ㄹ',
+        'ㅁ',
+        'ㅂ',
+        'ㅃ',
+        'ㅅ',
+        'ㅆ',
+        'ㅇ',
+        'ㅈ',
+        'ㅉ',
+        'ㅊ',
+        'ㅋ',
+        'ㅌ',
+        'ㅍ',
+        'ㅎ',
+    ];
+    const medial = [
+        'ㅏ',
+        'ㅐ',
+        'ㅑ',
+        'ㅒ',
+        'ㅓ',
+        'ㅔ',
+        'ㅕ',
+        'ㅖ',
+        'ㅗ',
+        'ㅘ',
+        'ㅙ',
+        'ㅚ',
+        'ㅛ',
+        'ㅜ',
+        'ㅝ',
+        'ㅞ',
+        'ㅟ',
+        'ㅠ',
+        'ㅡ',
+        'ㅢ',
+        'ㅣ',
+    ];
+    const final = [
+        '',
+        'ㄱ',
+        'ㄲ',
+        'ㄳ',
+        'ㄴ',
+        'ㄵ',
+        'ㄶ',
+        'ㄷ',
+        'ㄹ',
+        'ㄺ',
+        'ㄻ',
+        'ㄼ',
+        'ㄽ',
+        'ㄾ',
+        'ㄿ',
+        'ㅀ',
+        'ㅁ',
+        'ㅂ',
+        'ㅄ',
+        'ㅅ',
+        'ㅆ',
+        'ㅇ',
+        'ㅈ',
+        'ㅊ',
+        'ㅋ',
+        'ㅌ',
+        'ㅍ',
+        'ㅎ',
+    ];
+
+    const result = [];
+
+    for (let i = 0; i < text.length; i++) {
+        const character = text[i];
+        const code = character.charCodeAt(i);
+
+        if (code >= hangulStart && code <= hangulEnd) {
+            // This is a Hangul character, so we can break it down into the individual constants and vowel
+            const syllableIndex = code - hangulStart;
+
+            // See the linked blog posts for more information on this math
+            const initialIndex = Math.floor(syllableIndex / (21 * 28));
+            const medialIndex = Math.floor((syllableIndex % (21 * 28)) / 28);
+            const finalIndex = syllableIndex % 28;
+
+            const jama = [];
+            jama.push(initial[initialIndex]);
+            jama.push(medial[medialIndex]);
+            if (final[finalIndex]) {
+                jama.push(final[finalIndex]);
+            }
+            result.push({character, jama});
+        } else {
+            // This is some other character, so just add it separately
+            result.push({character});
+        }
+    }
+
+    return result;
+}

e2e-tests/playwright/lib/src/index.ts

@@ -6,6 +6,7 @@ export {testConfig} from './test_config';
 export {baseGlobalSetup} from './global_setup';
 export {TestBrowser} from './browser_context';
 export {getBlobFromAsset, getFileFromAsset} from './file';
+export {koreanTestPhrase, typeKoreanWithIme} from './ime';
 export {duration, wait} from './util';
 
 export {

e2e-tests/playwright/specs/functional/channels/search/find_channels_korean.spec.ts

@@ -0,0 +1,63 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See LICENSE.txt for license information.
+
+import {expect, koreanTestPhrase, test, typeKoreanWithIme} from '@mattermost/playwright-lib';
+
+test('Find Channels modal handles Korean IME input correctly', async ({pw, browserName}) => {
+    test.skip(browserName !== 'chromium', 'The API used to test this is only available in Chrome');
+
+    const {adminClient, user, team} = await pw.initSetup();
+
+    // # Create a channel named after the test phrase
+    const fullMatchChannel = pw.random.channel({
+        teamId: team.id,
+        name: 'full-match-channel',
+        displayName: koreanTestPhrase,
+    });
+    await adminClient.createChannel(fullMatchChannel);
+
+    // # And create a channel matching part of the test phrase
+    const partialMatchChannel = pw.random.channel({
+        teamId: team.id,
+        name: 'partial-match-channel',
+        displayName: koreanTestPhrase.substring(0, 10),
+    });
+    await adminClient.createChannel(partialMatchChannel);
+
+    // # Log in and go to Channels
+    const {channelsPage, page} = await pw.testBrowser.login(user);
+
+    await channelsPage.goto();
+    await channelsPage.toBeVisible();
+
+    // # Open the channel switcher
+    await channelsPage.sidebarLeft.findChannelButton.click();
+    await channelsPage.findChannelsModal.toBeVisible();
+
+    // # Focus the input
+    const input = channelsPage.findChannelsModal.input;
+    await input.focus();
+
+    const firstHalf = koreanTestPhrase.substring(0, 5);
+    const secondHalf = koreanTestPhrase.substring(5);
+
+    // # Type the first half of the test phrase
+    await typeKoreanWithIme(page, firstHalf);
+
+    // * Verify that characters are correctly composed and weren't doubled up
+    await expect(input).toHaveValue(firstHalf);
+
+    // * Verify that both channels are visible
+    await expect(page.getByRole('option', {name: fullMatchChannel.display_name, exact: true})).toBeVisible();
+    await expect(page.getByRole('option', {name: partialMatchChannel.display_name, exact: true})).toBeVisible();
+
+    // # Type the second half of the test phrase
+    await typeKoreanWithIme(page, secondHalf);
+
+    // * Verify that characters are correctly composed and weren't doubled up
+    await expect(input).toHaveValue(koreanTestPhrase);
+
+    // * Verify that the first channel is still visible but that the second is not
+    await expect(page.getByRole('option', {name: fullMatchChannel.display_name, exact: true})).toBeVisible();
+    await expect(page.getByRole('option', {name: partialMatchChannel.display_name, exact: true})).not.toBeAttached();
+});

e2e-tests/playwright/specs/functional/channels/search/search_box_korean.spec.ts

@@ -0,0 +1,42 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See LICENSE.txt for license information.
+
+import {expect, koreanTestPhrase, test, typeKoreanWithIme} from '@mattermost/playwright-lib';
+
+test('Search box handles Korean IME correctly', async ({pw, browserName}) => {
+    test.skip(browserName !== 'chromium', 'The API used to test this is only available in Chrome');
+
+    const {userClient, user, team} = await pw.initSetup();
+
+    // # Create a channel named after the test phrase
+    const testChannel = pw.random.channel({
+        teamId: team.id,
+        name: 'korean-test-channel',
+        displayName: koreanTestPhrase,
+    });
+    await userClient.createChannel(testChannel);
+
+    // # Log in a user in new browser context
+    const {channelsPage, page} = await pw.testBrowser.login(user);
+
+    // # Visit a default channel page
+    await channelsPage.goto();
+    await channelsPage.toBeVisible();
+
+    // # Open the search UI
+    await channelsPage.globalHeader.openSearch();
+
+    const {searchInput} = channelsPage.searchBox;
+    await searchInput.focus();
+
+    // # Type into the textbox
+    const searchText = 'in:' + koreanTestPhrase.substring(0, 3);
+    await typeKoreanWithIme(page, searchText);
+
+    // * Verify that the text was typed correctly into the search box
+    await expect(searchInput).toHaveValue(searchText);
+
+    // * Verify that the channel is suggested
+    await expect(channelsPage.searchBox.selectedSuggestion).toBeVisible();
+    await expect(channelsPage.searchBox.selectedSuggestion).toHaveText(testChannel.display_name);
+});

webapp/channels/src/components/add_user_to_channel_modal/__snapshots__/add_user_to_channel_modal.test.tsx.snap

@@ -73,7 +73,6 @@ exports[`components/AddUserToChannelModal should match snapshot 1`] = `
         <Connect(SuggestionBox)
           className="form-control focused"
           completeOnTab={false}
-          delayInputUpdate={true}
           listComponent={[Function]}
           listPosition="bottom"
           maxLength="64"

webapp/channels/src/components/add_user_to_channel_modal/add_user_to_channel_modal.tsx

@@ -270,7 +270,6 @@ export default class AddUserToChannelModal extends React.PureComponent<Props, St
                 providers={this.suggestionProviders}
                 listPosition='bottom'
                 completeOnTab={false}
-                delayInputUpdate={true}
                 openWhenEmpty={false}
             />
         );

webapp/channels/src/components/quick_input/quick_input.tsx

@@ -11,12 +11,6 @@ import WithTooltip from 'components/with_tooltip';
 
 export type Props = {
 
-    /**
-     * Whether to delay updating the value of the textbox from props. Should only be used
-     * on textboxes that to properly compose CJK characters as the user types.
-     */
-    delayInputUpdate?: boolean;
-
     /**
      * An optional React component that will be used instead of an HTML input when rendering
      */
@@ -92,7 +86,6 @@ const defaultClearableTooltipText = (
 // A component that can be used to make controlled inputs that function properly in certain
 // environments (ie. IE11) where typing quickly would sometimes miss inputs
 export const QuickInput = React.memo(({
-    delayInputUpdate = false,
     value = '',
     clearable = false,
     autoFocus,
@@ -119,23 +112,11 @@ export const QuickInput = React.memo(({
     }, []);
 
     useEffect(() => {
-        const updateInputFromProps = () => {
+        if (!inputRef.current || inputRef.current.value === value) {
-            if (!inputRef.current || inputRef.current.value === value) {
+            return;
-                return;
-            }
-
-            inputRef.current.value = value;
-        };
-
-        if (delayInputUpdate) {
-            requestAnimationFrame(updateInputFromProps);
-        } else {
-            updateInputFromProps();
         }
 
-        /* eslint-disable-next-line react-hooks/exhaustive-deps --
+        inputRef.current.value = value;
-         * This 'useEffect' should run only when 'value' prop changes.
-         **/
     }, [value]);
 
     const setInputRef = useCallback((input: HTMLInputElement) => {

webapp/channels/src/components/quick_switch_modal/__snapshots__/quick_switch_modal.test.tsx.snap

@@ -52,7 +52,6 @@ exports[`components/QuickSwitchModal should match snapshot 1`] = `
       aria-label="quick switch input"
       className="form-control focused"
       completeOnTab={false}
-      delayInputUpdate={true}
       forceSuggestionsWhenBlur={true}
       id="quickSwitchInput"
       listComponent={[Function]}

webapp/channels/src/components/quick_switch_modal/quick_switch_modal.tsx

@@ -235,7 +235,6 @@ export class QuickSwitchModal extends React.PureComponent<Props, State> {
                         providers={providers}
                         completeOnTab={false}
                         spellCheck='false'
-                        delayInputUpdate={true}
                         openWhenEmpty={true}
                         onSuggestionsReceived={this.handleSuggestionsReceived}
                         forceSuggestionsWhenBlur={true}

webapp/channels/src/components/search_bar/search_bar.tsx

@@ -170,7 +170,6 @@ const SearchBar: React.FunctionComponent<Props> = (props: Props): JSX.Element =>
                     dateComponent={SuggestionDate}
                     providers={suggestionProviders}
                     type='search'
-                    delayInputUpdate={true}
                     renderDividers={['all']}
                     clearable={true}
                     onClear={props.handleClear}