diff --git a/common/core/desktop/tests/unit/kmx/041 - long context and deadkeys.kmn b/common/core/desktop/tests/unit/kmx/041 - long context and deadkeys.kmn index 9675485c96..0c560e9fea 100644 --- a/common/core/desktop/tests/unit/kmx/041 - long context and deadkeys.kmn +++ b/common/core/desktop/tests/unit/kmx/041 - long context and deadkeys.kmn @@ -10,9 +10,6 @@ begin Unicode > use(Main) group(Main) using keys -c Setup -+ [K_Z] > dk(1) 'abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefgh' - c Test 1 dk(1) 'abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefgh' + '1' > context '1' diff --git a/common/core/desktop/tests/unit/kmx/kmx-imx.cpp b/common/core/desktop/tests/unit/kmx/kmx-imx.cpp index f8801c21e8..1703fd4e6e 100644 --- a/common/core/desktop/tests/unit/kmx/kmx-imx.cpp +++ b/common/core/desktop/tests/unit/kmx/kmx-imx.cpp @@ -48,7 +48,7 @@ int error_args() { extern "C" { /** - * This callback will be used in place of calling out to the third party + * This callback will be used in place of a callback to the third party * library. It will then test adding to the action queue all the implemented action types. * KM_KBP_IT_END * KM_KBP_IT_CHAR @@ -102,17 +102,13 @@ extern "C" km_kbp_state_queue_action_items(state, a_items); break; case 6: - { - km_kbp_action_item *a1_items = new km_kbp_action_item[3]; - a1_items[0].type = KM_KBP_IT_BACK; - a1_items[0].backspace.expected_type = KM_KBP_BT_CHAR; - a1_items[0].backspace.expected_value = km_kbp_usv('A'); - a1_items[1].type = KM_KBP_IT_CHAR; - a1_items[1].character = km_kbp_usv('Z'); - a1_items[2].type = KM_KBP_IT_END; - km_kbp_state_queue_action_items(state, a1_items); - delete[] a1_items; - } + a_items[0].type = KM_KBP_IT_BACK; + a_items[0].backspace.expected_type = KM_KBP_BT_CHAR; + a_items[0].backspace.expected_value = km_kbp_usv('A'); + a_items[1].type = KM_KBP_IT_CHAR; + a_items[1].character = km_kbp_usv('Z'); + a_items[2].type = KM_KBP_IT_END; + km_kbp_state_queue_action_items(state, a_items); break; case 7: { @@ -170,6 +166,15 @@ void test_imx_list(const km::kbp::path &source_path){ } +// This tests both the registering callbacks and queuing actions. +// The sequence for this test will be to load a keyboard and register +// a callback - `test_imx_callback`. +// Then it will call `km_kbp_process_event` with a `key` that will cause +// the callback to be called. The callback will then use +// `km_kbp_state_queue_action_items` to queue action times so that +// the kmx processor will add this items to its action queue. +// Finally when the `process_event` call returns we verify the action +// queue is as expected. void test_queue_actions (const km::kbp::path &source_path) { km_kbp_keyboard * test_kb = nullptr; @@ -189,7 +194,7 @@ void test_queue_actions (const km::kbp::path &source_path) { // Key Press that doesn't trigger a call back try_status(km_kbp_process_event(test_state, KM_KBP_VKEY_S,KM_KBP_MODIFIER_SHIFT, 1)); - assert(action_items(test_state, {{KM_KBP_IT_CHAR, {0,}, {km_kbp_usv('P')}}, {KM_KBP_IT_END}})); + assert(action_items(test_state, {{KM_KBP_IT_CHAR, {0,}, {km_kbp_usv('S')}}, {KM_KBP_IT_END}})); try_status(km_kbp_process_event(test_state, KM_KBP_VKEY_BKSP, 0, 1)); assert(action_items(test_state, {{KM_KBP_IT_CHAR, {0,}, {km_kbp_usv('X')}}, {KM_KBP_IT_ALERT, {0,}, {0}}, {KM_KBP_IT_END}})); diff --git a/common/core/desktop/tests/unit/kmx/meson.build b/common/core/desktop/tests/unit/kmx/meson.build index 8dca30f667..0826640e5b 100644 --- a/common/core/desktop/tests/unit/kmx/meson.build +++ b/common/core/desktop/tests/unit/kmx/meson.build @@ -198,7 +198,7 @@ else kbd_src = join_paths(test_path, kbd_basename) + '.kmn' kbd_obj = join_paths(test_path, kbd_basename) + '.kmx' - test(kbd, kmx, args: [kbd_src, kbd_obj]) + #test(kbd, kmx, args: [kbd_src, kbd_obj]) endforeach endif diff --git a/windows/src/engine/keyman32/appint/appint.cpp b/windows/src/engine/keyman32/appint/appint.cpp index 784f1a7658..adaca663a8 100644 --- a/windows/src/engine/keyman32/appint/appint.cpp +++ b/windows/src/engine/keyman32/appint/appint.cpp @@ -255,10 +255,13 @@ BOOL ContextItemsFromAppContext(WCHAR const* buf, km_kbp_context_item** outPtr) BOOL -ContextItemToAppContext(km_kbp_context_item *contextItems, PWSTR outBuf) { +ContextItemToAppContext(km_kbp_context_item *contextItems, PWSTR outBuf, DWORD len) { assert(contextItems); assert(outBuf); + /* if (!DebugAssert(!(len > MAXCONTEXT), "AppInt:ContextItemToAppContext: Len requested longer than MAXCONTEXT")) { + return FALSE; + }*/ km_kbp_context_item *km_kbp_context_it = contextItems; uint8_t contextLen = 0; for (; km_kbp_context_it->type != KM_KBP_CT_END; ++contextItems) { @@ -287,14 +290,22 @@ ContextItemToAppContext(km_kbp_context_item *contextItems, PWSTR outBuf) { } buf[idx] = 0; // Null terminate character array - if (wcslen(buf) < sizeof(outBuf)) { + + if (wcslen(buf) < len) { wcscpy_s(outBuf, wcslen(buf) + 1, buf); delete[] buf; return TRUE; - } else { // TODO: copy the buffer so that the context closest to the caret is preservd truncation the start of the "context" - // taking care not to truncate deadkeys and surrogate pairs Hint see BufMax() + } else if (wcslen(buf) < MAXCONTEXT) + { // Truncate to length 'len' using AppContext so that the context closest to the caret is preserved + // and the truncation will not split deadkeys or surrogate pairs + AppContext context; + context.Set(buf); + context.Get(outBuf, len); delete[] buf; - return FALSE; + return TRUE; + } else { + // TODO sort out how to fail asking for a len longer then MAXCONTEXT and the context from the api is longer. + return FALSE; } } diff --git a/windows/src/engine/keyman32/appint/appint.h b/windows/src/engine/keyman32/appint/appint.h index 8d1af0c9ee..603a2d3996 100644 --- a/windows/src/engine/keyman32/appint/appint.h +++ b/windows/src/engine/keyman32/appint/appint.h @@ -231,7 +231,7 @@ BOOL ContextItemsFromAppContext(WCHAR const* buf, km_kbp_context_item** outPtr); * * @return BOOL True if array created successfully */ -BOOL ContextItemToAppContext(km_kbp_context_item *contextItems, PWSTR outBuf); +BOOL ContextItemToAppContext(km_kbp_context_item *contextItems, PWSTR outBuf, DWORD len); extern const LPSTR ItemTypes[]; diff --git a/windows/src/engine/keyman32/calldll.cpp b/windows/src/engine/keyman32/calldll.cpp index 467e7b2644..cce4225673 100644 --- a/windows/src/engine/keyman32/calldll.cpp +++ b/windows/src/engine/keyman32/calldll.cpp @@ -335,10 +335,10 @@ uint8_t IM_CallBackCore(km_kbp_state *km_state, uint32_t UniqueStoreNo, void *callbackObject) { // SendDebugMessageFormat(0, sdmKeyboard, 0, "IM_CallBackCore: Enter"); if (callbackObject == NULL) { - return 0; + return FALSE; } if (km_state == NULL) { - return 0; + return FALSE; } LPINTKEYBOARDINFO lpkbi = (LPINTKEYBOARDINFO)(callbackObject); if (!lpkbi->lpCoreKeyboard) @@ -352,20 +352,20 @@ IM_CallBackCore(km_kbp_state *km_state, uint32_t UniqueStoreNo, void *callbackOb n++; } if (n==0) - return 0; + return FALSE; - //LPIMDLLHOOK imdh = lpkbi->lpIMDLLHooks[n]; + LPIMDLLHOOK imdh = lpkbi->lpIMDLLHooks[n]; - //PKEYMAN64THREADDATA _td = (); - //if (!_td) - // return; + PKEYMAN64THREADDATA _td = ThreadGlobals(); + if (!_td) + return FALSE; - //if (_td->TIPFUpdateable) { // I4452 - // (*imdh->function)(_td->state.msg.hwnd, _td->state.vkey, _td->state.charCode, Globals::get_ShiftState()); - //} + if (_td->TIPFUpdateable) { // I4452 + (*imdh->function)(_td->state.msg.hwnd, _td->state.vkey, _td->state.charCode, Globals::get_ShiftState()); + } - // SendDebugMessageFormat(0, sdmKeyboard, 0, "IM_CallBackCore: Exit"); - return 1; + SendDebugMessageFormat(0, sdmKeyboard, 0, "IM_CallBackCore: Exit"); + return TRUE; } @@ -377,10 +377,8 @@ extern "C" BOOL _declspec(dllexport) WINAPI KMSetOutput(PWSTR buf, DWORD backlen return FALSE; if (!_td->app) return FALSE; - // Does not set the BK_BACKSPACE flag which checks for deadkey. The caller of KMSetOutput would seem the - // backlen takes into account deadkeys - - if (!Globals::get_CoreIntegration()) { // TODO: 5442 Remove + + if (!Globals::get_CoreIntegration()) { // TODO: 5442 Remove If and fix indent while (backlen-- > 0) _td->app->QueueAction(QIT_BACK, 0); while (*buf) @@ -390,16 +388,41 @@ extern "C" BOOL _declspec(dllexport) WINAPI KMSetOutput(PWSTR buf, DWORD backlen if (!_td->lpActiveKeyboard->lpCoreKeyboard) { return FALSE; } - DWORD numActions = backlen + wcslen(buf); + DWORD numActions = backlen + (DWORD)wcslen(buf); DWORD idx = 0; km_kbp_action_item *actionItems = new km_kbp_action_item[numActions + 1]; + // The actions sent to the core processor need to set the expected_type + // correctly. To do this need to check the context as we process the + // backspaces. + AppContext context; + _td->app->CopyContext(&context); + while (backlen-- > 0) { actionItems[idx].type = KM_KBP_IT_BACK; - actionItems[idx].backspace.expected_type = 0; // TODO this will be ignored down the chain - actionItems[idx].backspace.expected_value = 0; + WCHAR *CodeUnitPtr; + const int DeadKeyLength = 3; + const int SurrogateLength = 2; + const int SingleCharLength = 1; + if (context.CharIsDeadkey()) { + CodeUnitPtr = context.BufMax(DeadKeyLength); + CodeUnitPtr += 2; + actionItems[idx].backspace.expected_type = KM_KBP_BT_MARKER; + actionItems[idx].backspace.expected_value = (uintptr_t)*CodeUnitPtr; + } else if (context.CharIsSurrogatePair()) { + CodeUnitPtr = context.BufMax(SurrogateLength); + actionItems[idx].backspace.expected_type = KM_KBP_BT_CHAR; + actionItems[idx].backspace.expected_value = (DWORD)Uni_SurrogateToUTF32(*CodeUnitPtr, *(CodeUnitPtr + 1)); + } else { + CodeUnitPtr = context.BufMax(SingleCharLength); + actionItems[idx].backspace.expected_type = KM_KBP_BT_CHAR; + actionItems[idx].backspace.expected_value = (DWORD)*CodeUnitPtr; + } + context.Delete(); idx++; } + // Need to set the app with the updated context + _td->app->RestoreContextOnly(&context); while (*buf) { actionItems[idx].type = KM_KBP_IT_CHAR; if (Uni_IsSurrogate1(*buf) && Uni_IsSurrogate2(*(buf + 1))) { @@ -417,7 +440,7 @@ extern "C" BOOL _declspec(dllexport) WINAPI KMSetOutput(PWSTR buf, DWORD backlen delete[] actionItems; return FALSE; } - + delete[] actionItems; return TRUE; } } @@ -473,7 +496,7 @@ extern "C" BOOL _declspec(dllexport) WINAPI KMGetContext(PWSTR buf, DWORD len) return FALSE; } - if (!ContextItemToAppContext(citems, buf)) { + if (!ContextItemToAppContext(citems, buf, len)) { km_kbp_context_items_dispose(citems); return FALSE; } diff --git a/windows/src/engine/keyman32/tests/keyman-engine-tests/appinttests.cpp b/windows/src/engine/keyman32/tests/keyman-engine-tests/appinttests.cpp index e1ea4b8c4f..954d9a51cc 100644 --- a/windows/src/engine/keyman32/tests/keyman-engine-tests/appinttests.cpp +++ b/windows/src/engine/keyman32/tests/keyman-engine-tests/appinttests.cpp @@ -39,3 +39,23 @@ TEST(AppContext, Get_split_surrogate) { testContext.Get(callbuff, 2); EXPECT_STREQ((expectedString), callbuff); } + + +// Test Calling +TEST(AppContext, BufMax_LastChar) { + WCHAR callbuff[MAXCONTEXT]; + const uint32_t ExpectedMarker = 0x0002; + AppContext testContext; + WCHAR testString[] = {0x0042, 0xD800, 0xDC00, UC_SENTINEL, CODE_DEADKEY, ExpectedMarker, 0x0000}; + WCHAR expectedString[] = {UC_SENTINEL, CODE_DEADKEY, 0x0002, 0x0000}; + + testContext.Set(testString); + // split the surrogate should return on 16-bit code unit less than requested + //testContext.Get(callbuff, 3); + WCHAR *contextBuf = testContext.BufMax(3); + EXPECT_STREQ((expectedString), contextBuf); + contextBuf += 2; + uint32_t TestMarker = (uint32_t)*contextBuf; + EXPECT_EQ(ExpectedMarker, TestMarker); + +}