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.

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

IME fails very frequently in the v0.10 release #5054

Closed
etern opened this issue Mar 21, 2020 · 12 comments · Fixed by #5109
Closed

IME fails very frequently in the v0.10 release #5054

etern opened this issue Mar 21, 2020 · 12 comments · Fixed by #5109
Assignees
Labels
Area-Input Related to input processing (key presses, mouse, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Priority-1 A description (P1) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Milestone

Comments

@etern
Copy link

etern commented Mar 21, 2020

In the middle of typing, IME frequently fails, I tried but still can't figure out how the failure is triggered, but only if I type and delete for a while, it always happen, so I get the chance to record a gif:
terminal_IME_fail

From the gif, you can see the first two characters were typed correctly, after that, nothing from the IME can be typed again.

Environment

Microsoft Windows [版本 10.0.18362.476]
The gif is recorded in v0.10.761.0, and it still happens in v0.10.781.0
I have been using Terminal v0.9, which works fine, the error comes with v0.10

Steps to reproduce

Can't figure out exactly, keep typing and deleting in a prompt for a while, it always happen

Expected behavior

characters from IME can be typd onto terminal

Actual behavior

IME fails

@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Mar 21, 2020
@leonMSFT leonMSFT added Area-Input Related to input processing (key presses, mouse, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-1 A description (P1) Product-Terminal The new Windows Terminal. labels Mar 23, 2020
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Mar 23, 2020
@leonMSFT leonMSFT self-assigned this Mar 23, 2020
@leonMSFT leonMSFT added this to the Terminal v1.0 milestone Mar 23, 2020
@leonMSFT
Copy link
Contributor

Just curious, what's the version of Windows you're running?

@leonMSFT
Copy link
Contributor

leonMSFT commented Mar 23, 2020

@etern I've managed to half repro the issue you're seeing (my work machine can't repro, but my personal PC can repro part of the bug), so I'm not sure if I'm fixing the exact issue you're seeing. I have a potential fix in the branch named dev/lelian/imeweird. If you have a chance to take a look and try it out, it would help me know whether or not I'm encountering the same bug you are!

@etern
Copy link
Author

etern commented Mar 23, 2020

@leonMSFT My Windows version is 10.0.18362.476.

If you can fix this, you might know how it can be triggered (hope it's not random behavior). I can verify the trigger method on my PC, so we know if you fixed the right problem.

If that doesn't help, I will try to build your branch by this weekend. Since I'm not familiar with the code, I don't have the confidence to make it work

@leonMSFT
Copy link
Contributor

I believe the trigger is to start composition with something like "wei'yua" but backspace delete your composition completely before you complete the composition. After deleting the composition, try creating a composition again and nothing will show up but the composition picker box will show and update as if you're typing normally.

@etern
Copy link
Author

etern commented Mar 24, 2020

Yes, "delete composition completely" can reproduce the bug every time, you get it right. Please merge the fix.

@leonMSFT
Copy link
Contributor

Actually, I have one more follow up question: after deleting the composition "wei'yuan", does the starting letter 'w' reappear?

@etern
Copy link
Author

etern commented Mar 24, 2020

Yes, the first character reappear

@etern
Copy link
Author

etern commented Mar 24, 2020

One more thing, after the deletion triggers the bug, I just Enter to a new line, IME works.

But sometimes, when I switch from other place to terminal window, IME fails and can't be restored by Enter to a new prompt. In this occasion, I have to close the tab and open a new one.

These two problems may be related because they all come with v0.10, I don't know how to reproduce this, see the Enter operation in the gif:
terminal_ime_fail

@skyline75489
Copy link
Collaborator

I can reproduce this easily on my laptop with Windows 10.0.18363.720 and Windows Terminal at a338227 under two different circumstances:

  • First, backspace deleting all composition completely, basically the same as @etern pointed out. After deleting the composition the letter 'w' will reappear.
  • Second, unfocus WT window while the IME prompt is displayed. The entire composition (wei'ruan) will reappear.

@leonMSFT Your fix seems to be working with both cases. But the reappearing is still there, which I suppose is something you are aware of.

@leonMSFT
Copy link
Contributor

Yeah the reappearing character is a particularly annoying problem. For some reason, I'm receiving a CompositionCompleted event before getting the TextUpdate that the last character was deleted. This causes me to believe the Composition is finished, so I send what's in our buffer, but then right after sending it, I get an update saying the last character is deleted, so then I delete it from the buffer.

@leonMSFT
Copy link
Contributor

I've made a PR to at the very least allow y'all to continue typing in Chinese IME, but I haven't come up with a way to resolve the reappearing initial character due to not understanding why the text server API must act this way 😢

@ghost ghost closed this as completed in #5109 Mar 25, 2020
@ghost ghost removed the In-PR This issue has a related PR label Mar 25, 2020
ghost pushed a commit that referenced this issue Mar 25, 2020
## Summary of the Pull Request
This PR fixes an out of bounds access when deleting composition during Chinese IME. What's happening is that we're receiving a CompositionCompleted before receiving the TextUpdate to tell us to delete the last character in the composition. This creates two problems for us:
1. The final character gets sent to the Terminal when it should have been deleted.
2. `_activeTextStart` gets set to `_inputBuffer.length()` when sending the character to the terminal, so when the `TextUpdate` comes right after the `CompositionCompleted` event, `_activeTextStart` is out of sync.

This PR fixes the second issue, by updating `_activeTextStart` during a `TextUpdate` in case we run into this issue. 
The first issue is trickier to resolve since we assume that if the text server api tells us a composition is completed, we should send what we have. It'll be tracked here: #5110.
At the very least, this PR will let users continue to type in Chinese IME without it breaking, but it will still be annoying to see the first letter of your composition reappear after deleting it.

## PR Checklist
* [x] Closes #5054
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [x] Tests added/passed

## Validation Steps Performed
Play around with Chinese IME deleting and composing, and play around with Korean and Japanese IME to see that it still works as expected.
@ghost ghost added the Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. label Mar 25, 2020
@ghost
Copy link

ghost commented Apr 22, 2020

🎉This issue was addressed in #5109, which has now been successfully released as Windows Terminal Preview v0.11.1121.0.:tada:

Handy links:

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Input Related to input processing (key presses, mouse, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Priority-1 A description (P1) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants