fix(windows): add text selected bool emit backspace key when text selected in TSF

[rc-swag]

Fixes: #7870
Add a bool to the KeymanGetContext call, the Windows engine will emit backspace keystroke when text is selected and a backspace key is pressed allowing the application to make the correct decision on how to handle it.

This change feels like it is half the solution the full solution would be to have a mechanism for allowing the Keyman Core to know if text has been selected. &hasSelection As discussed in issue #9073 as a Future solution.

With this change the core will request the character closest to the selection to be deleted its internal cache will match this, even though the engine will essentially overide this request and just emit the backspace keystroke. As it is TSF - context aware it will be updated on the next key stroke anyway with the correct context.

User Testing

TEST_DELETE_SELECTED_TEXT

Install the Keyman for Windows build artifact with this PR.
This test must be done with an application using the Text Services Framework (TSF). You cannot use
the Web version of Word. Use WordPad that comes installed with Windows 10 and Windows 11.

Install a Keyman keyboard like EuroLatin

1. Select the EuroLatin Keyboard
2. Open MS Word or WordPad
3. Type mySchool
4. Select School and type backspace

Expected result
my

Some regression tests.

TEST_BACKSPACE_NO_SELECTION

Install a Keyman keyboard like euro_latin

1. Select the EuroLatin Keyboard
2. Open MS Word or WordPad
3. Type mySchool
4. With the cursor still to the right of 'l' type backspace

Expected result
mySchoo

TEST_SELECTION_TYPE_CHAR

1. Select the EuroLatin Keyboard
2. Open MS Word or WordPad
3. Type mySchool
4. Select School and type t

Expected result
myt

[keymanapp-test-bot]

User Test Results

Test specification and instructions

• ✅ TEST_DELETE_SELECTED_TEXT (PASSED) (notes)
• ✅ TEST_BACKSPACE_NO_SELECTION (PASSED) (notes)
• ✅ TEST_SELECTION_TYPE_CHAR (PASSED) (notes)

Test Artifacts

• Windows
  • Keyman for Windows
  • FirstVoices Keyboards for Windows
  • Text Editor (32 bit)
  • Text Editor (64 bit)

[rc-swag]

When KeymanGetContext calls GetLeftofSelection add the cFetched, tfSelection and hr result as parameters so the the TSF API call GetSelection is just called once and the information will be available to IsKeymanIsTextSelected, no need for it to make that API call again. Make KeymanIsTextSelected a private function as it should not be called independently of KeymanGetContext.

[dinakaranr]

Test Results

I tested this issue with the attached "keyman-18.0.61-alpha-test-11884" build on the Windows 10 & 11 OS environment: Here is my observation.

Prerequisite steps:

1. Installed the "keyman-18.0.61.exe" file.
2. Keyman keyboard added in the system tray.
3. Open the keyman "Configuration" window.
4. Add the "EuroLatin" keyboard by clicking the "Download keyboard" button.

• TEST_DELETE_SELECTED_TEXT (Passed):

1. Followed the above prerequisite steps:
2. Open the Wordpad.
3. Type "mySchool" and select "School" by clicking the mouse.
4. Type the "Backspace" key.
   Results: Here, It is showing the "my" word in the Wordpad.

• TEST_BACKSPACE_NO_SELECTION (Passed):

1. Followed the above prerequisite steps:
2. Open the Wordpad.
3. Type "myschool" and then move the cursor to the right of "l"
4. Type the "Backspace" key.
   Results: Here, It is showing the "mySchoo" word in the Wordpad.

• TEST_SELECTION_TYPE_CHAR (Passed):

1. Followed the above prerequisite steps:
2. Open the Wordpad.
3. Type "mySchool" and select "School" by clicking the mouse.
4. Type the "t" key.
   Results: Here, It is showing the "myt" word in the Wordpad.
   It is working as per the expected results. Thank you.

[keyman-server]

Changes in this pull request will be available for download in Keyman version 18.0.73-alpha

[mcdurdin]

One concern here is that changing an existing API endpoint can be catastrophic if we end up with divergent versions of the DLLs loaded (e.g. if a user doesn't restart Windows after upgrading Keyman -- then any processes with Keyman loaded can crash). Is is usually safer to add another API and mark the original as deprecated; the same checks for whether the API is available can be used.

[mcdurdin]

I know this is after-merge, but I had some concerns... sorry!

[mcdurdin]

This shouldn't be here -- it doesn't correspond to acquisition of the range. It should be done in the caller -- even though it is implicitly calling tfSelection.range->AddRef() due to the call to GetLeftOfSelection, as least that clarifies the lifecycle due to the variable being local to that function. Also helps where range is not released on line 303 above.

[mcdurdin]

It feels weird to be passing hrGetSelection and cFetched in to this function. It seems like we could avoid a bunch of complexity by making this a simple BOOL return value if we didn't have to control for errors -- you don't use the hr result in the caller, so just log failure within the function and return TRUE/FALSE.

  // now check for selected text
  isTextSelected = KeymanIsTextSelected(hrGetSelection, cFetched, tfSelection);

  if(tfSelection.range) {
    tfSelection.range->Release();
  }

  // ...

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;

  // 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;
  }

  return !isEmpty;
}

[mcdurdin]

@rc-swag just following up on the concerns I noted post-merge