Skip to content

Commit

Permalink
Merged PR 5984262: [Git2Git] Merged PR 5982901: Reintroduce GetQuickC…
Browse files Browse the repository at this point in the history 
…harWidth for numpad event synthesis

When we encounter clipboard input that cannot be mapped to a keyboard
key, we try our best to map it to an event. If if is an alphanumeric
character or a wide glyph (per the old width algorithm), we will pass it
through as the UnicodeChar of a key event. If it's *not* wide and *not*
alphanumeric, we will synthesize a set of Alt+NumPad events.

Those events comprise a set containing...

1. Alt Down (modifiers = LAlt, UnicodeChar = 0)
2. Numpad 0 Down/Up (modifiers = LAlt, UnicodeChar = 0)
3. Numpad 1 Down/Up (modifiers = LAlt, UnicodeChar = 0)
4. Numpad 2 Down/Up (modifiers = LAlt, UnicodeChar = 0)
5. Alt Up (modifiers = 0, UnicodeChar = [THE CHARACTER])

Because of event group 5, application developers seem to have taken a
dependency on receiving Alt Up + Character and don't seek to recompose
the original character from its numpad representation.

In pull request GH-8035 (!5394370), we stripped the old width algorithm
out and replaced it with a lookup table (finally!). Unfortunately, that
broke clipboard input for Chinese text as it was no longer considered
"Wide" for the purposes of detecting whether we should use numpad
events.

This commit introduces a version of GetQuickCharWidth that fulfills the
exact contract CharToKeyEvents needs, and doesn't answer for a codepoint
more. We'll use it in Windows to fix MSFT-32901370.

The Terminal analogue of this bug, GH-9052, is fixed by *never emitting
numpad events again.* We think this is okay because it looks like nobody
was ever handling numpad events... and that we were only using them as a
way to communicate within conhost (which we're *also* not using) and any
public exposition of event 5 as a contract was unintended.

VALIDATION
----------
I took this new implementation (with an early return) and the old
implementation and compared whether they would yield a numpad event or a
key event over the entire supported codepoint space [0000..FFFF].

They matched.

Fixes MSFT-32901370
Fixes GH-9052

Retrieved from https://microsoft.visualstudio.com os.2020 OS official/rs_wdx_dxp_windev 224751bbb061f5f59d794c2c9bdac5a9674ebde6
  • Loading branch information
@DHowett
DHowett committed Apr 27, 2021
1 parent d7f690d commit 66b9b9d
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 36 deletions.
42 changes: 27 additions & 15 deletions src/host/ut_host/ClipboardTests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -261,37 +261,43 @@ class ClipboardTests
}
}

#ifdef __INSIDE_WINDOWS
TEST_METHOD(CanConvertCharsRequiringAltGr)
{
const std::wstring wstr = L"\x20ac"; // € char U+20AC

const short keyState = VkKeyScanW(wstr[0]);
const WORD virtualKeyCode = LOBYTE(keyState);
const WORD virtualScanCode = static_cast<WORD>(MapVirtualKeyW(virtualKeyCode, MAPVK_VK_TO_VSC));

if (keyState == -1 || HIBYTE(keyState) == 0 /* no modifiers required */)
{
Log::Comment(L"This test only works on keyboard layouts where the Euro symbol exists and requires AltGr.");
Log::Result(WEX::Logging::TestResults::Skipped);
return;
}

std::deque<std::unique_ptr<IInputEvent>> events = Clipboard::Instance().TextToKeyEvents(wstr.c_str(),
wstr.size());

std::deque<KeyEvent> expectedEvents;
// should be converted to:
// 1. AltGr keydown
// 2. € keydown
// 3. € keyup
// 4. AltGr keyup
const size_t convertedSize = 4;
VERIFY_ARE_EQUAL(convertedSize, events.size());

const short keyState = VkKeyScanW(wstr[0]);
const WORD virtualKeyCode = LOBYTE(keyState);

std::deque<KeyEvent> expectedEvents;
expectedEvents.push_back({ TRUE, 1, VK_MENU, altScanCode, L'\0', (ENHANCED_KEY | LEFT_CTRL_PRESSED | RIGHT_ALT_PRESSED) });
expectedEvents.push_back({ TRUE, 1, virtualKeyCode, 0, wstr[0], (LEFT_CTRL_PRESSED | RIGHT_ALT_PRESSED) });
expectedEvents.push_back({ FALSE, 1, virtualKeyCode, 0, wstr[0], (LEFT_CTRL_PRESSED | RIGHT_ALT_PRESSED) });
expectedEvents.push_back({ TRUE, 1, virtualKeyCode, virtualScanCode, wstr[0], (LEFT_CTRL_PRESSED | RIGHT_ALT_PRESSED) });
expectedEvents.push_back({ FALSE, 1, virtualKeyCode, virtualScanCode, wstr[0], (LEFT_CTRL_PRESSED | RIGHT_ALT_PRESSED) });
expectedEvents.push_back({ FALSE, 1, VK_MENU, altScanCode, L'\0', ENHANCED_KEY });

VERIFY_ARE_EQUAL(expectedEvents.size(), events.size());

for (size_t i = 0; i < events.size(); ++i)
{
const KeyEvent currentKeyEvent = *reinterpret_cast<const KeyEvent* const>(events[i].get());
VERIFY_ARE_EQUAL(expectedEvents[i], currentKeyEvent, NoThrowString().Format(L"i == %d", i));
}
}
#endif

