From c6879d75afc500bb731c5cff9f3802749b16066a Mon Sep 17 00:00:00 2001
From: "msftbot[bot]" <48340428+msftbot[bot]@users.noreply.github.com>
Date: Wed, 4 Mar 2020 20:01:01 +0000
Subject: [PATCH] Make Korean IME input more consistent (#4796)
## Summary of the Pull Request
Korean IME was not working correctly due to way we were clearing the input buffer inside of `TSFInputControl`. We wanted to clear our input buffer and tell TSF to clear its input buffer as well when we receive a `CompositionCompleted` event. This works fine in some IME languages such as Chinese and Japanese. However, Korean IME composes characters differently in such a way where we can't tell TSF to clear their buffer during a `CompositionCompleted` event because it would clear the character that triggered the `CompositionCompleted` event in the first place.
The solution in this PR is to keep our `_inputBuffer` intact until the user presses Enter or Esc, in which case we clear our buffer and the TSF buffer. I've chosen these two keys because it seems to make sense to clear the buffer after text is sent to the terminal with Enter, and Esc usually means to cancel a current composition anyway.
This means we need to keep track of our last known "Composition Start Point", which is represented by `_activeTextStart`. Whenever we complete a composition, we'll send the portion of the input buffer between `_activeTextStart` and the end of the input buffer to the terminal. Then, we'll update `_activeTextStart` to be the end of the input buffer so that the next time we send text to the terminal, we'll only send the portion of our buffer that's "active".
## PR Checklist
* [x] Closes #4226
* [x] CLA signed
* [x] Tests added/passed
## Validation Steps Performed
Manual testing with Chinese, Japanese, and Korean IME.
---
.../TerminalControl/TSFInputControl.cpp | 65 ++++++++++++-------
.../TerminalControl/TSFInputControl.h | 4 +-
.../TerminalControl/TSFInputControl.idl | 1 +
src/cascadia/TerminalControl/TermControl.cpp | 6 ++
4 files changed, 50 insertions(+), 26 deletions(-)
diff --git a/src/cascadia/TerminalControl/TSFInputControl.cpp b/src/cascadia/TerminalControl/TSFInputControl.cpp
index a51969f2d..0d8a86864 100644
--- a/src/cascadia/TerminalControl/TSFInputControl.cpp
+++ b/src/cascadia/TerminalControl/TSFInputControl.cpp
@@ -1,4 +1,4 @@
-// Copyright (c) Microsoft Corporation.
+// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.
#include "pch.h"
@@ -18,7 +18,8 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
{
TSFInputControl::TSFInputControl() :
_editContext{ nullptr },
- _inComposition{ false }
+ _inComposition{ false },
+ _activeTextStart{ 0 }
{
InitializeComponent();
@@ -92,6 +93,28 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
}
}
+ // Method Description:
+ // - Clears the input buffer and tells the text server to clear their buffer as well.
+ // Also clears the TextBlock and sets the active text starting point to 0.
+ // Arguments:
+ // -
+ // Return Value:
+ // -
+ void TSFInputControl::ClearBuffer()
+ {
+ if (!_inputBuffer.empty())
+ {
+ TextBlock().Text(L"");
+ const auto bufLen = ::base::ClampedNumeric(_inputBuffer.length());
+ _inputBuffer.clear();
+ _editContext.NotifyFocusLeave();
+ _editContext.NotifyTextChanged({ 0, bufLen }, 0, { 0, 0 });
+ _editContext.NotifyFocusEnter();
+ _activeTextStart = 0;
+ _inComposition = false;
+ }
+ }
+
// Method Description:
// - Handler for LayoutRequested event by CoreEditContext responsible
// for returning the current position the IME should be placed
@@ -271,20 +294,15 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
// -
void TSFInputControl::_textUpdatingHandler(CoreTextEditContext sender, CoreTextTextUpdatingEventArgs const& args)
{
- const auto text = args.Text();
+ const auto incomingText = args.Text();
const auto range = args.Range();
try
{
- Canvas().Visibility(Visibility::Visible);
-
- const auto length = ::base::ClampSub(range.EndCaretPosition, range.StartCaretPosition);
_inputBuffer = _inputBuffer.replace(
range.StartCaretPosition,
- length,
- text);
-
- TextBlock().Text(_inputBuffer);
+ ::base::ClampSub(range.EndCaretPosition, range.StartCaretPosition),
+ incomingText);
// If we receive tabbed IME input like emoji, kaomojis, and symbols, send it to the terminal immediately.
// They aren't composition, so we don't want to wait for the user to start and finish a composition to send the text.
@@ -292,6 +310,12 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
{
_SendAndClearText();
}
+ else
+ {
+ Canvas().Visibility(Visibility::Visible);
+ const auto text = _inputBuffer.substr(range.StartCaretPosition, range.EndCaretPosition - range.StartCaretPosition + 1);
+ TextBlock().Text(text);
+ }
// Notify the TSF that the update succeeded
args.Result(CoreTextTextUpdatingResult::Succeeded);
@@ -306,29 +330,22 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
}
// Method Description:
- // - Sends the currently held text in the input buffer to the parent and
- // clears the input buffer and text block for the next round of input.
- // Then hides the text block control until the next time text received.
+ // - Send the portion of the textBuffer starting at _activeTextStart to the end of the buffer.
+ // Then clear the TextBlock and hide it until the next time text is received.
// Arguments:
// -
// Return Value:
// -
void TSFInputControl::_SendAndClearText()
{
- // call event handler with data handled by parent
- _compositionCompletedHandlers(_inputBuffer);
+ const auto text = _inputBuffer.substr(_activeTextStart, _inputBuffer.length() - _activeTextStart);
+
+ _compositionCompletedHandlers(text);
+
+ _activeTextStart = _inputBuffer.length();
- // clear the buffer for next round
- const auto bufferLength = ::base::ClampedNumeric(_inputBuffer.length());
- _inputBuffer.clear();
TextBlock().Text(L"");
- // Leaving focus before NotifyTextChanged seems to guarantee that the next
- // composition will send us a CompositionStarted event.
- _editContext.NotifyFocusLeave();
- _editContext.NotifyTextChanged({ 0, bufferLength }, 0, { 0, 0 });
- _editContext.NotifyFocusEnter();
-
// hide the controls until text input starts again
Canvas().Visibility(Visibility::Collapsed);
}
diff --git a/src/cascadia/TerminalControl/TSFInputControl.h b/src/cascadia/TerminalControl/TSFInputControl.h
index f463444c0..f57925b2c 100644
--- a/src/cascadia/TerminalControl/TSFInputControl.h
+++ b/src/cascadia/TerminalControl/TSFInputControl.h
@@ -36,11 +36,10 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
void NotifyFocusEnter();
void NotifyFocusLeave();
+ void ClearBuffer();
void Close();
- static void OnCompositionChanged(Windows::UI::Xaml::DependencyObject const&, Windows::UI::Xaml::DependencyPropertyChangedEventArgs const&);
-
// -------------------------------- WinRT Events ---------------------------------
TYPED_EVENT(CurrentCursorPosition, TerminalControl::TSFInputControl, TerminalControl::CursorPositionEventArgs);
TYPED_EVENT(CurrentFontInfo, TerminalControl::TSFInputControl, TerminalControl::FontInfoEventArgs);
@@ -72,6 +71,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
std::wstring _inputBuffer;
bool _inComposition;
+ size_t _activeTextStart;
void _SendAndClearText();
};
}
diff --git a/src/cascadia/TerminalControl/TSFInputControl.idl b/src/cascadia/TerminalControl/TSFInputControl.idl
index d97ebcf20..0722f6d5a 100644
--- a/src/cascadia/TerminalControl/TSFInputControl.idl
+++ b/src/cascadia/TerminalControl/TSFInputControl.idl
@@ -27,6 +27,7 @@ namespace Microsoft.Terminal.TerminalControl
void NotifyFocusEnter();
void NotifyFocusLeave();
+ void ClearBuffer();
void Close();
}
diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp
index 148f56401..df35e490f 100644
--- a/src/cascadia/TerminalControl/TermControl.cpp
+++ b/src/cascadia/TerminalControl/TermControl.cpp
@@ -741,6 +741,12 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
}
}
+ if (vkey == VK_ESCAPE ||
+ vkey == VK_RETURN)
+ {
+ TSFInputControl().ClearBuffer();
+ }
+
// If the terminal translated the key, mark the event as handled.
// This will prevent the system from trying to get the character out
// of it and sending us a CharacterReceived event.