Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix incorrect data in compositionend event with Korean IME on IE11 #12563

Merged
merged 2 commits into from
Jun 14, 2018
Merged

Fix incorrect data in compositionend event with Korean IME on IE11 #12563

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
06524c6
Fix incorrect data in compositionend event with Korean IME on IE11 (#…
crux153 Apr 6, 2018
49a9d71
Merge branch 'master' into fix-korean-input
gaearon Jun 14, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 18 additions & 2 deletions packages/react-dom/src/events/BeforeInputEventPlugin.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,20 @@ function getDataFromCustomEvent(nativeEvent) {
return null;
}

/**
* Check if a composition event was triggered by Korean IME.
* Our fallback mode does not work well with IE's Korean IME,
* so just use native composition events when Korean IME is used.
* Although CompositionEvent.locale property is deprecated,
* it is available in IE, where our fallback mode is enabled.
*
* @param {object} nativeEvent
* @return {boolean}
*/
function isUsingKoreanIME(nativeEvent) {
return nativeEvent.locale === 'ko';
}

// Track the current IME composition status, if any.
let isComposing = false;

Expand Down Expand Up @@ -229,7 +243,7 @@ function extractCompositionEvent(
return null;
}

if (useFallbackCompositionData) {
if (useFallbackCompositionData && !isUsingKoreanIME(nativeEvent)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any reason you don't make this check a part of useFallbackCompositionData declaration itself?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since my implementation of isUsingKoreanIME depends on CompositionEvent.locale property from composition event, making it part of useFallbackCompositionData requires it to be a function that needs to be invoked every time a composition event occurs.

Current declaration of useFallbackCompositionData happens at load time as it only checks for browser. Check for Korean IME only needs to be invoked when using fallback mode, so I thought additional function invocation would be inefficient for those with modern browsers, though it will be certainly better for code readability.

// The current composition is stored statically and must not be
// overwritten while composition continues.
if (!isComposing && eventType === eventTypes.compositionStart) {
Expand Down Expand Up @@ -378,7 +392,9 @@ function getFallbackBeforeInputChars(topLevelType: TopLevelType, nativeEvent) {
}
return null;
case TOP_COMPOSITION_END:
return useFallbackCompositionData ? null : nativeEvent.data;
return useFallbackCompositionData && !isUsingKoreanIME(nativeEvent)
? null
: nativeEvent.data;
default:
return null;
}
Expand Down