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

Terminal hangs or prints repetitive characters when using Japanese IME #14349

Closed
hnakamur opened this issue Nov 6, 2022 · 18 comments · Fixed by #17067
Closed

Terminal hangs or prints repetitive characters when using Japanese IME #14349

hnakamur opened this issue Nov 6, 2022 · 18 comments · Fixed by #17067
Assignees
Labels
Area-Input Related to input processing (key presses, mouse, etc.) Help Wanted We encourage anyone to jump in on these. In-PR This issue has a related PR Issue-Task It's a feature request, but it doesn't really need a major design. Needs-Tag-Fix Doesn't match tag requirements Product-Terminal The new Windows Terminal.

Comments

@hnakamur
Copy link

hnakamur commented Nov 6, 2022


Important

📌 plan of record: move the console tsf implementation to the Terminal codebase


Windows Terminal version

1.15.2875.0

Windows build number

10.0.22621.755

Other Software

WSL2 Ubuntu 22.04.1 LTS (version 2204.1.23.0)

Steps to reproduce

Input some Japanese characters using Microsoft IME at the right side edge of the terminal window.

Expected Behavior

Characters are converted correctly.

Actual Behavior

Sometimes repetitive characters are inputted, sometimes the terminal hangs or crashes.

2022-11-06-12-46-12.mp4
2022-11-06-13-03-10.mp4
@hnakamur hnakamur added 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 labels Nov 6, 2022
@hnakamur
Copy link
Author

hnakamur commented Nov 6, 2022

The same problem occurs with PowerShell version 5.1.22621.608.

2022-11-06-13-35-41.mp4

@lhecker
Copy link
Member

lhecker commented Nov 7, 2022

Input some Japanese characters using Microsoft IME at the right side edge of the terminal window.

Just to be sure: You're saying this never happens if you are not at the right side edge? Because if that's true, then that would narrow down the issue quite a lot and make fixing it much easier.

@carlos-zamora carlos-zamora added Area-Input Related to input processing (key presses, mouse, etc.) Product-Terminal The new Windows Terminal. and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Nov 7, 2022
@carlos-zamora carlos-zamora added this to the Backlog milestone Nov 7, 2022
@hnakamur
Copy link
Author

hnakamur commented Nov 8, 2022

I tested this again and found out this also happens when I am not at the right side edge.

Steps to reproduce:
Input some Japanese texts and delete them and repeat that.

2022-11-08-12-36-32.mp4

@lhecker lhecker modified the milestones: Backlog, Terminal v1.18 Nov 8, 2022
@lhecker lhecker self-assigned this Nov 8, 2022
@hnakamur
Copy link
Author

I uninstalled version 1.15.2875.0 and installed https://github.com/microsoft/terminal/releases/download/v1.14.2281.0/Microsoft.WindowsTerminal_Win11_1.14.2282.0_8wekyb3d8bbwe.msixbundle and it seems the problem go away.

@dogatana
Copy link

dogatana commented Feb 1, 2023

Unfortunately, 1.16 still has this issue.
As a workaround, I'm using 1.14 now, but sometimes it's updated to 1.15 automatically.
And I cannot re-install 1.14 onto 1.15 with the following message.

アプリをインストールできませんでした。エラー メッセージ: API Logging data because access was denied for file: \?\C:\Program Files\WindowsApps\Microsoft.WindowsTerminal_1.14.2281.0_x64__8wekyb3d8bbwe\CascadiaCode.ttf, user SID: S-1-5-18 からのエラー 0x0 が発生したため、AppX 展開操作に失敗しました (0x80070005)

Although this is a Japanese message, I hope you can get information from this. 😅

Every time this happens, I need to uninstall 1.15, install 1.14 and recover setting.json.

I checked the settings, but I cloud not found the setting about auto-update.
Any suggestions are most welcome.

@dogatana
Copy link

dogatana commented Feb 2, 2023

1.17 has this too.
I should use the following workaround.

https://superuser.com/questions/1676001/windows-stop-killing-my-terminals-for-an-update

@lhecker lhecker added the Help Wanted We encourage anyone to jump in on these. label Sep 7, 2023
@lhecker
Copy link
Member

lhecker commented Sep 7, 2023

I genuinely can't figure out how to reproduce the issue. I've tried writing some variation of "どうぞよろしくお願いいたします。" over and over again for about 15min an hour and it didn't happen.

But clearly this regression is caused by either of these two commits, both of which first shipped in v1.15:

I don't think either of the two commits themselves is "buggy", but rather that they make bugs in TSF (Text Services Framework) obvious. This is because backspacing like in hnakamur's videos causes the TermControl::_CharacterHandler event handler to be called (the character is \b). This will cause TSFInputControl().ClearBuffer() to be called which should instruct TSF to flush its entire buffer and reset its state.

I think what's happening here is that TSF is either:

  • ignoring this request
  • getting into a bad state
  • running out of process in the "Text Input Management Service" and that may result in yet-unknown, subtle race conditions? 1

