diff --git a/windows/src/engine/keyman32/appint/aiTIP.cpp b/windows/src/engine/keyman32/appint/aiTIP.cpp index f56bd98620..a2baada55b 100644 --- a/windows/src/engine/keyman32/appint/aiTIP.cpp +++ b/windows/src/engine/keyman32/appint/aiTIP.cpp @@ -114,6 +114,12 @@ void ProcessToggleChange(UINT key) { // I4793 } } +void UpdateLastKeyCache(PKEYMAN64THREADDATA _td, WPARAM wParam, BYTE scan, BYTE keyTransition) { + _td->LastKey = wParam; + _td->LastScanCode = scan; + _td->LastTransition = keyTransition; +} + BOOL TIPProcessKeyInternal( PKEYMAN64THREADDATA _td, WPARAM wParam, @@ -128,23 +134,34 @@ BOOL TIPProcessKeyInternal( BOOL isUp = keyFlags & KF_UP ? TRUE : FALSE; BOOL extended = keyFlags & KF_EXTENDED ? TRUE : FALSE; BYTE scan = keyFlags & 0xFF; + BYTE keyTransition = KEYMSG_FLAG_TRANSITION(lParam); + WPARAM prevKey = _td->LastKey; + BYTE prevScanCode = _td->LastScanCode; + BYTE prevKeyTransition = _td->LastTransition; + UpdateLastKeyCache(_td, wParam, scan, keyTransition); SendDebugEntry(); SendDebugMessageFormat("VirtualKey=%s lParam=%x IsUp=%d Extended=%d Updateable=%d Preserved=%d", Debug_VirtualKey((WORD) wParam), lParam, isUp, extended, Updateable, Preserved); - if(_td->LastKey == wParam && scan == 0) { // I4642 + if (_td->LastKey == wParam && scan == 0) { // I4642 - handle issue with Logos application. scan = _td->LastScanCode; SendDebugMessageFormat("Scan code was zero so using cached scan code %x", scan); } - if(scan == SCAN_FLAG_KEYMAN_KEY_EVENT) { // I4370 + // If this key event was generated by Keyman, then we should return FALSE without + // processing it in the core processor. If it is a CapsLock key we update our + // Globals::ShiftState. Since this could run before kmhook_getmessage processes the event, + // we also check for the scan code flag that we set when generating a key event from Keyman. + if ((prevKey == wParam && prevScanCode == SCAN_FLAG_KEYMAN_KEY_EVENT && prevKeyTransition == keyTransition) + || scan == SCAN_FLAG_KEYMAN_KEY_EVENT) { if (wParam == VK_CAPITAL && !isUp) { // Must also record toggle state change when Keyman has generated // a Caps Lock event ProcessToggleChange((UINT)wParam); // I4793 } - SendDebugMessageFormat("Virtual Key was generated by Keyman [Scan=0xFF]"); + SendDebugMessageFormat("Virtual Key was generated by Keyman [Scan=%x wParam=%x] [prevKey=%x prevScanCode=%x prevKeyTransition=%x]", + scan, wParam, prevKey, prevScanCode, prevKeyTransition); return_SendDebugExit(FALSE); } diff --git a/windows/src/engine/keyman32/appint/aiTIP.h b/windows/src/engine/keyman32/appint/aiTIP.h index a5d84f92a8..e379b49941 100644 --- a/windows/src/engine/keyman32/appint/aiTIP.h +++ b/windows/src/engine/keyman32/appint/aiTIP.h @@ -80,10 +80,27 @@ public: /** * ProcessToggleChange - * Toggles the state of FLAGS in the Globals::ShiftState bit mask + * Sets or clears the state of FLAGS in the Globals::ShiftState bit mask + * Using the status of the key as determined by GetKeyState, + * ensuring it is consistent with the actual state of the key. * Supports VK_CAPITAL and VK_NUMLOCK * It DOES NOT generate a system event change for these flags * @param key */ void ProcessToggleChange(UINT key); + +// Forward declaration - final in globals.h +struct tagKEYMAN64THREADDATA; +typedef struct tagKEYMAN64THREADDATA *PKEYMAN64THREADDATA; + +/** + * Update the cache of the last key event received, this is set here - TIP processing, and by + * the GetMessage hook. + * @param _td Thread data to update + * @param wParam WPARAM of the key event + * @param scan Scan code of the key event + * @param keyTransition Transition state of the key event (00b = key down, 01b = repeat, 11b = keyup) + */ +void UpdateLastKeyCache(PKEYMAN64THREADDATA _td, WPARAM wParam, BYTE scan, BYTE keyTransition); + #endif diff --git a/windows/src/engine/keyman32/globals.h b/windows/src/engine/keyman32/globals.h index 3c71163446..9db8044394 100644 --- a/windows/src/engine/keyman32/globals.h +++ b/windows/src/engine/keyman32/globals.h @@ -239,6 +239,7 @@ typedef struct tagKEYMAN64THREADDATA WPARAM LastKey; // I4642 BYTE LastScanCode; // I4642 + BYTE LastTransition; /* Serialized key events */ diff --git a/windows/src/engine/keyman32/keyman64.h b/windows/src/engine/keyman32/keyman64.h index e46dad1782..eb199722c7 100644 --- a/windows/src/engine/keyman32/keyman64.h +++ b/windows/src/engine/keyman32/keyman64.h @@ -118,8 +118,15 @@ #define KEYMSG_FLAG_DLGMODE(lParam) (HIWORD(lParam) & KF_DLGMODE ? 1 : 0) #define KEYMSG_FLAG_MENUMODE(lParam) (HIWORD(lParam) & KF_MENUMODE ? 1 : 0) #define KEYMSG_FLAG_ALTDOWN(lParam) (HIWORD(lParam) & KF_ALTDOWN ? 1 : 0) +// Repeat is actually previous KF_UP value of the key. +// It is always set to 1 for WM_KEYUP and WM_SYSKEYUP messages. +// It is set to 1 for WM_KEYDOWN and WM_SYSKEYDOWN keystroke messages generated by the automatic repeat feature. +// see https://learn.microsoft.com/en-us/windows/win32/inputdev/about-keyboard-input#previous-key-state-flag #define KEYMSG_FLAG_REPEAT(lParam) (HIWORD(lParam) & KF_REPEAT ? 1 : 0) #define KEYMSG_FLAG_UP(lParam) (HIWORD(lParam) & KF_UP ? 1 : 0) +// Combine the transition flags into a single value for higher chance of identification +// 00b = key down, 01b = repeat, 11b = keyup +#define KEYMSG_FLAG_TRANSITION(lParam) ((BYTE)((HIWORD(lParam) & (KF_UP | KF_REPEAT)) >> 14)) // TODO: Deprecate overloading of scancodes and use dwExtraInfo instead #define SCAN_FLAG_KEYMAN_KEY_EVENT 0xFF diff --git a/windows/src/engine/keyman32/kmhook_getmessage.cpp b/windows/src/engine/keyman32/kmhook_getmessage.cpp index 4fee835254..4c11dc31b5 100644 --- a/windows/src/engine/keyman32/kmhook_getmessage.cpp +++ b/windows/src/engine/keyman32/kmhook_getmessage.cpp @@ -156,9 +156,9 @@ LRESULT _kmnGetMessageProc(int nCode, WPARAM wParam, LPARAM lParam) } BYTE scan = KEYMSG_LPARAM_SCAN(mp->lParam); + BYTE keyTransitionEvent = KEYMSG_FLAG_TRANSITION(mp->lParam); CheckScheduledRefresh(); - _td->LastScanCode = scan; - _td->LastKey = mp->wParam; + UpdateLastKeyCache(_td, mp->wParam, scan, keyTransitionEvent); switch (mp->wParam) { case VK_MENU: @@ -174,6 +174,11 @@ LRESULT _kmnGetMessageProc(int nCode, WPARAM wParam, LPARAM lParam) if (mp->wParam != VK_BACK) { if(scan == SCAN_FLAG_KEYMAN_KEY_EVENT) { mp->lParam = (mp->lParam & 0xFF00FFFFL) | (MapVirtualKey((UINT)mp->wParam, 0) << 16); + if (mp->wParam == VK_CAPITAL) { + ProcessToggleChange(VK_CAPITAL); + } + SendDebugMessageFormat("WMKEY=%x Clear `SCAN_FLAG_KEYMAN_KEY_EVENT` wParam=%x lParam=%x Set LastKey=%x LastScanCode=%x LastTransition=%x", + mp->message, mp->wParam, mp->lParam, _td->LastKey, _td->LastScanCode, _td->LastTransition); } } } diff --git a/windows/src/engine/keyman32/kmprocess.cpp b/windows/src/engine/keyman32/kmprocess.cpp index 8ba4c22a8b..6e1eff08e3 100644 --- a/windows/src/engine/keyman32/kmprocess.cpp +++ b/windows/src/engine/keyman32/kmprocess.cpp @@ -131,7 +131,7 @@ BOOL ProcessHook() _td->lpActiveKeyboard->lpCoreKeyboardState, KM_CORE_DEBUG_CONTEXT_CACHED ); - SendDebugMessageFormatW(L"Key %s: %hs Context '%s'", + SendDebugMessageFormatW(L"Key %s: %hs Core Cached Context '%s'", _td->state.isDown ? L"pressed" : L"released", Debug_VirtualKey(_td->state.vkey), debug_context); km_core_cu_dispose(debug_context); diff --git a/windows/src/engine/keyman32/kmprocessactions.cpp b/windows/src/engine/keyman32/kmprocessactions.cpp index bdc8c69e66..2694658762 100644 --- a/windows/src/engine/keyman32/kmprocessactions.cpp +++ b/windows/src/engine/keyman32/kmprocessactions.cpp @@ -77,38 +77,29 @@ processPersistOpt(km_core_actions const* actions, LPINTKEYBOARDINFO activeKeyboa } } -static void processCapsLock(const km_core_caps_state caps_lock_state, BOOL isUp, BOOL Updateable, BOOL externalEvent) { +static void processCapsLock(const km_core_caps_state caps_state_change, BOOL isUp, BOOL Updateable, BOOL externalEvent) { + BOOL isCapsOn = IsCapsLockOn(); - // We only want to process the Caps Lock key event once -- - // in the first pass (!Updateable). - if (Updateable){ + // This debug message is useful for understanding the sequence of events around caps lock changes + //SendDebugMessageFormat("ACTION CAPS STATE:%d FIsUp=%d Updateable=%d ExternalEvent=%d CapsState=%d", caps_state_change, isUp, Updateable, + // externalEvent, isCapsOn); + + // We only want to process the Caps Lock key event once; + // it has to be when updateble=1 as TSF does not consistently + // have updateable=0 events. + if (!Updateable || caps_state_change == KM_CORE_CAPS_UNCHANGED) { return; } + // Turn three state value into a boolean for whether capslock should be on or off, + // we only want to process the key event if the state is changing. + BOOL required_caps_state = (caps_state_change == KM_CORE_CAPS_ON); - if (caps_lock_state == KM_CORE_CAPS_ON) { - // This case would occur for the keyboard system store setting `store(&CapsOnOnly) '1'` - if (isUp && !IsCapsLockOn()) { // I267 - 24/11/2006 invert GetKeyState test - SendDebugMessageFormat("TURN CAPS ON: FIsUp=%d CapsState=%d", isUp, IsCapsLockOn()); - keybd_event(VK_CAPITAL, SCAN_FLAG_KEYMAN_KEY_EVENT, 0, 0); - keybd_event(VK_CAPITAL, SCAN_FLAG_KEYMAN_KEY_EVENT, KEYEVENTF_KEYUP, 0); - } - - // This case would occur for the keyboard system store setting `store(&CapsAlwaysOff) '1'` - // A trick is being played here of synthesising a release the CAPSLOCK key event - // then a depress CAPSLOCK key event - else if (!isUp && IsCapsLockOn()) { // I267 - 24/11/2006 invert GetKeyState test - SendDebugMessageFormat("TURN CAPS OFF: FIsUp=%d CapsState=%d", isUp, IsCapsLockOn()); - keybd_event(VK_CAPITAL, SCAN_FLAG_KEYMAN_KEY_EVENT, KEYEVENTF_KEYUP, 0); - keybd_event(VK_CAPITAL, SCAN_FLAG_KEYMAN_KEY_EVENT, 0, 0); - } - } else if (caps_lock_state == KM_CORE_CAPS_OFF) { - // This case would occur for the keyboard system store setting `store(&ShiftFreesCaps) '1'` - // OR selecting a keyboard with CAPs always off rule - if ((!isUp && IsCapsLockOn()) || (externalEvent && IsCapsLockOn())) { - SendDebugMessageFormat("TURN CAPS OFF: FIsUp=%d CapsState=%d", isUp, IsCapsLockOn()); - keybd_event(VK_CAPITAL, SCAN_FLAG_KEYMAN_KEY_EVENT, 0, 0); - keybd_event(VK_CAPITAL, SCAN_FLAG_KEYMAN_KEY_EVENT, KEYEVENTF_KEYUP, 0); - } + if (isCapsOn != required_caps_state) { + SendDebugMessageFormat( + "Simulate CapsLock %s: FIsUp=%d CurrentCapsState=%d ExternalEvent=%d", + required_caps_state ? "ON" : "OFF", isUp, isCapsOn, externalEvent); + keybd_event(VK_CAPITAL, SCAN_FLAG_KEYMAN_KEY_EVENT, 0, 0); + keybd_event(VK_CAPITAL, SCAN_FLAG_KEYMAN_KEY_EVENT, KEYEVENTF_KEYUP, 0); } } @@ -151,7 +142,6 @@ ProcessActionsNonUpdatableParse(BOOL* emitKeystroke) { km_core_actions const* core_actions = km_core_state_get_actions(_td->lpActiveKeyboard->lpCoreKeyboardState); - processCapsLock(core_actions->new_caps_lock_state, !_td->state.isDown, _td->TIPFUpdateable, FALSE); if (core_actions->emit_keystroke) { *emitKeystroke = TRUE; SendDebugMessageFormat("EMIT_KEYSTROKE"); @@ -167,6 +157,6 @@ ProcessActionsExternalEvent() { return FALSE; } km_core_actions const* core_actions = km_core_state_get_actions(_td->lpActiveKeyboard->lpCoreKeyboardState); - processCapsLock(core_actions->new_caps_lock_state, !_td->state.isDown, FALSE, TRUE); + processCapsLock(core_actions->new_caps_lock_state, !_td->state.isDown, TRUE, TRUE); return TRUE; } diff --git a/windows/src/engine/kmtip/keys.cpp b/windows/src/engine/kmtip/keys.cpp index 50f2ade3b9..9bf67a5f99 100644 --- a/windows/src/engine/kmtip/keys.cpp +++ b/windows/src/engine/kmtip/keys.cpp @@ -75,7 +75,8 @@ BOOL CKMTipTextService::_InitKeystrokeSink() pKeystrokeMgr->Release(); - memset(fEatenBuf, 0, sizeof(fEatenBuf)); + memset(fEatenBuf, 0, sizeof(fEatenBuf)); // OnKeyDown/Up + memset(fOnTestEatenBuf, 0, sizeof(fOnTestEatenBuf)); // OnTestKeyDown/Up return_SendDebugExit(_keystrokeSinkInitialized = (hr == S_OK)); } @@ -178,17 +179,8 @@ STDAPI CKMTipTextService::OnTestKeyDown(ITfContext *pContext, WPARAM wParam, LPA { SendDebugEntry(); LogKey(0, wParam, lParam); - // If the keystroke is a Keyman-generated key, ignore it - // But we need to pass Caps Lock through, even if we generated it, so we can track Caps Lock state. - // TODO: Fix magic constants - if ((lParam & 0x00FF0000L) == 0xFF0000L && - wParam != VK_CAPITAL) { - *pfEaten = FALSE; - } - else { - *pfEaten = _KeymanProcessKeystroke(pContext, wParam, lParam, FALSE, FALSE); // I3588 -// SendDebugMessageFormat("pfEaten=%s", *pfEaten ? "TRUE" : "FALSE"); - } + fOnTestEatenBuf[wParam] = *pfEaten = _KeymanProcessKeystroke(pContext, wParam, lParam, FALSE, FALSE); // I3588 + SendDebugMessageFormat(L"pfEaten=%s, wParam=%x, lParam=%x", *pfEaten ? L"TRUE" : L"FALSE", wParam, lParam); SendDebugExit(); return S_OK; } @@ -205,8 +197,8 @@ STDAPI CKMTipTextService::OnKeyDown(ITfContext *pContext, WPARAM wParam, LPARAM { SendDebugEntry(); LogKey(1, wParam, lParam); - fEatenBuf[wParam] = *pfEaten = _KeymanProcessKeystroke(pContext, wParam, lParam, TRUE, FALSE); // I3588 -// SendDebugMessageFormat("pfEaten=%s", *pfEaten ? "TRUE" : "FALSE"); + fEatenBuf[wParam] = *pfEaten = _KeymanProcessKeystroke(pContext, wParam, lParam, TRUE, FALSE); // I3588 + SendDebugMessageFormat(L"pfEaten=%s wParam=%x lParam=%x", *pfEaten ? L"TRUE" : L"FALSE", wParam, lParam); SendDebugExit(); return S_OK; } @@ -222,17 +214,9 @@ STDAPI CKMTipTextService::OnTestKeyUp(ITfContext *pContext, WPARAM wParam, LPARA { SendDebugEntry(); LogKey(2, wParam, lParam); - // If the keystroke is a Keyman-generated key, ignore it - // But we need to pass Caps Lock through, even if we generated it, so we can track Caps Lock state. - if ((lParam & 0x00FF0000L) == 0xFF0000L && - wParam != VK_CAPITAL) { // I3566 - *pfEaten = FALSE; - } - else { - _KeymanProcessKeystroke(pContext, wParam, lParam, FALSE, FALSE); // I3588 - *pfEaten = fEatenBuf[wParam]; - } -// SendDebugMessageFormat("pfEaten=%s", *pfEaten ? "TRUE" : "FALSE"); + _KeymanProcessKeystroke(pContext, wParam, lParam, FALSE, FALSE); // I3588 + *pfEaten = fOnTestEatenBuf[wParam]; + SendDebugMessageFormat(L"pfEaten=%s wParam=%x lParam=%x", *pfEaten ? L"TRUE" : L"FALSE", wParam, lParam); SendDebugExit(); return S_OK; } @@ -251,16 +235,11 @@ STDAPI CKMTipTextService::OnKeyUp(ITfContext *pContext, WPARAM wParam, LPARAM lP LogKey(3, wParam, lParam); // If the keystroke is a Keyman-generated key, ignore it // But we need to pass Caps Lock through, even if we generated it, so we can track Caps Lock state. - if ((lParam & 0x00FF0000L) == 0xFF0000L && - wParam != VK_CAPITAL) { // I3566 // I3605 - *pfEaten = FALSE; - } - else - { - _KeymanProcessKeystroke(pContext, wParam, lParam, TRUE, FALSE); // I3588 // I3605 - *pfEaten = fEatenBuf[wParam]; - } -// SendDebugMessageFormat("pfEaten=%s", *pfEaten ? "TRUE" : "FALSE"); + + _KeymanProcessKeystroke(pContext, wParam, lParam, TRUE, FALSE); // I3588 // I3605 + *pfEaten = fEatenBuf[wParam]; + + SendDebugMessageFormat(L"pfEaten=%s wParam=%x lParam=%x", *pfEaten ? L"TRUE" : L"FALSE", wParam, lParam); SendDebugExit(); return S_OK; } diff --git a/windows/src/engine/kmtip/kmkey.cpp b/windows/src/engine/kmtip/kmkey.cpp index 609c98e431..1b767f4946 100644 --- a/windows/src/engine/kmtip/kmkey.cpp +++ b/windows/src/engine/kmtip/kmkey.cpp @@ -102,15 +102,16 @@ BOOL CKMTipTextService::_KeymanProcessKeystroke(ITfContext *pContext, WPARAM wPa // Don't process Unicode characters injected or ProcessKey events which are generated by Windows if (wParam == VK_PACKET || wParam == VK_PROCESSKEY) { + SendDebugMessageFormat(L"wParam=%x early return FALSE", wParam); return FALSE; // I3608 // I4201 } SendDebugEntry(); SendDebugMessageFormat(L"%x %x %s %s ex=%x", wParam, lParam, fUpdate ? L"update" : L"", fPreserved ? L"preserved" : L"", GetMessageExtraInfo()); // I4378 - // Don't process keystrokes generated by Keyman (scan code = 0xFF) - // But we need to pass Caps Lock through, even if we generated it, so we can track Caps Lock state. - // TODO: This is done in multiple places, but we probably only need to do it once + // Don't process keystrokes generated by Keyman (scan code = 0xFF = SCAN_FLAG_KEYMAN_KEY_EVENT) + // But we need to pass Caps Lock through to the keyman engine (but not the core), + // even if we generated it, so we can track Caps Lock state. if ((lParam & 0xFF0000) == 0xFF0000 && wParam != VK_CAPITAL) { return_SendDebugExit(FALSE); diff --git a/windows/src/engine/kmtip/kmtip.h b/windows/src/engine/kmtip/kmtip.h index b4f307488b..a067617d44 100644 --- a/windows/src/engine/kmtip/kmtip.h +++ b/windows/src/engine/kmtip/kmtip.h @@ -117,6 +117,7 @@ private: BOOL _keystrokeSinkInitialized; BOOL fEatenBuf[256]; + BOOL fOnTestEatenBuf[256]; ITfThreadMgr *_pThreadMgr; TfClientId _tfClientId;