Clean up TSFInputControl a bit #13677
Conversation
lhecker
force-pushed
the
dev/lhecker/tsf-cleanup
branch
from
e158d89 to
87a660e
Compare
| _editContext.NotifyFocusEnter(); | ||
| _focused = true; |
There was a problem hiding this comment.
Nothing in this class sets _editContext to nullptr. 🤔
There was a problem hiding this comment.
Hmm. Good point. We'll totally blow up in the ctor long before we get here.
| if (!_inputBuffer.empty()) | ||
| { | ||
| _SendAndClearText(); | ||
| } |
There was a problem hiding this comment.
_inputBuffer.empty() doesn't really matter. What matters is whether _activeTextStart == _inputBuffer.size(), because _activeTextStart indicates the actual start of the text we currently edit.
| // GH#5054: Pressing backspace might move the caret before the _activeTextStart. | ||
| _activeTextStart = ::base::ClampMin(_activeTextStart, ::base::ClampedNumeric<size_t>(range.StartCaretPosition)); |
There was a problem hiding this comment.
I don't think "some machines will fire [...]". As far as I can see all machines do that, but it depends on the circumstances. I've thus changed the comment to just be about backspacing.
lhecker
added
the
Area-CodeHealth
|
nit: name in imperative style remember: you have a stacked PR, don't bot-merge this! |
lhecker
changed the title
| _editContext.NotifyFocusLeave(); | ||
| _editContext.NotifyTextChanged({ 0, bufLen }, 0, { 0, 0 }); | ||
| _editContext.NotifyTextChanged({ 0, INT32_MAX }, 0, _selection); |
There was a problem hiding this comment.
What does passing INT32_MAX here instead of _inputBuffer.length get us? Is there
Oh right we're clearing the buffer. Yea, blow away everything. makes sense.
| const auto textRequested = _inputBuffer.substr(range.StartCaretPosition, length); | ||
|
|
||
| args.Request().Text(textRequested); | ||
| const auto range = args.Request().Range(); |
There was a problem hiding this comment.
we don't need to clamp here anymore?
This commit builds directly on the changes made in #13677 and fixes: * TSF resetting to AlphaNumeric ("ASCII") input mode when pressing enter * Vietnamese IME not composing a new word after pressing whitespace, etc. Closes #11479 Closes #13398 ## Validation Steps Performed * Japanese IME (Full-Width Katakana) Typing "saitama" produces "サイタマ" ✅ * Korean IME Typing "gksrmf" produces "한글" ✅ * Vietnamese IME Typing "xin chaof" produces "xin chào" ✅ * Emoji Picker (Win+.) ✅
|
🎉 Handy links: |
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
This commit builds directly on the changes made in microsoft#13677 and fixes: * TSF resetting to AlphaNumeric ("ASCII") input mode when pressing enter * Vietnamese IME not composing a new word after pressing whitespace, etc. Closes microsoft#11479 Closes microsoft#13398 ## Validation Steps Performed * Japanese IME (Full-Width Katakana) Typing "saitama" produces "サイタマ" ✅ * Korean IME Typing "gksrmf" produces "한글" ✅ * Vietnamese IME Typing "xin chaof" produces "xin chào" ✅ * Emoji Picker (Win+.) ✅ (cherry picked from commit ed800dc) Service-Card-Id: 84832470 Service-Version: 1.15
While working on #13398 I felt that
TSFInputControlwasn't up to sniff.This commit is a minor cleanup of the class:
_SendAndClearTextValidation Steps Performed
Typing "saitama" produces "サイタマ" ✅
Typing "gksrmf" produces "한글" ✅
Typing "xin chaof" continues to produce broken "xin xinchaof"
(It's supposed to produce "xin chào")
✅