TEST_METHOD(CanConvertCharsOutsideKeyboardLayout)
{
Expand All @@ -301,23 +307,29 @@ class ClipboardTests
std::deque<std::unique_ptr<IInputEvent>> events = Clipboard::Instance().TextToKeyEvents(wstr.c_str(),
wstr.size());

std::deque<KeyEvent> expectedEvents;
#ifdef __INSIDE_WINDOWS
// Inside Windows, where numpad events are enabled, this generated numpad events.
// should be converted to:
// 1. left alt keydown
// 2. 1st numpad keydown
// 3. 1st numpad keyup
// 4. 2nd numpad keydown
// 5. 2nd numpad keyup
// 6. left alt keyup
const size_t convertedSize = 6;
VERIFY_ARE_EQUAL(convertedSize, events.size());

std::deque<KeyEvent> expectedEvents;
expectedEvents.push_back({ TRUE, 1, VK_MENU, altScanCode, L'\0', LEFT_ALT_PRESSED });
expectedEvents.push_back({ TRUE, 1, 0x66, 0x4D, L'\0', LEFT_ALT_PRESSED });
expectedEvents.push_back({ FALSE, 1, 0x66, 0x4D, L'\0', LEFT_ALT_PRESSED });
expectedEvents.push_back({ TRUE, 1, 0x63, 0x51, L'\0', LEFT_ALT_PRESSED });
expectedEvents.push_back({ FALSE, 1, 0x63, 0x51, L'\0', LEFT_ALT_PRESSED });
expectedEvents.push_back({ FALSE, 1, VK_MENU, altScanCode, wstr[0], 0 });
#else
// Outside Windows, without numpad events, we just emit the key with a nonzero UnicodeChar
expectedEvents.push_back({ TRUE, 1, 0, 0, wstr[0], 0 });
expectedEvents.push_back({ FALSE, 1, 0, 0, wstr[0], 0 });
#endif

VERIFY_ARE_EQUAL(expectedEvents.size(), events.size());

for (size_t i = 0; i < events.size(); ++i)
{
Expand Down
75 changes: 54 additions & 21 deletions src/interactivity/base/EventSynthesis.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,44 @@
static constexpr WORD altScanCode = 0x38;
static constexpr WORD leftShiftScanCode = 0x2A;

#ifdef __INSIDE_WINDOWS
// To reduce the risk of compatibility issues inside Windows, we're going to continue using the old
// version of GetQuickCharWidth to determine whether a character should be synthesized into numpad
// events.
static constexpr bool useNumpadEvents{ true };
#else // !defined(__INSIDE_WINDOWS)
// In Terminal, however, we will *always* use normal key events (!)
static constexpr bool useNumpadEvents{ false };
#endif // __INSIDE_WINDOWS

// Routine Description:
// - naively determines the width of a UCS2 encoded wchar (with caveats noted above)
#pragma warning(suppress : 4505) // this function will be deleted if useNumpadEvents is false
static CodepointWidth GetQuickCharWidthLegacyForNumpadEventSynthesis(const wchar_t wch) noexcept
{
if ((0x1100 <= wch && wch <= 0x115f) // From Unicode 9.0, Hangul Choseong is wide
|| (0x2e80 <= wch && wch <= 0x303e) // From Unicode 9.0, this range is wide (assorted languages)
|| (0x3041 <= wch && wch <= 0x3094) // Hiragana
|| (0x30a1 <= wch && wch <= 0x30f6) // Katakana
|| (0x3105 <= wch && wch <= 0x312c) // Bopomofo
|| (0x3131 <= wch && wch <= 0x318e) // Hangul Elements
|| (0x3190 <= wch && wch <= 0x3247) // From Unicode 9.0, this range is wide
|| (0x3251 <= wch && wch <= 0x4dbf) // Unicode 9.0 CJK Unified Ideographs, Yi, Reserved, Han Ideograph (hexagrams from 4DC0..4DFF are ignored
|| (0x4e00 <= wch && wch <= 0xa4c6) // Unicode 9.0 CJK Unified Ideographs, Yi, Reserved, Han Ideograph (hexagrams from 4DC0..4DFF are ignored
|| (0xa960 <= wch && wch <= 0xa97c) // Wide Hangul Choseong
|| (0xac00 <= wch && wch <= 0xd7a3) // Korean Hangul Syllables
|| (0xf900 <= wch && wch <= 0xfaff) // From Unicode 9.0, this range is wide [CJK Compatibility Ideographs, Includes Han Compatibility Ideographs]
|| (0xfe10 <= wch && wch <= 0xfe1f) // From Unicode 9.0, this range is wide [Presentation forms]
|| (0xfe30 <= wch && wch <= 0xfe6b) // From Unicode 9.0, this range is wide [Presentation forms]
|| (0xff01 <= wch && wch <= 0xff5e) // Fullwidth ASCII variants
|| (0xffe0 <= wch && wch <= 0xffe6)) // Fullwidth symbol variants
{
return CodepointWidth::Wide;
}

return CodepointWidth::Invalid;
}

std::deque<std::unique_ptr<KeyEvent>> Microsoft::Console::Interactivity::CharToKeyEvents(const wchar_t wch,
const unsigned int codepage)
{
Expand All @@ -24,31 +62,26 @@ std::deque<std::unique_ptr<KeyEvent>> Microsoft::Console::Interactivity::CharToK

if (keyState == invalidKey)
{
// Determine DBCS character because these character does not know by VkKeyScan.
// GetStringTypeW(CT_CTYPE3) & C3_ALPHA can determine all linguistic characters. However, this is
// not include symbolic character for DBCS.
WORD CharType = 0;
GetStringTypeW(CT_CTYPE3, &wch, 1, &CharType);

if (WI_IsFlagSet(CharType, C3_ALPHA) || GetQuickCharWidth(wch) == CodepointWidth::Wide)
if constexpr (useNumpadEvents)
{
keyState = 0;
}
}
// Determine DBCS character because these character does not know by VkKeyScan.
// GetStringTypeW(CT_CTYPE3) & C3_ALPHA can determine all linguistic characters. However, this is
// not include symbolic character for DBCS.
WORD CharType = 0;
GetStringTypeW(CT_CTYPE3, &wch, 1, &CharType);

std::deque<std::unique_ptr<KeyEvent>> convertedEvents;
if (keyState == invalidKey)
{
// if VkKeyScanW fails (char is not in kbd layout), we must
// emulate the key being input through the numpad
convertedEvents = SynthesizeNumpadEvents(wch, codepage);
}
else
{
convertedEvents = SynthesizeKeyboardEvents(wch, keyState);
if (!(WI_IsFlagSet(CharType, C3_ALPHA) || GetQuickCharWidthLegacyForNumpadEventSynthesis(wch) == CodepointWidth::Wide))
{
// It wasn't alphanumeric or determined to be wide by the old algorithm
// if VkKeyScanW fails (char is not in kbd layout), we must
// emulate the key being input through the numpad
return SynthesizeNumpadEvents(wch, codepage);
}
}
keyState = 0; // SynthesizeKeyboardEvents would rather get 0 than -1
}

return convertedEvents;
return SynthesizeKeyboardEvents(wch, keyState);
}

// Routine Description:
Expand Down

0 comments on commit 66b9b9d

Please sign in to comment.