From f3d73defcf6a010900d21414042eb5a0feb4082e Mon Sep 17 00:00:00 2001 From: Harrison Healey Date: Sun, 15 Feb 2026 22:00:59 -0500 Subject: [PATCH] MM-66937 Fix broken IME handling in Find Channels modal (#35264) * 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 | 184 ++++++++++++++++++ e2e-tests/playwright/lib/src/index.ts | 1 + .../search/find_channels_korean.spec.ts | 63 ++++++ .../channels/search/search_box_korean.spec.ts | 42 ++++ .../add_user_to_channel_modal.test.tsx.snap | 1 - .../add_user_to_channel_modal.tsx | 1 - .../components/quick_input/quick_input.tsx | 25 +-- .../quick_switch_modal.test.tsx.snap | 1 - .../quick_switch_modal/quick_switch_modal.tsx | 1 - .../src/components/search_bar/search_bar.tsx | 1 - 10 files changed, 293 insertions(+), 27 deletions(-) create mode 100644 e2e-tests/playwright/lib/src/ime.ts create mode 100644 e2e-tests/playwright/specs/functional/channels/search/find_channels_korean.spec.ts create mode 100644 e2e-tests/playwright/specs/functional/channels/search/search_box_korean.spec.ts diff --git a/e2e-tests/playwright/lib/src/ime.ts b/e2e-tests/playwright/lib/src/ime.ts new file mode 100644 index 00000000000..4e58bb60729 --- /dev/null +++ b/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; +} diff --git a/e2e-tests/playwright/lib/src/index.ts b/e2e-tests/playwright/lib/src/index.ts index 514df2bb21d..c84a9b09087 100644 --- a/e2e-tests/playwright/lib/src/index.ts +++ b/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 { diff --git a/e2e-tests/playwright/specs/functional/channels/search/find_channels_korean.spec.ts b/e2e-tests/playwright/specs/functional/channels/search/find_channels_korean.spec.ts new file mode 100644 index 00000000000..7990282d288 --- /dev/null +++ b/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(); +}); diff --git a/e2e-tests/playwright/specs/functional/channels/search/search_box_korean.spec.ts b/e2e-tests/playwright/specs/functional/channels/search/search_box_korean.spec.ts new file mode 100644 index 00000000000..1f7ada0987b --- /dev/null +++ b/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); +}); diff --git a/webapp/channels/src/components/add_user_to_channel_modal/__snapshots__/add_user_to_channel_modal.test.tsx.snap b/webapp/channels/src/components/add_user_to_channel_modal/__snapshots__/add_user_to_channel_modal.test.tsx.snap index a3c0e9b4699..10e7e90ee6e 100644 --- a/webapp/channels/src/components/add_user_to_channel_modal/__snapshots__/add_user_to_channel_modal.test.tsx.snap +++ b/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`] = ` ); diff --git a/webapp/channels/src/components/quick_input/quick_input.tsx b/webapp/channels/src/components/quick_input/quick_input.tsx index 7a8eef609e7..a9b13e4bc71 100644 --- a/webapp/channels/src/components/quick_input/quick_input.tsx +++ b/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 */ @@ -93,7 +87,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, @@ -122,23 +115,11 @@ export const QuickInput = React.memo(({ }, []); useEffect(() => { - const updateInputFromProps = () => { - if (!inputRef.current || inputRef.current.value === value) { - return; - } - - inputRef.current.value = value; - }; - - if (delayInputUpdate) { - requestAnimationFrame(updateInputFromProps); - } else { - updateInputFromProps(); + if (!inputRef.current || inputRef.current.value === value) { + return; } - /* eslint-disable-next-line react-hooks/exhaustive-deps -- - * This 'useEffect' should run only when 'value' prop changes. - **/ + inputRef.current.value = value; }, [value]); const setInputRef = useCallback((input: HTMLInputElement) => { diff --git a/webapp/channels/src/components/quick_switch_modal/__snapshots__/quick_switch_modal.test.tsx.snap b/webapp/channels/src/components/quick_switch_modal/__snapshots__/quick_switch_modal.test.tsx.snap index b0341525742..324fb573b25 100644 --- a/webapp/channels/src/components/quick_switch_modal/__snapshots__/quick_switch_modal.test.tsx.snap +++ b/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]} diff --git a/webapp/channels/src/components/quick_switch_modal/quick_switch_modal.tsx b/webapp/channels/src/components/quick_switch_modal/quick_switch_modal.tsx index dc48a4c03a0..4ceea9bf972 100644 --- a/webapp/channels/src/components/quick_switch_modal/quick_switch_modal.tsx +++ b/webapp/channels/src/components/quick_switch_modal/quick_switch_modal.tsx @@ -230,7 +230,6 @@ export class QuickSwitchModal extends React.PureComponent { providers={providers} completeOnTab={false} spellCheck='false' - delayInputUpdate={true} openWhenEmpty={true} onSuggestionsReceived={this.handleSuggestionsReceived} forceSuggestionsWhenBlur={true} diff --git a/webapp/channels/src/components/search_bar/search_bar.tsx b/webapp/channels/src/components/search_bar/search_bar.tsx index 3f6f890a7dd..f09825cda6e 100644 --- a/webapp/channels/src/components/search_bar/search_bar.tsx +++ b/webapp/channels/src/components/search_bar/search_bar.tsx @@ -170,7 +170,6 @@ const SearchBar: React.FunctionComponent = (props: Props): JSX.Element => dateComponent={SuggestionDate} providers={suggestionProviders} type='search' - delayInputUpdate={true} clearable={true} onClear={props.handleClear} />