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

Japanese IME doesn't preserve the text that it overwrites #6186

Closed
j4james opened this issue May 25, 2020 · 0 comments · Fixed by #17067
Closed

Japanese IME doesn't preserve the text that it overwrites #6186

j4james opened this issue May 25, 2020 · 0 comments · Fixed by #17067
Labels
Area-i18n Internationalization issues (e.g.: non-US input handling doesn't work) Area-Input Related to input processing (key presses, mouse, etc.) Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues In-PR This issue has a related PR Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Priority-2 A description (P2) Product-Conhost For issues in the Console codebase
Milestone

Comments

@j4james
Copy link
Collaborator

j4james commented May 25, 2020

Environment

Windows build number: Version 10.0.18362.720
Windows Terminal version (if applicable): Commit 2af722b

Steps to reproduce

  1. Install the Japanese language pack.
  2. Open a cmd shell in conhost (built from source).
  3. Open the Properties dialog and change the font to MS Gothic.
  4. Switch the input language to Japanese Microsoft IME.
  5. Make sure the IME option is initially Half-Width Alphanumeric.
  6. Type abcdefgh
  7. Move the cursor back to the start of the line.
  8. Switch the IME option to Hiragana.
  9. Type sushi then press Esc to close the IME.
  10. Press Esc again to cancel the added text.

Expected behavior

In the process of typing sushi, the latin characters abcdef will be temporarily overwritten, but after step 9, I'd expect the ef restored, and only the abcd overwritten with すし. After step 10, I'd expect to see all the latin characters restored.

This is what those two steps looks like in the cmd shell in my current version of Windows.

image

Actual behavior

After step 9, the ef characters have been replaced with blanks, and after step 10 they're still blank.

image

What is weird is that I tried reverting the code back to the first public commit and it still seems to have that problem. I would have thought the cmd shell that shipped with Windows was much more recent than that. So I don't know if maybe it's just something about the build process that I'm getting wrong.

@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 May 25, 2020
@zadjii-msft zadjii-msft added Area-Input Related to input processing (key presses, mouse, etc.) Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Conhost For issues in the Console codebase labels May 26, 2020
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label May 26, 2020
@zadjii-msft zadjii-msft added this to the 21H1 milestone May 26, 2020
@zadjii-msft zadjii-msft removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label May 27, 2020
@zadjii-msft zadjii-msft modified the milestones: Windows vNext, 22H1 Jan 4, 2022
@zadjii-msft zadjii-msft modified the milestones: 22H1, Terminal v1.14 Feb 2, 2022
@zadjii-msft zadjii-msft added the Area-i18n Internationalization issues (e.g.: non-US input handling doesn't work) label Feb 3, 2022
@zadjii-msft zadjii-msft modified the milestones: Terminal v1.14, 22H2 Mar 10, 2022
@zadjii-msft zadjii-msft modified the milestones: 22H2, Backlog Jul 5, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot added the In-PR This issue has a related PR label Apr 16, 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-i18n Internationalization issues (e.g.: non-US input handling doesn't work) Area-Input Related to input processing (key presses, mouse, etc.) Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues In-PR This issue has a related PR Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Priority-2 A description (P2) Product-Conhost For issues in the Console codebase
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants