Skip to content

Commit 66b9b9d

Browse files
DHowettDHowett
committed
Merged PR 5984262: [Git2Git] Merged PR 5982901: Reintroduce GetQuickCharWidth 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
1 parent d7f690d commit 66b9b9d

File tree

2 files changed

+81
-36
lines changed

2 files changed

+81
-36
lines changed

‎src/host/ut_host/ClipboardTests.cpp‎

Lines changed: 27 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -261,37 +261,43 @@ class ClipboardTests
261261
}
262262
}
263263

264-
#ifdef __INSIDE_WINDOWS
265264
TEST_METHOD(CanConvertCharsRequiringAltGr)
266265
{
267266
const std::wstring wstr = L"\x20ac"; // € char U+20AC
267+
268+
const short keyState = VkKeyScanW(wstr[0]);
269+
const WORD virtualKeyCode = LOBYTE(keyState);
270+
const WORD virtualScanCode = static_cast<WORD>(MapVirtualKeyW(virtualKeyCode, MAPVK_VK_TO_VSC));
271+
272+
if (keyState == -1 || HIBYTE(keyState) == 0 /* no modifiers required */)
273+
{
274+
Log::Comment(L"This test only works on keyboard layouts where the Euro symbol exists and requires AltGr.");
275+
Log::Result(WEX::Logging::TestResults::Skipped);
276+
return;
277+
}
278+
268279
std::deque<std::unique_ptr<IInputEvent>> events = Clipboard::Instance().TextToKeyEvents(wstr.c_str(),
269280
wstr.size());
270281

282+
std::deque<KeyEvent> expectedEvents;
271283
// should be converted to:
272284
// 1. AltGr keydown
273285
// 2. € keydown
274286
// 3. € keyup
275287
// 4. AltGr keyup
276-
const size_t convertedSize = 4;
277-
VERIFY_ARE_EQUAL(convertedSize, events.size());
278-
279-
const short keyState = VkKeyScanW(wstr[0]);
280-
const WORD virtualKeyCode = LOBYTE(keyState);
281-
282-
std::deque<KeyEvent> expectedEvents;
283288
expectedEvents.push_back({ TRUE, 1, VK_MENU, altScanCode, L'\0', (ENHANCED_KEY | LEFT_CTRL_PRESSED | RIGHT_ALT_PRESSED) });
284-
expectedEvents.push_back({ TRUE, 1, virtualKeyCode, 0, wstr[0], (LEFT_CTRL_PRESSED | RIGHT_ALT_PRESSED) });
285-
expectedEvents.push_back({ FALSE, 1, virtualKeyCode, 0, wstr[0], (LEFT_CTRL_PRESSED | RIGHT_ALT_PRESSED) });
289+
expectedEvents.push_back({ TRUE, 1, virtualKeyCode, virtualScanCode, wstr[0], (LEFT_CTRL_PRESSED | RIGHT_ALT_PRESSED) });
290+
expectedEvents.push_back({ FALSE, 1, virtualKeyCode, virtualScanCode, wstr[0], (LEFT_CTRL_PRESSED | RIGHT_ALT_PRESSED) });
286291
expectedEvents.push_back({ FALSE, 1, VK_MENU, altScanCode, L'\0', ENHANCED_KEY });
287292

293+
VERIFY_ARE_EQUAL(expectedEvents.size(), events.size());
294+
288295
for (size_t i = 0; i < events.size(); ++i)
289296
{
290297
const KeyEvent currentKeyEvent = *reinterpret_cast<const KeyEvent* const>(events[i].get());
291298
VERIFY_ARE_EQUAL(expectedEvents[i], currentKeyEvent, NoThrowString().Format(L"i == %d", i));
292299
}
293300
}
294-
#endif
295301

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

310+
std::deque<KeyEvent> expectedEvents;
311+
#ifdef __INSIDE_WINDOWS
312+
// Inside Windows, where numpad events are enabled, this generated numpad events.
304313
// should be converted to:
305314
// 1. left alt keydown
306315
// 2. 1st numpad keydown
307316
// 3. 1st numpad keyup
308317
// 4. 2nd numpad keydown
309318
// 5. 2nd numpad keyup
310319
// 6. left alt keyup
311-
const size_t convertedSize = 6;
312-
VERIFY_ARE_EQUAL(convertedSize, events.size());
313-
314-
std::deque<KeyEvent> expectedEvents;
315320
expectedEvents.push_back({ TRUE, 1, VK_MENU, altScanCode, L'\0', LEFT_ALT_PRESSED });
316321
expectedEvents.push_back({ TRUE, 1, 0x66, 0x4D, L'\0', LEFT_ALT_PRESSED });
317322
expectedEvents.push_back({ FALSE, 1, 0x66, 0x4D, L'\0', LEFT_ALT_PRESSED });
318323
expectedEvents.push_back({ TRUE, 1, 0x63, 0x51, L'\0', LEFT_ALT_PRESSED });
319324
expectedEvents.push_back({ FALSE, 1, 0x63, 0x51, L'\0', LEFT_ALT_PRESSED });
320325
expectedEvents.push_back({ FALSE, 1, VK_MENU, altScanCode, wstr[0], 0 });
326+
#else
327+
// Outside Windows, without numpad events, we just emit the key with a nonzero UnicodeChar
328+
expectedEvents.push_back({ TRUE, 1, 0, 0, wstr[0], 0 });
329+
expectedEvents.push_back({ FALSE, 1, 0, 0, wstr[0], 0 });
330+
#endif
331+
332+
VERIFY_ARE_EQUAL(expectedEvents.size(), events.size());
321333