2c922e1 could have triggered the regression because it drops clamping the signed range values from TSF. I believe at the time I didn't consider it possible that TSF would actually pass either negative indices or a range where StartCaretPosition is greater than EndCaretPosition. I'll be more cautious in the code I write from now on.

ed800dc could have triggered the regression because it changed this:

_activeTextStart = ::base::ClampMin(_activeTextStart, ::base::ClampedNumeric<size_t>(range.StartCaretPosition));

into this:

_activeTextStart = std::min(_activeTextStart, _inputBuffer.size());

I think the latter is technically more correct 2, but if TSF is in a bad state where the internal state didn't properly reset, then this may be off.

Footnotes

  1. Microservices in your OS! Got a problem? Now it's a problem and a distributed problem! 100% more value! /s

  2. The design of the TSF API implies that it is random access. When the buffer contains "foo" and the _activeTextStart is 3 (= at the end), then we shouldn't move the start to 0 just because TSF replaced the entire string with "foobar". It should remain at 3, because the active string is "bar". It's unclear how TSF is supposed to be used in this case because TSF lacks documentation and is not designed to be used for a terminal.

@lhecker
Copy link
Member

lhecker commented Sep 7, 2023

I've marked #15938 as closing this issue, but I'll have to unfortunately rely on you to validate it, as I was entirely unable to reproduce the issue. 😟

@hnakamur
Copy link
Author

hnakamur commented Sep 7, 2023

Can I download the binary built with #15938 included?
Then I can validate it.

As of 1.17.11461.0, I just reproduce the issue just by inputting and deleting "日本語を入力して" three times.
I'm using the US keyboard on Japanese Windows 11.

@zadjii-msft
Copy link
Member

@hnakamur sure can!

You should be able to go to: https://dev.azure.com/ms/terminal/_build/results?buildId=494745&view=artifacts&pathAsName=false&type=publishedArtifacts

Then navigate to the build-x64-release > _unpackaged > WindowsTerminalDev_0.0.1.0_x64.zip file. (or for a shortcut: I think this link will work?)

If there are subsequent commits, then this link:
image

will take you to the latest CI run

@hnakamur
Copy link
Author

hnakamur commented Sep 7, 2023

Thanks, your shortcut link works.
I'm afraid the issue is not resolved.

2023-09-07.22-53-27.mp4

@hnakamur
Copy link
Author

hnakamur commented Sep 7, 2023

I review my Windows settings and I found 「以前のバージョンのMicrosoft IMEを使う」 was on.
I changed it off and the issue seems go away on both Windows Terminal 1.17.11461.0 and WindowsTerminalDev_0.0.1.0_x64.zip!
image

@hnakamur
Copy link
Author

hnakamur commented Sep 7, 2023

I tried again with「以前のバージョンのMicrosoft IMEを使う」 on, the issue was reproduced.
So it seems there is a problem in the combination of the old version Microsoft IME and Windows Terminal.

As for me, I am fine with the current version of Microsoft IME.

@lhecker
Copy link
Member

lhecker commented Sep 7, 2023

Hmm, I think that's fairly good news for us (Windows Terminal) at least, because it implies that this is indeed a bug in TSF. (An application shouldn't notice a difference in behavior between the legacy and new IME after all.) Would it be fine for us to only support the new IME?

BTW I tried to reproduce the issue with the legacy IME and again couldn't get it to reproduce it. 😟
I don't speak Japanese, so maybe I'm just not using the right input? I saw you write "kokodenennkannn" and tried that and that didn't do anything for me. Do you use any specific input that reliably reproduces the issue for you after a few tries?

@hnakamur
Copy link
Author

hnakamur commented Sep 7, 2023

I suppose the support for the old IME would be nice, but I am fine without it.

I was using 「日本語を入力して」"nihongowonyuuryokusite" to reproduce the issue.

@lhecker
Copy link
Member

lhecker commented Sep 7, 2023

I was using 「日本語を入力して」"nihongowonyuuryokusite" to reproduce the issue.

Thanks! But I still can't get it to reproduce the issue. 🥲 I just genuinely can't figure this out.
And I noticed I haven't said this before: I'm sorry that it took so long for us to try and address this bug.

I suppose the support for the old IME would be nice, but I am fine without it.

One of my main tasks has been to improve the Internationalization of this project for a while now and I don't intend to give up just yet. 🙂 I was hoping to unify conhost's IME with the one in Windows Terminal at some point. conhost's IME implementation has two benefits: It uses TSF directly (without the WinRT CoreTextEditContext wrapper; less code = less bugs), and it writes the text during composition directly into the terminal viewport, so when you finish the composition it doesn't look quite as weird as it does now:

image
vs.
image

(I prefer the latter a lot.)

I'm just not sure when I can get to that. There's a lot of remaining Internationalization bugs in this project after all... But there's a good chance that using TSF directly without the WinRT wrapper will allow us to properly support the legacy IME and fix all those weird edge cases.
(Korean and Vietnamese in particular have a very difficult IME. By fixing those 2 IMEs, I broke the Japanese legacy IME. 😟 The conhost TSF/IME implementation never had those issues to begin with, thanks to its tight TSF integration.)


