fix(windows): bool return value of KeymanIsTextSelected

keymanapp · Dec 19, 2024 · 725bfa7 · 725bfa7
1 parent 0501e45
commit 725bfa7
Showing 1 changed file with 20 additions and 35 deletions.
diff --git a/windows/src/engine/kmtip/kmkey.cpp b/windows/src/engine/kmtip/kmkey.cpp
@@ -69,7 +69,7 @@ class CKeymanEditSession : public CEditSessionBase
    * @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.
+   * @return                      BOOL if successful.
    */
   HRESULT WINAPI KeymanGetContext(int n, PWSTR buf, BOOL* isTextSelected);   // I3567
   HRESULT GetResult() { return _hr; }
@@ -88,10 +88,9 @@ class CKeymanEditSession : public CEditSessionBase
    * @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);
+  BOOL CKeymanEditSession::KeymanIsTextSelected(HRESULT hrGetSelection, ULONG cFetched, TF_SELECTION tfSelection);
 };
 
 #define KEYEVENT_EXTRAINFO_KEYMAN 0xF00F0000   // I4370
@@ -275,47 +274,33 @@ HRESULT WINAPI CKeymanEditSession::KeymanGetContext(int n, PWSTR buf, BOOL* isTe
   }
 
   // now check for selected text
-  if (!SUCCEEDED(hr = KeymanIsTextSelected(hrGetSelection, cFetched, tfSelection, isTextSelected))) {
-    SendDebugMessage(L"Warning: KeymanIsTextSelected failed");
-    // 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.
+  *isTextSelected = KeymanIsTextSelected(hrGetSelection, cFetched, tfSelection);
+
+  if(tfSelection.range) {
+    tfSelection.range->Release();
   }
+
   return_SendDebugExit(S_OK);
 }
 
-HRESULT
-CKeymanEditSession::KeymanIsTextSelected(HRESULT hrGetSelection, ULONG cFetched, TF_SELECTION tfSelection, BOOL *isTextSelected) {
-  SendDebugEntry();
+BOOL
+CKeymanEditSession::KeymanIsTextSelected(HRESULT hrGetSelection, ULONG cFetched, TF_SELECTION tfSelection) {
+  if (hrGetSelection == TF_E_NOSELECTION || cFetched == 0 || !SUCCEEDED(hrGetSelection) || !tfSelection.range) {
+    // No selection is present, or an error occurred trying to get the selection
+    return FALSE;
+  }
 
+  // check if the range is empty
   HRESULT hr;
+  BOOL isEmpty = TRUE;
 
-  *isTextSelected = FALSE;
-
-  if (!SUCCEEDED(hrGetSelection)) {
-    return_SendDebugExit(E_FAIL);
+  // Compare the start and end of the range
+  if (!SUCCEEDED(hr = tfSelection.range->IsEmpty(_ec, &isEmpty))) {
+    SendDebugMessageFormat(L"KeymanIsTextSelected: Exit: Testing range->isEmpty failed with %x", hr);
+    return FALSE;
   }
-  if (hrGetSelection == TF_E_NOSELECTION) {
-    // No selection is present
-    return_SendDebugExit(S_OK);
-  }
-
-  if (cFetched > 0) {
-    // check if the range is empty
-    BOOL isEmpty = FALSE;
-
-    // Compare the start and end of the range
-    if (!SUCCEEDED(hr = tfSelection.range->IsEmpty(_ec, &isEmpty))) {
-      SendDebugMessage(L"Exit: Testing range->isEmpty");
-      return_SendDebugExit(hr);
-    }
 
-    if (!isEmpty) {
-      *isTextSelected = TRUE;
-    }
-    tfSelection.range->Release();
-  }
-  return_SendDebugExit(S_OK);
+  return !isEmpty;
 }
 
 BOOL CKMTipTextService::TIPNotifyActivate(GUID *guidProfile)   // I3581   // I4274