Fix Hangul typing does not work properly

[jungeonkim]

compositionstart event may fire in the meantime because compositionend event is being processed asynchronously.

[bwindels]

Hi @jungeonkim, thank you for working on this!

We'll need you to sign off on your code, as described in here, just writing this here will do:

Signed-off-by: Your Name <your@email.example.org>

As for the code, it looks somewhat sensible, but I'm not sure what is going wrong when you hit your problem. Could you elaborate a bit how this fixes the problem?
What browser version are you using?
What kind of input events are you seeing between compositionstart and compositionend? Could you provide a screenshot of typing your sequence in this tool to get a log of all the events?

Perhaps once clarified, we can add more comments in the code to explain how and what this fixes, so future modifications don't undo it.

Thank you!

[jungeonkim]

I've confirmed that this issue occurs in Chrome 80 (Windows or macOS), electron app (Windows or macOS).

In most Hangul IME, compositionstart event occurs immediately after compositionend event.
However, onCompositionEnd's anonymous function is called after onCompositionStart due to use setTimeout.

[bwindels]

Thank you for the extra information. I'm still not 100% sure how this fix works though.

Concretely, I'm interpreting your explanation as something is going wrong in this sequence, here simplified with only two compositions:

1. compositionstart is fired, _isIMEComposing is set to true
2. some typing occurs, but DOM updates are blocked in _onInput to not interfere with the native composition support showing what is being composed.
3. compositionend is fired, _isIMEComposing is set to false, and a timer is being set to call _onInput in case the browser doesn't fire an input event afterwards.
4. compositionstart is fired again, _isIMEComposing is set to true
5. _onInput from the timer is now run, but _isIMEComposing is set, so it exits before doing anything
6. typing occurs, compostion is updated but DOM updates are blocked.
7. compositionend is fired (for the last time when you stop typing), _isIMEComposing is set to false, and a timer is being set to call _onInput.
8. _onInput from the second timer is now run, and _isIMEComposing is not set, so it updates the DOM.

Is the problem that the DOM does not get updated between the two compositions and therefore the first composition is lost or something? Is that what your added condition addresses, to not block the deferred DOM updates between two compositions to not loose the first composition?

Sorry for the detailed questioning, I'm applying extra caution here, as I don't want to break anything for other input languages or browsers, and would like to document the problem in the code so we don't regress in the future.

[jungeonkim]

In the current implementation, If keep typing in Hangul only unless typing a character that doesn't use Hangul IME, the model is not updated because _isIMEComposing is true in _onInput via compositionend and _onInput by input event.
So look at the gif image I commented first.
Hangul characters typed after the non-hangul character (in this case, space) have not been updated in the model. and posted as missing.

[bwindels]

Right, I understand now, thank you. I think that the PR, as it stands, might regress element-hq/element-web#10646.

I'm not immediately sure how to solve this. It sounds like:

• Chrome fires doesn't fire an input after the compositionend event, as addressed in New composer: force model update after IME composition finishes #3364
• (Some) Japanese IMEs cancel the composition if the DOM is modified during a composition, as addressed in New composer: don't update model while doing IME compositions #3357
• (Some) Hangul IMEs fire the next compositionstart so fast the the mitigation for the above problems (a fake deferred input to update the model) makes that the deferred input is always ignored, as we're already again in a new composition session. The model is not reliably updated and when submitting the message, parts are missing.

None of this is being made easier by the fact that it is very hard to test all the combinations of this.

I'll have a think about what we can do about this...

[jungeonkim]

I have confirmed that the japanese ime on macOS works fine, but I haven't tested on other cases.

[bwindels]

@jungeonkim Could you try if using Promise.resolve().then(...) on line 206 instead of setTimeout(..., 0) fixes the problem for you (without the change you made in this PR)? E.g.:

Promise.resolve().then(() => {
    this._onInput({inputType: "insertCompositionText"});
});

[bwindels]

I have confirmed that the japanese ime on macOS works fine, but I haven't tested on other cases.

While I'd still prefer a less intrusive solution, that is good to know 👍

[jungeonkim]

Promise.resolve().then(() => {

It fixes the problem on macOS. I am trying to test it in other environments too.

[jungeonkim]

I have confirmed that it solves the problem on Windows and macOS (chrome, electron app) and fixed the PR.

[bwindels]

Great, thank you!!