Fix incorrect data in compositionend event with Korean IME on IE11 #12563
Conversation
…acebook#10217) * Add isUsingKoreanIME function to check if a composition event was triggered by Korean IME * Add Korean IME check alongside useFallbackCompositionData and disable fallback mode with Korean IME
9e1cfa2
to
06524c6
Compare
|
Are there any other languages where this is applicable? |
|
Thank you for your response. I would love to see this PR or alternative fix to the issue gets merged soon, as it is quite big problem to Korean users. Due to the strange web environments of Korea, with weird dependency to old, unfashioned ActiveX, IE's market share still exceeds 35% of total desktop traffic here. (http://gs.statcounter.com/browser-market-share/desktop/south-korea) As this issue renders rich text input of React completely unusable with Korean, it forces developers to either abandon IE11 users or throw up React totally, or just stick to plain text input. Not surprisingly, this seems to be a long issue with Facebook itself.
I think this issue happens because composition events of Korean characters may affect the following character and React's fallback mode is not able to handle it due to its implementation. Another approach to fix this issue might be a rewrite of FallbackCompositionState itself, but that would ends up with bunch of unnecessary boilerplates to other language inputs in my opinion. I thought about keeping blacklisted locales in an array, instead of checking only Korean IME, but I was not able to find any other languages with this kind of problem. I've applied a custom build of React with this fix on website I manage (with 600K monthly pageviews and 10K visitors) at the time of opening this PR, and there have been no issue reported so far. |
| @@ -229,7 +243,7 @@ function extractCompositionEvent( | |||
| return null; | |||
| } | |||
|
|
|||
| if (useFallbackCompositionData) { | |||
| if (useFallbackCompositionData && !isUsingKoreanIME(nativeEvent)) { | |||
There was a problem hiding this comment.
Is there any reason you don't make this check a part of useFallbackCompositionData declaration itself?
There was a problem hiding this comment.
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.
|
ReactDOM: size: 0.0%, gzip: 🔺+0.1% Details of bundled changes.Comparing: bc963f3...49a9d71 react-dom
Generated by 🚫 dangerJS |
|
The fix is well-contained and I don't see harm in merging this. Let's see if we'll get any other reports. |
This fixes wrong behavior with composition events when using Korean IME on IE11. (#10217)
It seems that it is fine to use native compositionend event from Korean IME, so this simply disable using fallback composition mode when using Korean IME.
Check for Korean IME is implemented using
CompositionEvent.localeproperty. Although it is deprecated according to MDN, it is still available in IE (where fallback composition mode gets enabled) and works well with detecting Korean IME.I checked the functionality of this fix manually on IE9, 10, 11, though it seems that fallback composition mode only causes problem in IE11. I think it should have no impact on environments rather than IE with Korean IME.
It also fixes other rich text editors depending on React, such as Draft.js.
Before
After