322334
for (size_t i = 0; i < events.size(); ++i)
323335
{

‎src/interactivity/base/EventSynthesis.cpp‎

Lines changed: 54 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,44 @@
1616
static constexpr WORD altScanCode = 0x38;
1717
static constexpr WORD leftShiftScanCode = 0x2A;
1818

19+
#ifdef __INSIDE_WINDOWS
20+
// To reduce the risk of compatibility issues inside Windows, we're going to continue using the old
21+
// version of GetQuickCharWidth to determine whether a character should be synthesized into numpad
22+
// events.
23+
static constexpr bool useNumpadEvents{ true };
24+
#else // !defined(__INSIDE_WINDOWS)
25+
// In Terminal, however, we will *always* use normal key events (!)
26+
static constexpr bool useNumpadEvents{ false };
27+
#endif // __INSIDE_WINDOWS
28+
29+
// Routine Description:
30+
// - naively determines the width of a UCS2 encoded wchar (with caveats noted above)
31+
#pragma warning(suppress : 4505) // this function will be deleted if useNumpadEvents is false
32+
static CodepointWidth GetQuickCharWidthLegacyForNumpadEventSynthesis(const wchar_t wch) noexcept
33+
{
34+
if ((0x1100 <= wch && wch <= 0x115f) // From Unicode 9.0, Hangul Choseong is wide
35+
|| (0x2e80 <= wch && wch <= 0x303e) // From Unicode 9.0, this range is wide (assorted languages)
36+
|| (0x3041 <= wch && wch <= 0x3094) // Hiragana
37+
|| (0x30a1 <= wch && wch <= 0x30f6) // Katakana
38+
|| (0x3105 <= wch && wch <= 0x312c) // Bopomofo
39+
|| (0x3131 <= wch && wch <= 0x318e) // Hangul Elements
40+
|| (0x3190 <= wch && wch <= 0x3247) // From Unicode 9.0, this range is wide
41+
|| (0x3251 <= wch && wch <= 0x4dbf) // Unicode 9.0 CJK Unified Ideographs, Yi, Reserved, Han Ideograph (hexagrams from 4DC0..4DFF are ignored
42+
|| (0x4e00 <= wch && wch <= 0xa4c6) // Unicode 9.0 CJK Unified Ideographs, Yi, Reserved, Han Ideograph (hexagrams from 4DC0..4DFF are ignored
43+
|| (0xa960 <= wch && wch <= 0xa97c) // Wide Hangul Choseong
44+
|| (0xac00 <= wch && wch <= 0xd7a3) // Korean Hangul Syllables
45+
|| (0xf900 <= wch && wch <= 0xfaff) // From Unicode 9.0, this range is wide [CJK Compatibility Ideographs, Includes Han Compatibility Ideographs]
46+
|| (0xfe10 <= wch && wch <= 0xfe1f) // From Unicode 9.0, this range is wide [Presentation forms]
47+
|| (0xfe30 <= wch && wch <= 0xfe6b) // From Unicode 9.0, this range is wide [Presentation forms]
48+
|| (0xff01 <= wch && wch <= 0xff5e) // Fullwidth ASCII variants
49+
|| (0xffe0 <= wch && wch <= 0xffe6)) // Fullwidth symbol variants
50+
{
51+
return CodepointWidth::Wide;
52+
}
53+
54+
return CodepointWidth::Invalid;
55+
}
56+
1957
std::deque<std::unique_ptr<KeyEvent>> Microsoft::Console::Interactivity::CharToKeyEvents(const wchar_t wch,
2058
const unsigned int codepage)
2159
{
@@ -24,31 +62,26 @@ std::deque<std::unique_ptr<KeyEvent>> Microsoft::Console::Interactivity::CharToK
2462

2563
if (keyState == invalidKey)
2664
{
27-
// Determine DBCS character because these character does not know by VkKeyScan.
28-
// GetStringTypeW(CT_CTYPE3) & C3_ALPHA can determine all linguistic characters. However, this is
29-
// not include symbolic character for DBCS.
30-
WORD CharType = 0;
31-
GetStringTypeW(CT_CTYPE3, &wch, 1, &CharType);
32-
33-
if (WI_IsFlagSet(CharType, C3_ALPHA) || GetQuickCharWidth(wch) == CodepointWidth::Wide)
65+
if constexpr (useNumpadEvents)
3466
{
35-
keyState = 0;
36-
}
37-
}
67+
// Determine DBCS character because these character does not know by VkKeyScan.
68+
// GetStringTypeW(CT_CTYPE3) & C3_ALPHA can determine all linguistic characters. However, this is
69+
// not include symbolic character for DBCS.
70+
WORD CharType = 0;
71+
GetStringTypeW(CT_CTYPE3, &wch, 1, &CharType);
3872

39-
std::deque<std::unique_ptr<KeyEvent>> convertedEvents;
40-
if (keyState == invalidKey)
41-
{
42-
// if VkKeyScanW fails (char is not in kbd layout), we must
43-
// emulate the key being input through the numpad
44-
convertedEvents = SynthesizeNumpadEvents(wch, codepage);
45-
}
46-
else
47-
{
48-
convertedEvents = SynthesizeKeyboardEvents(wch, keyState);
73+
if (!(WI_IsFlagSet(CharType, C3_ALPHA) || GetQuickCharWidthLegacyForNumpadEventSynthesis(wch) == CodepointWidth::Wide))
74+
{
75+
// It wasn't alphanumeric or determined to be wide by the old algorithm
76+
// if VkKeyScanW fails (char is not in kbd layout), we must
77+
// emulate the key being input through the numpad
78+
return SynthesizeNumpadEvents(wch, codepage);
79+
}
80+
}
81+
keyState = 0; // SynthesizeKeyboardEvents would rather get 0 than -1
4982
}
5083

51-
return convertedEvents;
84+
return SynthesizeKeyboardEvents(wch, keyState);
5285
}
5386

5487
// Routine Description:

0 commit comments

Comments
 (0)