Skip to content

Commit

Permalink
Clean up TSFInputControl a bit (microsoft#13677)
Browse files Browse the repository at this point in the history 
While working on microsoft#13398 I felt that `TSFInputControl` wasn't up to sniff.
This commit is a minor cleanup of the class:
* default member initializers
* Simplified use of STL classes which already perform boundary checks
* Correctly check text buffer emptiness in `_SendAndClearText`
* Track selection range as mandated by the API

## Validation Steps Performed
* Japanese IME (Full-Width Katakana)
  Typing "saitama" produces "サイタマ" ✅
* Korean IME
  Typing "gksrmf" produces "한글" ✅
* Vietnamese IME
  Typing "xin chaof" continues to produce broken "xin xinchaof"
  (It's supposed to produce "xin chào")
* Emoji Picker (Win+.)
  ✅

(cherry picked from commit 2c922e1)
Service-Card-Id: 85034073
Service-Version: 1.15
  • Loading branch information
@lhecker @DHowett
lhecker authored and DHowett committed Aug 16, 2022
1 parent f5aa18a commit 479f625
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 80 deletions.
97 changes: 32 additions & 65 deletions src/cascadia/TerminalControl/TSFInputControl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@
#include "TSFInputControl.h"
#include "TSFInputControl.g.cpp"

#include <Utils.h>

using namespace winrt::Windows::Foundation;
using namespace winrt::Windows::Graphics::Display;
using namespace winrt::Windows::UI::Core;
Expand All @@ -16,17 +14,7 @@ using namespace winrt::Windows::UI::Xaml;

namespace winrt::Microsoft::Terminal::Control::implementation
{
TSFInputControl::TSFInputControl() :
_editContext{ nullptr },
_inComposition{ false },
_activeTextStart{ 0 },
_focused{ false },
_currentTerminalCursorPos{ 0, 0 },
_currentCanvasWidth{ 0.0 },
_currentTextBlockHeight{ 0.0 },
_currentTextBounds{ 0, 0, 0, 0 },
_currentControlBounds{ 0, 0, 0, 0 },
_currentWindowBounds{ 0, 0, 0, 0 }
TSFInputControl::TSFInputControl()
{
InitializeComponent();

Expand Down Expand Up @@ -83,11 +71,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
// - <none>
void TSFInputControl::NotifyFocusEnter()
{
if (_editContext != nullptr)
{
_editContext.NotifyFocusEnter();
_focused = true;
}
_editContext.NotifyFocusEnter();
_focused = true;
}

// Method Description:
Expand All @@ -99,11 +84,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
// - <none>
void TSFInputControl::NotifyFocusLeave()
{
if (_editContext != nullptr)
{
_editContext.NotifyFocusLeave();
_focused = false;
}
_editContext.NotifyFocusLeave();
_focused = false;
}

// Method Description:
Expand All @@ -117,14 +99,13 @@ namespace winrt::Microsoft::Terminal::Control::implementation
{
if (!_inputBuffer.empty())
{
TextBlock().Text(L"");
const auto bufLen = ::base::ClampedNumeric<int32_t>(_inputBuffer.length());
_inputBuffer.clear();
_selection = {};
_activeTextStart = 0;
_editContext.NotifyFocusLeave();
_editContext.NotifyTextChanged({ 0, bufLen }, 0, { 0, 0 });
_editContext.NotifyTextChanged({ 0, INT32_MAX }, 0, _selection);
_editContext.NotifyFocusEnter();
_activeTextStart = 0;
_inComposition = false;
TextBlock().Text({});
}
}

Expand Down Expand Up @@ -303,12 +284,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
void TSFInputControl::_compositionCompletedHandler(CoreTextEditContext sender, const CoreTextCompositionCompletedEventArgs& /*args*/)
{
_inComposition = false;

// only need to do work if the current buffer has text
if (!_inputBuffer.empty())
{
_SendAndClearText();
}
_SendAndClearText();
}

// Method Description:
Expand Down Expand Up @@ -336,16 +312,13 @@ namespace winrt::Microsoft::Terminal::Control::implementation
// - <none>
void TSFInputControl::_textRequestedHandler(CoreTextEditContext sender, const CoreTextTextRequestedEventArgs& args)
{
// the range the TSF wants to know about
const auto range = args.Request().Range();

try
{
const auto textEnd = ::base::ClampMin<size_t>(range.EndCaretPosition, _inputBuffer.length());
const auto length = ::base::ClampSub<size_t>(textEnd, range.StartCaretPosition);
const auto textRequested = _inputBuffer.substr(range.StartCaretPosition, length);

args.Request().Text(textRequested);
const auto range = args.Request().Range();
const auto text = _inputBuffer.substr(
range.StartCaretPosition,
range.EndCaretPosition - range.StartCaretPosition);
args.Request().Text(text);
}
CATCH_LOG();
}
Expand All @@ -360,8 +333,9 @@ namespace winrt::Microsoft::Terminal::Control::implementation
// - args: CoreTextSelectionRequestedEventArgs for providing data for the SelectionRequested event. Not used in method.
// Return Value:
// - <none>
void TSFInputControl::_selectionRequestedHandler(CoreTextEditContext sender, const CoreTextSelectionRequestedEventArgs& /*args*/)
void TSFInputControl::_selectionRequestedHandler(CoreTextEditContext sender, const CoreTextSelectionRequestedEventArgs& args)
{
args.Request().Selection(_selection);
}

// Method Description:
Expand All @@ -374,8 +348,10 @@ namespace winrt::Microsoft::Terminal::Control::implementation
// - args: CoreTextSelectionUpdatingEventArgs for providing data for the SelectionUpdating event. Not used in method.
// Return Value:
// - <none>
void TSFInputControl::_selectionUpdatingHandler(CoreTextEditContext sender, const CoreTextSelectionUpdatingEventArgs& /*args*/)
void TSFInputControl::_selectionUpdatingHandler(CoreTextEditContext sender, const CoreTextSelectionUpdatingEventArgs& args)
{
_selection = args.Selection();
args.Result(CoreTextSelectionUpdatingResult::Succeeded);
}

// Method Description:
Expand All @@ -388,24 +364,18 @@ namespace winrt::Microsoft::Terminal::Control::implementation
// - <none>
void TSFInputControl::_textUpdatingHandler(CoreTextEditContext sender, const CoreTextTextUpdatingEventArgs& args)
{
const auto incomingText = args.Text();
const auto range = args.Range();

try
{
// When a user deletes the last character in their current composition, some machines
// will fire a CompositionCompleted before firing a TextUpdating event that deletes the last character.
// The TextUpdating will have a lower StartCaretPosition, so in this scenario, _activeTextStart
// needs to update to be the StartCaretPosition.
// A known issue related to this behavior is that the last character that's deleted from a composition
// will get sent to the terminal before we receive the TextUpdate to delete the character.
// See GH #5054.
_activeTextStart = ::base::ClampMin(_activeTextStart, ::base::ClampedNumeric<size_t>(range.StartCaretPosition));
const auto incomingText = args.Text();
const auto range = args.Range();

_inputBuffer = _inputBuffer.replace(
range.StartCaretPosition,
::base::ClampSub<size_t>(range.EndCaretPosition, range.StartCaretPosition),
range.EndCaretPosition - range.StartCaretPosition,
incomingText);
_selection = args.NewSelection();
// GH#5054: Pressing backspace might move the caret before the _activeTextStart.
_activeTextStart = ::base::ClampMin(_activeTextStart, ::base::ClampedNumeric<size_t>(range.StartCaretPosition));

// Emojis/Kaomojis/Symbols chosen through the IME without starting composition
// will be sent straight through to the terminal.
Expand All @@ -432,22 +402,19 @@ namespace winrt::Microsoft::Terminal::Control::implementation
}
}

// Method Description:
// - 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:
// - <none>
// Return Value:
// - <none>
void TSFInputControl::_SendAndClearText()
{
const auto text = _inputBuffer.substr(_activeTextStart);
if (text.empty())
{
return;
}

_CompositionCompletedHandlers(text);

_activeTextStart = _inputBuffer.length();
_activeTextStart = _inputBuffer.size();

TextBlock().Text(L"");
TextBlock().Text({});

// After we reset the TextBlock to empty string, we want to make sure
// ActualHeight reflects the respective height. It seems that ActualHeight
Expand Down
30 changes: 15 additions & 15 deletions src/cascadia/TerminalControl/TSFInputControl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ namespace winrt::Microsoft::Terminal::Control::implementation
void _textUpdatingHandler(winrt::Windows::UI::Text::Core::CoreTextEditContext sender, const winrt::Windows::UI::Text::Core::CoreTextTextUpdatingEventArgs& args);
void _formatUpdatingHandler(winrt::Windows::UI::Text::Core::CoreTextEditContext sender, const winrt::Windows::UI::Text::Core::CoreTextFormatUpdatingEventArgs& args);

void _SendAndClearText();
void _RedrawCanvas();

winrt::Windows::UI::Text::Core::CoreTextEditContext::TextRequested_revoker _textRequestedRevoker;
winrt::Windows::UI::Text::Core::CoreTextEditContext::SelectionRequested_revoker _selectionRequestedRevoker;
winrt::Windows::UI::Text::Core::CoreTextEditContext::FocusRemoved_revoker _focusRemovedRevoker;
Expand All @@ -68,22 +71,19 @@ namespace winrt::Microsoft::Terminal::Control::implementation
winrt::Windows::UI::Text::Core::CoreTextEditContext::CompositionStarted_revoker _compositionStartedRevoker;
winrt::Windows::UI::Text::Core::CoreTextEditContext::CompositionCompleted_revoker _compositionCompletedRevoker;

Windows::UI::Text::Core::CoreTextEditContext _editContext;

Windows::UI::Text::Core::CoreTextEditContext _editContext{ nullptr };
std::wstring _inputBuffer;

bool _inComposition;
size_t _activeTextStart;
void _SendAndClearText();
void _RedrawCanvas();
bool _focused;

til::point _currentTerminalCursorPos;
double _currentCanvasWidth;
double _currentTextBlockHeight;
winrt::Windows::Foundation::Rect _currentControlBounds;
winrt::Windows::Foundation::Rect _currentTextBounds;
winrt::Windows::Foundation::Rect _currentWindowBounds;
winrt::Windows::UI::Text::Core::CoreTextRange _selection{};
size_t _activeTextStart = 0;
bool _inComposition = false;
bool _focused = false;

til::point _currentTerminalCursorPos{};
double _currentCanvasWidth = 0.0;
double _currentTextBlockHeight = 0.0;
winrt::Windows::Foundation::Rect _currentControlBounds{};
winrt::Windows::Foundation::Rect _currentTextBounds{};
winrt::Windows::Foundation::Rect _currentWindowBounds{};
};
}
namespace winrt::Microsoft::Terminal::Control::factory_implementation
Expand Down

0 comments on commit 479f625

Please sign in to comment.