From 442903c4f3e7326f1a75b2c2528f4d1aa8c1d31c Mon Sep 17 00:00:00 2001 From: rc-swag <58423624+rc-swag@users.noreply.github.com> Date: Thu, 27 Jun 2024 10:58:51 +1000 Subject: [PATCH 1/3] fix(windows): add text selected bool emit key when true Add a bool to the KeymanGetContext call, allowing the windows engine to make the decistion to emit backspace key stroke when text is selecting allowing the application to make the correct decision on how to handle it. Usually deleting the selected text. --- windows/src/engine/keyman32/appint/aiTIP.cpp | 3 +- windows/src/engine/keyman32/appint/aiTIP.h | 5 ++ windows/src/engine/keyman32/globals.h | 2 +- .../src/engine/keyman32/kmprocessactions.cpp | 19 ++++- windows/src/engine/kmtip/kmkey.cpp | 71 +++++++++++++++++-- windows/src/engine/kmtip/kmtip.h | 4 +- .../i3619tip/i3619tip/i3619tip/kmkey.cpp | 18 ++--- 7 files changed, 102 insertions(+), 20 deletions(-) diff --git a/windows/src/engine/keyman32/appint/aiTIP.cpp b/windows/src/engine/keyman32/appint/aiTIP.cpp index 93ffcb445d..0d0cce987c 100644 --- a/windows/src/engine/keyman32/appint/aiTIP.cpp +++ b/windows/src/engine/keyman32/appint/aiTIP.cpp @@ -276,8 +276,7 @@ BOOL AITIP::ReadContext(PWSTR buf) { PKEYMAN64THREADDATA _td = ThreadGlobals(); if(!_td) return FALSE; - - if(_td->TIPGetContext && (*_td->TIPGetContext)(MAXCONTEXT-1, buf) == S_OK) { // I3575 // I4262 + if (_td->TIPGetContext && (*_td->TIPGetContext)(MAXCONTEXT - 1, buf, &isTextSelected) == S_OK) { // I3575 // I4262 if(ShouldDebug(sdmKeyboard)) { SendDebugMessageFormat(0, sdmAIDefault, 0, "AITIP::ReadContext: full context [Updateable=%d] %s", _td->TIPFUpdateable, Debug_UnicodeString(buf)); } diff --git a/windows/src/engine/keyman32/appint/aiTIP.h b/windows/src/engine/keyman32/appint/aiTIP.h index bfbca87d89..62aaaaddc2 100644 --- a/windows/src/engine/keyman32/appint/aiTIP.h +++ b/windows/src/engine/keyman32/appint/aiTIP.h @@ -39,6 +39,7 @@ class AITIP : public AIWin2000Unicode { private: BOOL useLegacy; + BOOL isTextSelected; BOOL PostKeys(); //TOUCH void PostTouchContext(); @@ -68,6 +69,10 @@ public: /* TIP interactions */ BOOL IsLegacy() { return useLegacy; } + BOOL + IsTextSelected() { + return isTextSelected; + } }; /** diff --git a/windows/src/engine/keyman32/globals.h b/windows/src/engine/keyman32/globals.h index 8f1dfc4c90..345c775971 100644 --- a/windows/src/engine/keyman32/globals.h +++ b/windows/src/engine/keyman32/globals.h @@ -151,7 +151,7 @@ public: /* External interface functions */ typedef HRESULT (WINAPI *PKEYMANPROCESSOUTPUTFUNC)(int n, WCHAR *buf, int nbuf); -typedef HRESULT (WINAPI *PKEYMANGETCONTEXTFUNC)(int n, PWSTR buf); +typedef HRESULT (WINAPI *PKEYMANGETCONTEXTFUNC)(int n, PWSTR buf, BOOL* isTextSelected); typedef BOOL (WINAPI *PTIPCALLBACK)(); // Tells the TIP to update its status (used to be done through 0x88) typedef BOOL (WINAPI *PKeymanOutputBackspace)(HWND hwnd); diff --git a/windows/src/engine/keyman32/kmprocessactions.cpp b/windows/src/engine/keyman32/kmprocessactions.cpp index a65d5e89a9..412523e389 100644 --- a/windows/src/engine/keyman32/kmprocessactions.cpp +++ b/windows/src/engine/keyman32/kmprocessactions.cpp @@ -30,13 +30,28 @@ static void processAlert(AITIP* app) { } static void -processBack(AITIP* app, const unsigned int code_points_to_delete, const km_core_usv* delete_context) { +processBack( + AITIP* app, + const unsigned int code_points_to_delete, + const km_core_usv* delete_context, + BOOL* emitKeystroke, + WORD vkey) { if (app->IsLegacy()) { for (unsigned int i = 0; i < code_points_to_delete; i++) { app->QueueAction(QIT_BACK, BK_DEFAULT); } } else { + // If there is a selection the TSF documentation suggests an empty string + // to the select range. However this would require a change to QueueAction + // and the ProcessOutput callback. Instead we will pass through the emit the character + // and let the application clear the the selected text. + if (app->IsTextSelected() && (vkey == VK_BACK)) { + //SendDebugMessageFormat(0, sdmGlobal, 0, "ProcessBack: Emit Keystroke [%d].", app->IsTextSelected()); + *emitKeystroke = TRUE; + return; + } + km_core_usv const* delete_context_ptr = delete_context; while (*delete_context_ptr) { delete_context_ptr++; @@ -109,7 +124,7 @@ BOOL ProcessActions(BOOL* emitKeystroke) _td->CoreProcessEventRun = FALSE; - processBack(_td->app, core_actions->code_points_to_delete, core_actions->deleted_context); + processBack(_td->app, core_actions->code_points_to_delete, core_actions->deleted_context, emitKeystroke, _td->state.vkey); processUnicodeChar(_td->app, core_actions->output); if (core_actions->persist_options != NULL) { processPersistOpt(core_actions, _td->lpActiveKeyboard); diff --git a/windows/src/engine/kmtip/kmkey.cpp b/windows/src/engine/kmtip/kmkey.cpp index 482790340d..5cbdc097bd 100644 --- a/windows/src/engine/kmtip/kmkey.cpp +++ b/windows/src/engine/kmtip/kmkey.cpp @@ -64,7 +64,8 @@ public: STDMETHODIMP DoEditSession(TfEditCookie ec); HRESULT WINAPI KeymanProcessOutput(int n, PWSTR buf, int nbuf); // I3567 - HRESULT WINAPI KeymanGetContext(int n, PWSTR buf); // I3567 + HRESULT WINAPI KeymanGetContext(int n, PWSTR buf, BOOL* isTextSelected); // I3567 + HRESULT WINAPI KeymanIsTextSelected(BOOL* result); HRESULT GetResult() { return _hr; } private: @@ -153,11 +154,11 @@ extern "C" __declspec(dllexport) HRESULT WINAPI ExtKeymanProcessOutput(int n, WC return res; } -extern "C" __declspec(dllexport) HRESULT WINAPI ExtKeymanGetContext(int n, PWSTR buf) // I3567 +extern "C" __declspec(dllexport) HRESULT WINAPI ExtKeymanGetContext(int n, PWSTR buf, BOOL* isTextSelected) // I3567 { LogEnter(); HRESULT res; - if (ExtEditSession) res = ExtEditSession->KeymanGetContext(n, buf); + if (ExtEditSession) res = ExtEditSession->KeymanGetContext(n, buf, isTextSelected); else res = E_FAIL; // I3567 return res; } @@ -179,7 +180,7 @@ STDAPI CKeymanEditSession::DoEditSession(TfEditCookie ec) ExtEditSession = NULL; - // Call keyman32.processtipkey; this may call "KeymanGetContext(n, buf)"; + // Call keyman32.processtipkey; this may call "KeymanGetContext(n, buf, isTextSelected)"; // keyman32.processtipkey this will call "KeymanProcessOutput" // which will delete n chrs from left of cursor and output a text string // beeps, deadkeys, etc. will be managed from within keyman32.dll itself @@ -219,13 +220,15 @@ HRESULT WINAPI CKeymanEditSession::KeymanProcessOutput(int n, WCHAR *buf, int nb } -HRESULT WINAPI CKeymanEditSession::KeymanGetContext(int n, PWSTR buf) // I3567 +HRESULT WINAPI CKeymanEditSession::KeymanGetContext(int n, PWSTR buf, BOOL* isTextSelected) // I3567 { LogEnter(); HRESULT hr; TF_STATUS tfStatus; + *isTextSelected = FALSE; // set to false before any early returns + if (_dwDeepIntegration == DEEPINTEGRATION_DISABLE) { // I4375 Log(L"KeymanGetContext: Exit: deep integration disabled by registry (or default)"); return S_FALSE; @@ -247,6 +250,64 @@ HRESULT WINAPI CKeymanEditSession::KeymanGetContext(int n, PWSTR buf) // I3567 return S_FALSE; // I4933 } + // now check for selected text + if (!SUCCEEDED(hr = KeymanIsTextSelected(isTextSelected))) { + Log(L"KeymanGetContext: Exit: KeymanIsTextSelected failed"); + // Continue to return S_OK as the IsTextSelected will just be + // defaulting to not selected and the context is more important + //return S_FALSE; + } + return S_OK; +} + +HRESULT WINAPI CKeymanEditSession::KeymanIsTextSelected(BOOL* isTextSelected) +{ + LogEnter(); + + HRESULT hr; + TF_STATUS tfStatus; + TF_SELECTION tfSelection; + ULONG cFetched = 0; + *isTextSelected = FALSE; + + if (_dwDeepIntegration == DEEPINTEGRATION_DISABLE) { // I4375 + Log(L"KeymanIsTextSelected: Exit: deep integration disabled by registry (or default)"); + return S_FALSE; + } + + if (!SUCCEEDED(hr = _pContext->GetStatus(&tfStatus))) { // I3568 + Log(L"KeymanIsTextSelected: Exit -- Failed GetStatus = %x", hr); + return hr; + } + + if (tfStatus.dwStaticFlags & TF_SS_TRANSITORY) // I3568 + { + Log(L"KeymanIsTextSelected: Exit: Context does not support manipulation. Using legacy interaction"); + return S_FALSE; + } + + if (!SUCCEEDED(hr = _pContext->GetSelection(_ec, TF_DEFAULT_SELECTION, 1, &tfSelection, &cFetched))) { + Log(L"KeymanIsTextSelected: Exit: -- Failed GetSelection = %x ", hr); + return S_FALSE; + } + + if (hr == TF_E_NOSELECTION) { + // No selection is present + return S_OK; + } + + if (cFetched > 0) { + // check if the range is empty + BOOL isEmpty = FALSE; + LONG lResult = 0; + + // Compare the start and end of the range + hr = tfSelection.range->IsEmpty(_ec, &isEmpty); + if (SUCCEEDED(hr) && !isEmpty) { + *isTextSelected = TRUE; + } + tfSelection.range->Release(); + } return S_OK; } diff --git a/windows/src/engine/kmtip/kmtip.h b/windows/src/engine/kmtip/kmtip.h index ef1d5bafa2..6cb4c39c7f 100644 --- a/windows/src/engine/kmtip/kmtip.h +++ b/windows/src/engine/kmtip/kmtip.h @@ -139,7 +139,9 @@ private: // typedef HRESULT(WINAPI *PKEYMANPROCESSOUTPUTFUNC)(int n, WCHAR *buf, int nbuf); // I3567 -typedef HRESULT(WINAPI *PKEYMANGETCONTEXTFUNC)(int n, PWSTR buf); // I3567 +// Adding a new interface call IsTextSelected could make sense however this would +// also require update TIPProcessKey to have another call back function as an argument +typedef HRESULT(WINAPI *PKEYMANGETCONTEXTFUNC)(int n, PWSTR buf, BOOL* isTextSelected); // I3567 class Keyman32Interface { public: diff --git a/windows/src/test/manual-tests/test_i3619/i3619tip/i3619tip/i3619tip/kmkey.cpp b/windows/src/test/manual-tests/test_i3619/i3619tip/i3619tip/i3619tip/kmkey.cpp index 9f8a4278f3..44e213b892 100644 --- a/windows/src/test/manual-tests/test_i3619/i3619tip/i3619tip/i3619tip/kmkey.cpp +++ b/windows/src/test/manual-tests/test_i3619/i3619tip/i3619tip/i3619tip/kmkey.cpp @@ -1,18 +1,18 @@ /* Name: kmkey Copyright: Copyright (C) SIL International. - Documentation: - Description: + Documentation: + Description: Create Date: 19 Jun 2007 Modified Date: 1 Dec 2012 Authors: mcdurdin - Related Files: - Dependencies: + Related Files: + Dependencies: - Bugs: - Todo: - Notes: + Bugs: + Todo: + Notes: History: 19 Jun 2007 - mcdurdin - I890 - Deadkeys not working correctly in TSF 19 Jun 2007 - mcdurdin - I822 - TSF Addin not working 07 Sep 2009 - mcdurdin - I2095 - TSF addin is not threadsafe @@ -49,7 +49,7 @@ public: STDMETHODIMP DoEditSession(TfEditCookie ec); HRESULT WINAPI KeymanProcessOutput(int n, PWSTR buf, int nbuf); // I3567 - HRESULT WINAPI KeymanGetContext(int n, PWSTR buf); // I3567 + HRESULT WINAPI KeymanGetContext(int n, PWSTR buf, BOOL* isTextSelected); // I3567 HRESULT GetResult() { return _hr; } private: @@ -77,7 +77,7 @@ BOOL CKMTipTextService::_KeymanProcessKeystroke(ITfContext *pContext, WPARAM wPa } else { - if (pContext->RequestEditSession(_tfClientId, pEditSession, + if (pContext->RequestEditSession(_tfClientId, pEditSession, fUpdate ? TF_ES_SYNC | TF_ES_READWRITE : TF_ES_SYNC | TF_ES_READ, &hr) != S_OK) { hr = E_FAIL; From 3998c8b75c25410a17bdecc5e3418a2c2ce83b53 Mon Sep 17 00:00:00 2001 From: rc-swag <58423624+rc-swag@users.noreply.github.com> Date: Tue, 9 Jul 2024 22:39:01 +1000 Subject: [PATCH 2/3] fix(windows): make KeymanIsTextSelected Private Only call GetSelection once, pace the parameters through to the GetLeftSelection in order to get the results needed for KeymanIsTextSelected --- windows/src/engine/kmtip/globals.h | 2 +- windows/src/engine/kmtip/inserttext.cpp | 82 +++++++++++++++---------- windows/src/engine/kmtip/kmkey.cpp | 59 +++++++----------- 3 files changed, 71 insertions(+), 72 deletions(-) diff --git a/windows/src/engine/kmtip/globals.h b/windows/src/engine/kmtip/globals.h index e2bc01ce4e..a01938f6c5 100644 --- a/windows/src/engine/kmtip/globals.h +++ b/windows/src/engine/kmtip/globals.h @@ -45,7 +45,7 @@ void DllRelease(); void InsertTextAtSelection(TfEditCookie ec, ITfContext *pContext, const WCHAR *pchText, ULONG cchText); void DeleteLeftOfSelection(TfEditCookie ec, ITfContext *pContext, LONG n); -BOOL GetLeftOfSelection(TfEditCookie ec, ITfContext *pContext, WCHAR *buf, LONG n); // I4933 +BOOL GetLeftOfSelection(TfEditCookie ec, ITfContext *pContext, WCHAR *buf, LONG n, HRESULT *hrGetSelection, ULONG *cFetched, TF_SELECTION *tfSelection); // I4933 void GetDeadkeyFlags(TfEditCookie ec, ITfContext *pContext, PWSTR buf, int n); // diff --git a/windows/src/engine/kmtip/inserttext.cpp b/windows/src/engine/kmtip/inserttext.cpp index fa504b54ad..d296d0880b 100644 --- a/windows/src/engine/kmtip/inserttext.cpp +++ b/windows/src/engine/kmtip/inserttext.cpp @@ -1,18 +1,18 @@ /* Name: inserttext Copyright: Copyright (C) SIL International. - Documentation: - Description: + Documentation: + Description: Create Date: 19 Jun 2007 Modified Date: 28 Mar 2016 Authors: mcdurdin - Related Files: - Dependencies: + Related Files: + Dependencies: - Bugs: - Todo: - Notes: + Bugs: + Todo: + Notes: History: 19 Jun 2007 - mcdurdin - I890 - Fix deadkeys in TSF 19 Jun 2007 - mcdurdin - I890 - Fix crash with deadkeys - logging 11 Dec 2009 - mcdurdin - Header files @@ -67,7 +67,7 @@ void DePseudofy(WCHAR *pchText) // I3564 { if(*pchText == PseudoMap[k]) { - *pchText = k; + *pchText = k; break; } } @@ -78,7 +78,7 @@ void DePseudofy(WCHAR *pchText) // I3564 void InsertTextAtSelection(TfEditCookie ec, ITfContext *pContext, const WCHAR *pchText, ULONG cchText) { LogEnter(); - + ITfInsertAtSelection *pInsertAtSelection; ITfRange *pRange; TF_SELECTION tfSelection; @@ -89,7 +89,7 @@ void InsertTextAtSelection(TfEditCookie ec, ITfContext *pContext, const WCHAR *p #ifdef DEBUG_PSEUDO // I3607 WCHAR *PseudoBuf = new WCHAR[cchText+1]; // I3564 - + wcsncpy_s(PseudoBuf, cchText+1, pchText, cchText); PseudoBuf[cchText] = 0; Pseudofy(PseudoBuf); @@ -133,7 +133,7 @@ void DeleteLeftOfSelection(TfEditCookie ec, ITfContext *pContext, LONG n) if(pContext->GetSelection(ec, TF_DEFAULT_SELECTION, 1, &tfSelection, &cFetched) != S_OK || cFetched == 0) return; - + //TODO: log failures if(tfSelection.range->ShiftStart(ec, -n, &outn, NULL) != S_OK) { tfSelection.range->Release(); @@ -163,20 +163,27 @@ char *debugstr(PWSTR buf) { return bufout; } -BOOL GetLeftOfSelection(TfEditCookie ec, ITfContext *pContext, WCHAR *buf, LONG n) // I4933 +BOOL +GetLeftOfSelection( + TfEditCookie ec, + ITfContext *pContext, + WCHAR *buf, + LONG n, + HRESULT *hrGetSelection, + ULONG *cFetched, + TF_SELECTION *tfSelection) // I4933 { LogEnter(); - TF_SELECTION tfSelection = {0}; - TF_STATUS tfStatus = {0}; - ULONG cFetched; + TF_SELECTION tfSelectionLocal = {0}; + ULONG cFetchedLocal; LONG outn; ITfRange *pRange, *pRangeEnd; HRESULT hr; /* // I4933 First we will try to see if there is any text in the control, and if not, then treat - it as transitory (that is, no ability to read context. This seems to happen in Firefox + it as transitory (that is, no ability to read context. This seems to happen in Firefox with RICHEDIT controls - for example, SourceForge comment fields e.g. reported on page https://sourceforge.net/p/greekpolytonicsp/discussion/general/thread/9b6fa46d/ This also happens in Internet Explorer in the same fields. It is unclear at this point @@ -201,11 +208,11 @@ BOOL GetLeftOfSelection(TfEditCookie ec, ITfContext *pContext, WCHAR *buf, LONG return FALSE; } - BOOL bTreatAsTransitory = FALSE; - if(!SUCCEEDED(hr = pRange->GetText(ec, 0, buf, n, &cFetched))) { + BOOL bTreatAsTransitory = FALSE; + if(!SUCCEEDED(hr = pRange->GetText(ec, 0, buf, n, &cFetchedLocal))) { Log(L"GetLeftOfSelection: Exit -- Failed GetRange (all text to 63 chars) = %x", hr); bTreatAsTransitory = TRUE; - } else if(cFetched == 0) { + } else if(cFetchedLocal == 0) { Log(L"GetLeftOfSelection: Exit -- no text in edit control, treating as transitory"); bTreatAsTransitory = TRUE; } @@ -218,55 +225,62 @@ BOOL GetLeftOfSelection(TfEditCookie ec, ITfContext *pContext, WCHAR *buf, LONG return FALSE; } - /* + /* At this point, we know we can read content from the edit control, so we just need to read the range to the left of the selection - up to (n) characters. */ - if(!SUCCEEDED(hr = pContext->GetSelection(ec, TF_DEFAULT_SELECTION, 1, &tfSelection, &cFetched))) // I3565 + if (!SUCCEEDED(hr = *hrGetSelection = pContext->GetSelection(ec, TF_DEFAULT_SELECTION, 1, tfSelection, cFetched))) // I3565 { Log(L"GetLeftOfSelection: Exit -- Failed GetSelection = %x", hr); // I3565 return FALSE; } - if(cFetched == 0) // I3565 + // copy the values for local processing, preserving the function call variables + cFetchedLocal = *cFetched; + tfSelectionLocal = *tfSelection; + if (tfSelectionLocal.range) { + tfSelectionLocal.range->AddRef(); + } + + if(cFetchedLocal == 0) // I3565 { - Log(L"GetLeftOfSelection: Exit -- cFetched == 0"); // I3565 - if(tfSelection.range != NULL) - tfSelection.range->Release(); + Log(L"GetLeftOfSelection: Exit -- cFetchedLocal == 0"); // I3565 + if(tfSelectionLocal.range != NULL) + tfSelectionLocal.range->Release(); return FALSE; } - - if(!SUCCEEDED(hr = tfSelection.range->Clone(&pRange))) // I3565 + + if(!SUCCEEDED(hr = tfSelectionLocal.range->Clone(&pRange))) // I3565 { Log(L"GetLeftOfSelection: Failed range->Clone = %x", hr); // I3565 - tfSelection.range->Release(); + tfSelectionLocal.range->Release(); return FALSE; } if(!SUCCEEDED(hr = pRange->Collapse(ec, TF_ANCHOR_START))) // I3565 { Log(L"GetLeftOfSelection: Failed range->Collapse = %x", hr); // I3565 - tfSelection.range->Release(); + tfSelectionLocal.range->Release(); pRange->Release(); return FALSE; } if(!SUCCEEDED(hr = pRange->ShiftStart(ec, -n, &outn, NULL))) // I3565 { Log(L"GetLeftOfSelection: Failed range->ShiftStart = %x", hr); // I3565 - tfSelection.range->Release(); + tfSelectionLocal.range->Release(); pRange->Release(); return FALSE; } BOOL result = TRUE; - if(SUCCEEDED(hr = pRange->GetText(ec, 0, buf, n, &cFetched))) // I3565 + if(SUCCEEDED(hr = pRange->GetText(ec, 0, buf, n, &cFetchedLocal))) // I3565 { - buf[cFetched] = 0; + buf[cFetchedLocal] = 0; if(ShouldDebug()) { char *p = debugstr(buf); - Log(L"GetLeftOfSelection(%d) = %hs [%d fetched]", n, p, cFetched); + Log(L"GetLeftOfSelection(%d) = %hs [%d fetched]", n, p, cFetchedLocal); delete[] p; } #ifdef DEBUG_PSEUDO // I3607 @@ -281,7 +295,7 @@ BOOL GetLeftOfSelection(TfEditCookie ec, ITfContext *pContext, WCHAR *buf, LONG } pRange->Release(); - tfSelection.range->Release(); + tfSelectionLocal.range->Release(); return result; // I4933 } diff --git a/windows/src/engine/kmtip/kmkey.cpp b/windows/src/engine/kmtip/kmkey.cpp index 5cbdc097bd..5f7a7f6cbd 100644 --- a/windows/src/engine/kmtip/kmkey.cpp +++ b/windows/src/engine/kmtip/kmkey.cpp @@ -65,7 +65,6 @@ public: HRESULT WINAPI KeymanProcessOutput(int n, PWSTR buf, int nbuf); // I3567 HRESULT WINAPI KeymanGetContext(int n, PWSTR buf, BOOL* isTextSelected); // I3567 - HRESULT WINAPI KeymanIsTextSelected(BOOL* result); HRESULT GetResult() { return _hr; } private: @@ -76,6 +75,8 @@ private: LPARAM _lParam; // I3589 DWORD _dwDeepIntegration; // I4375 TfEditCookie _ec; + HRESULT KeymanIsTextSelected(HRESULT hrGetSelection, ULONG cFetched, TF_SELECTION tfSelection, BOOL *isTextSelected); + ; }; #define KEYEVENT_EXTRAINFO_KEYMAN 0xF00F0000 // I4370 @@ -224,7 +225,9 @@ HRESULT WINAPI CKeymanEditSession::KeymanGetContext(int n, PWSTR buf, BOOL* isTe { LogEnter(); - HRESULT hr; + HRESULT hr, hrGetSelection; + ULONG cFetched; + TF_SELECTION tfSelection = {0}; TF_STATUS tfStatus; *isTextSelected = FALSE; // set to false before any early returns @@ -246,52 +249,31 @@ HRESULT WINAPI CKeymanEditSession::KeymanGetContext(int n, PWSTR buf, BOOL* isTe return S_FALSE; } - if (!GetLeftOfSelection(_ec, _pContext, buf, n)) { // I4933 + if (!GetLeftOfSelection(_ec, _pContext, buf, n, &hrGetSelection, &cFetched, &tfSelection)) { // I4933 return S_FALSE; // I4933 } // now check for selected text - if (!SUCCEEDED(hr = KeymanIsTextSelected(isTextSelected))) { - Log(L"KeymanGetContext: Exit: KeymanIsTextSelected failed"); - // Continue to return S_OK as the IsTextSelected will just be - // defaulting to not selected and the context is more important - //return S_FALSE; + if (!SUCCEEDED(hr = KeymanIsTextSelected(hrGetSelection, cFetched, tfSelection, isTextSelected))) { + Log(L"KeymanGetContext: Warning: KeymanIsTextSelected failed"); + // Continue to return S_OK, as IsTextSelected defaults to not selected. + // If we have gotten this far, the context in buf is valid. } return S_OK; } -HRESULT WINAPI CKeymanEditSession::KeymanIsTextSelected(BOOL* isTextSelected) -{ +HRESULT +CKeymanEditSession::KeymanIsTextSelected(HRESULT hrGetSelection, ULONG cFetched, TF_SELECTION tfSelection, BOOL *isTextSelected) { LogEnter(); HRESULT hr; - TF_STATUS tfStatus; - TF_SELECTION tfSelection; - ULONG cFetched = 0; + *isTextSelected = FALSE; - if (_dwDeepIntegration == DEEPINTEGRATION_DISABLE) { // I4375 - Log(L"KeymanIsTextSelected: Exit: deep integration disabled by registry (or default)"); - return S_FALSE; + if (!SUCCEEDED(hrGetSelection)) { + return E_FAIL; } - - if (!SUCCEEDED(hr = _pContext->GetStatus(&tfStatus))) { // I3568 - Log(L"KeymanIsTextSelected: Exit -- Failed GetStatus = %x", hr); - return hr; - } - - if (tfStatus.dwStaticFlags & TF_SS_TRANSITORY) // I3568 - { - Log(L"KeymanIsTextSelected: Exit: Context does not support manipulation. Using legacy interaction"); - return S_FALSE; - } - - if (!SUCCEEDED(hr = _pContext->GetSelection(_ec, TF_DEFAULT_SELECTION, 1, &tfSelection, &cFetched))) { - Log(L"KeymanIsTextSelected: Exit: -- Failed GetSelection = %x ", hr); - return S_FALSE; - } - - if (hr == TF_E_NOSELECTION) { + if (hrGetSelection == TF_E_NOSELECTION) { // No selection is present return S_OK; } @@ -299,11 +281,14 @@ HRESULT WINAPI CKeymanEditSession::KeymanIsTextSelected(BOOL* isTextSelected) if (cFetched > 0) { // check if the range is empty BOOL isEmpty = FALSE; - LONG lResult = 0; // Compare the start and end of the range - hr = tfSelection.range->IsEmpty(_ec, &isEmpty); - if (SUCCEEDED(hr) && !isEmpty) { + if (!SUCCEEDED(hr = tfSelection.range->IsEmpty(_ec, &isEmpty))) { + Log(L"KeymanIsTextSelected: Exit: Testing range->isEmpty"); + return hr; + } + + if (!isEmpty) { *isTextSelected = TRUE; } tfSelection.range->Release(); From eb1711505885b1db405b699a3ccc0ac701aba3be Mon Sep 17 00:00:00 2001 From: rc-swag <58423624+rc-swag@users.noreply.github.com> Date: Wed, 10 Jul 2024 10:43:08 +1000 Subject: [PATCH 3/3] fix(windows): add comments for clarity --- windows/src/engine/keyman32/appint/aiTIP.cpp | 6 ++++- windows/src/engine/keyman32/appint/aiTIP.h | 5 ++++- .../src/engine/keyman32/kmprocessactions.cpp | 7 ++---- windows/src/engine/kmtip/globals.h | 13 +++++++++++ windows/src/engine/kmtip/inserttext.cpp | 16 +++++++------- windows/src/engine/kmtip/kmkey.cpp | 22 ++++++++++++++++--- 6 files changed, 51 insertions(+), 18 deletions(-) diff --git a/windows/src/engine/keyman32/appint/aiTIP.cpp b/windows/src/engine/keyman32/appint/aiTIP.cpp index 0d0cce987c..cb47cc9e32 100644 --- a/windows/src/engine/keyman32/appint/aiTIP.cpp +++ b/windows/src/engine/keyman32/appint/aiTIP.cpp @@ -275,7 +275,11 @@ BOOL AITIP::ReadContext(PWSTR buf) { } PKEYMAN64THREADDATA _td = ThreadGlobals(); - if(!_td) return FALSE; + + if(!_td) { + return FALSE; + } + if (_td->TIPGetContext && (*_td->TIPGetContext)(MAXCONTEXT - 1, buf, &isTextSelected) == S_OK) { // I3575 // I4262 if(ShouldDebug(sdmKeyboard)) { SendDebugMessageFormat(0, sdmAIDefault, 0, "AITIP::ReadContext: full context [Updateable=%d] %s", _td->TIPFUpdateable, Debug_UnicodeString(buf)); diff --git a/windows/src/engine/keyman32/appint/aiTIP.h b/windows/src/engine/keyman32/appint/aiTIP.h index 62aaaaddc2..a5d84f92a8 100644 --- a/windows/src/engine/keyman32/appint/aiTIP.h +++ b/windows/src/engine/keyman32/appint/aiTIP.h @@ -68,7 +68,10 @@ public: /* TIP interactions */ - BOOL IsLegacy() { return useLegacy; } + BOOL IsLegacy() { + return useLegacy; + } + BOOL IsTextSelected() { return isTextSelected; diff --git a/windows/src/engine/keyman32/kmprocessactions.cpp b/windows/src/engine/keyman32/kmprocessactions.cpp index 412523e389..c5447709c8 100644 --- a/windows/src/engine/keyman32/kmprocessactions.cpp +++ b/windows/src/engine/keyman32/kmprocessactions.cpp @@ -42,12 +42,9 @@ processBack( } } else { - // If there is a selection the TSF documentation suggests an empty string - // to the select range. However this would require a change to QueueAction - // and the ProcessOutput callback. Instead we will pass through the emit the character - // and let the application clear the the selected text. + // If there is a selection emit the key (backspace) + // allowing the application handle clearing selected text in the correct manner. if (app->IsTextSelected() && (vkey == VK_BACK)) { - //SendDebugMessageFormat(0, sdmGlobal, 0, "ProcessBack: Emit Keystroke [%d].", app->IsTextSelected()); *emitKeystroke = TRUE; return; } diff --git a/windows/src/engine/kmtip/globals.h b/windows/src/engine/kmtip/globals.h index a01938f6c5..5ce684524b 100644 --- a/windows/src/engine/kmtip/globals.h +++ b/windows/src/engine/kmtip/globals.h @@ -45,6 +45,19 @@ void DllRelease(); void InsertTextAtSelection(TfEditCookie ec, ITfContext *pContext, const WCHAR *pchText, ULONG cchText); void DeleteLeftOfSelection(TfEditCookie ec, ITfContext *pContext, LONG n); +/** + * Retrieves the text to the left of the current selection in the context. + * In addition it also returns the output values of the TSF call 'GetSelection'. + * + * @param[in] ec The edit cookie. + * @param[in] pContext The text input context. + * @param[out] buf The buffer to store the text. + * @param[in] n The maximum number of characters to retrieve. + * @param[out] hrGetSelection The result of the 'GetSelection' call. + * @param[out] cFetched The number of selections fetched. + * @param[out] tfSelection The selection details. + * @return TRUE if successful, otherwise FALSE. + */ BOOL GetLeftOfSelection(TfEditCookie ec, ITfContext *pContext, WCHAR *buf, LONG n, HRESULT *hrGetSelection, ULONG *cFetched, TF_SELECTION *tfSelection); // I4933 void GetDeadkeyFlags(TfEditCookie ec, ITfContext *pContext, PWSTR buf, int n); diff --git a/windows/src/engine/kmtip/inserttext.cpp b/windows/src/engine/kmtip/inserttext.cpp index d296d0880b..d2a38c46ff 100644 --- a/windows/src/engine/kmtip/inserttext.cpp +++ b/windows/src/engine/kmtip/inserttext.cpp @@ -165,13 +165,13 @@ char *debugstr(PWSTR buf) { BOOL GetLeftOfSelection( - TfEditCookie ec, - ITfContext *pContext, - WCHAR *buf, - LONG n, - HRESULT *hrGetSelection, - ULONG *cFetched, - TF_SELECTION *tfSelection) // I4933 + TfEditCookie ec, + ITfContext *pContext, + WCHAR *buf, + LONG n, + HRESULT *hrGetSelection, + ULONG *cFetched, + TF_SELECTION *tfSelection) // I4933 { LogEnter(); @@ -242,7 +242,7 @@ GetLeftOfSelection( if (tfSelectionLocal.range) { tfSelectionLocal.range->AddRef(); } - + if(cFetchedLocal == 0) // I3565 { Log(L"GetLeftOfSelection: Exit -- cFetchedLocal == 0"); // I3565 diff --git a/windows/src/engine/kmtip/kmkey.cpp b/windows/src/engine/kmtip/kmkey.cpp index 5f7a7f6cbd..3bbee00b55 100644 --- a/windows/src/engine/kmtip/kmkey.cpp +++ b/windows/src/engine/kmtip/kmkey.cpp @@ -64,6 +64,13 @@ public: STDMETHODIMP DoEditSession(TfEditCookie ec); HRESULT WINAPI KeymanProcessOutput(int n, PWSTR buf, int nbuf); // I3567 + /** + * Retrieves the current context and checks if text is selected. + * @param[in] n The size of the buffer. + * @param[out] buf The buffer to store the context. + * @param[out] isTextSelected A flag indicating whether text is selected. + * @return S_OK if successful, otherwise an HRESULT error code. + */ HRESULT WINAPI KeymanGetContext(int n, PWSTR buf, BOOL* isTextSelected); // I3567 HRESULT GetResult() { return _hr; } @@ -75,8 +82,16 @@ private: LPARAM _lParam; // I3589 DWORD _dwDeepIntegration; // I4375 TfEditCookie _ec; + /** + * Checks if any text is selected in the given context. + * + * @param[in] hrGetSelection The result of a call to GetSelection selection. + * @param[in] cFetched The number of selections fetched. + * @param[in] tfSelection The selection details. + * @param[out] isTextSelected A flag indicating whether text is selected. + * @return S_OK if successful, otherwise an HRESULT error code. + */ HRESULT KeymanIsTextSelected(HRESULT hrGetSelection, ULONG cFetched, TF_SELECTION tfSelection, BOOL *isTextSelected); - ; }; #define KEYEVENT_EXTRAINFO_KEYMAN 0xF00F0000 // I4370 @@ -256,8 +271,9 @@ HRESULT WINAPI CKeymanEditSession::KeymanGetContext(int n, PWSTR buf, BOOL* isTe // now check for selected text if (!SUCCEEDED(hr = KeymanIsTextSelected(hrGetSelection, cFetched, tfSelection, isTextSelected))) { Log(L"KeymanGetContext: Warning: KeymanIsTextSelected failed"); - // Continue to return S_OK, as IsTextSelected defaults to not selected. - // If we have gotten this far, the context in buf is valid. + // Continue to return S_OK, even though checking if a text selection exists has failed. + // Reaching this point means the context in 'buf' is valid and will be passed to the caller. + // isTextSelected will be false, which is better than returning E_FAIL at this point. } return S_OK; }