@zadjii-msft Should we close this bug, or repurpose it to track the Japanese legacy IME being broken? I think the latter might be good. This issue would get fixed properly if we ever manage to integrate the conhost TSF implementation into WT.

@lhecker lhecker removed the In-PR This issue has a related PR label Sep 11, 2023
@tobiBarth
Copy link

When using tab to cycle through items I noticed that when the text would hit the right side of the terminal it would start bugging out for consecutive uses of tab. It seems to print previous lines again although this is purely visual.

japanese.text.bug.mp4

Win Terminal version: 1.17.11461.0

@zadjii-msft zadjii-msft added Issue-Task It's a feature request, but it doesn't really need a major design. and removed Issue-Bug It either shouldn't be doing this or needs an investigation. labels Feb 7, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot added the In-PR This issue has a related PR label Feb 21, 2024
github-merge-queue bot pushed a commit that referenced this issue Apr 18, 2024
Next in the popular series of minor refactorings:
Out with the old, in with the new!

This PR removes all of the existing TSF code, both for conhost and
Windows Terminal. conhost's TSF implementation was awful:
It allocated an entire text buffer _per line_ of input.
Additionally, its implementation spanned a whopping 40 files and
almost 5000 lines of code. Windows Terminal's implementation was
absolutely fine in comparison, but it was user unfriendly due to
two reasons: Its usage of the `CoreTextServices` WinRT API indirectly
meant that it used a non-transitory TSF document, which is not the
right choice for a terminal. A `TF_SS_TRANSITORY` document (-context)
indicates to TSF that it cannot undo a previously completed composition
which is exactly what we need: Once composition has completed we send
the result to the shell and we cannot undo this later on.
The WinRT API does not allow us to use `TF_SS_TRANSITORY` and so it's
unsuitable for our application. Additionally, the implementation used
XAML to render the composition instead of being part of our text
renderer, which resulted in the text looking weird and hard to read.

The new implementation spans just 8 files and is ~1000 lines which
should make it significantly easier to maintain. The architecture is
not particularly great, but it's certainly better than what we had.
The implementation is almost entirely identical between both conhost
and Windows Terminal and thus they both also behave identical.
It fixes an uncountable number of subtle bugs in the conhost TSF
implementation, as it failed to check for status codes after calls.
It also adds several new features, like support for wavy underlines
(as used by the Japanese IME), dashed underlines (the default for
various languages now, like Vietnamese), colored underlines,
colored foreground/background controlled by the IME, and more!

I have tried to replicate the following issues and have a high
confidence that they're resolved now:
Closes #1304
Closes #3730
Closes #4052
Closes #5007  (as it is not applicable anymore)
Closes #5110
Closes #6186
Closes #6192
Closes #13805
Closes #14349
Closes #14407
Closes #16180

For the following issues I'm not entirely sure if it'll fix it,
but I suspect it's somewhat likely:
#13681
#16305
#16817

Lastly, there's one remaining bug that I don't know how to resolve.
However, that issue also plagues conhost and Windows Terminal
right now, so it's at least not a regression:
* Press Win+. (emoji picker) and close it
* Move the window around
* Press Win+.

This will open the emoji picker at the old window location.
It also occurs when the cursor moves within the window.
While this is super annoying, I could not find a way to fix it.

## Validation Steps Performed
* See the above closed issues
* Use Vietnamese Telex and type "xin choaf"
  Results in "xin chào" ✅
* Use the MS Japanese IME and press Alt+`
  Toggles between the last 2 modes ✅
* Use the MS Japanese IME, type "kyouhaishaheiku", and press Space
  * The text is converted, underlined and the first part is
    doubly underlined ✅
  * Left/Right moves between the 3 segments ✅
  * Home/End moves between start/end ✅
  * Esc puts a wavy line under the current segment ✅
* Use the Korean IME, type "gksgks"
  This results in "한한" ✅
* Use the Korean IME, type "gks", and press Right Ctrl
  Opens a popup which allows you to navigate with Arrow/Tab keys ✅
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Tag-Fix Doesn't match tag requirements label Apr 18, 2024
@lhecker
Copy link
Member

lhecker commented Apr 18, 2024

We just published a major update to our IME implementation in the nightly Canary branch. It was rewritten from the ground up and has tons of improvements! If you're interested in trying it out, you can get it here: https://aka.ms/terminal-canary-installer
If you already have the Canary build installed, you can use this link to force an update.

If you encounter any issues or have any suggestions, or if you simply like/dislike the changes, please let us know! Thank you for bearing with us. 😊

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.) Help Wanted We encourage anyone to jump in on these. In-PR This issue has a related PR Issue-Task It's a feature request, but it doesn't really need a major design. Needs-Tag-Fix Doesn't match tag requirements Product-Terminal The new Windows Terminal.
Projects
None yet
6 participants