Add check for empty compose string#29396
Conversation
gspencergoog
force-pushed
the
fix_compose_delete
branch
from
2104b3d to
2124c51
Compare
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat. If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
cbracken
left a comment
cbracken
left a comment
There was a problem hiding this comment.
Discussed offline, but I think what we should do is change this code:
engine/shell/platform/windows/text_input_manager_win32.cc
Lines 160 to 162 in fd992c6
to be:
if (compose_length < 0) {
return std::nullopt;
}i.e. just change the <= to a <.
I think TextInputManager::GetString() (and by extension GetComposingString and GetResultString) should return nullopt only when calling them is non-sensical (i.e. when the IME is not active, when the IME context is invalid, or when the window handle is invalid; in all other cases, we should return the composing string.
After the last backspace though, the composing string is empty and we should return it as such.
In terms of testing, we could create a small struct of trampoline functions for the Win32 IMM API calls here, and for the tests mock them out. GetString is private, but GetComposingString and GetResultString are both public and don't make too many API calls.
gspencergoog
force-pushed
the
fix_compose_delete
branch
from
2124c51 to
e8dd2da
Compare
2 participants
Description
Fixes the problem where deleting a composing string leaves the last character of the composing string in the text field.
Related Issues
Tests