Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(windows): add api GetContext and TIPProcessKeyEx with IsTextSelected bool #13023

Open
wants to merge 2 commits into
base: fix/windows/7870/add-is-text-selected-flag-api
Choose a base branch
from

Conversation

rc-swag
Copy link
Contributor

@rc-swag rc-swag commented Jan 24, 2025

as extension of #12649
This adds GetContextEx and TIPProcessKeyEx API call to handle having a output bool parameter that is Bool is there is text selected.

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
Copy link

keymanapp-test-bot bot commented Jan 24, 2025

User Test Results

Test specification and instructions

Test Artifacts

@keymanapp-test-bot keymanapp-test-bot bot added this to the A18S20 milestone Jan 24, 2025
@rc-swag rc-swag requested a review from mcdurdin January 24, 2025 04:01
Add the api calls that include the output bool isSelected, which is
true if text is selected.
@rc-swag rc-swag changed the base branch from master to fix/windows/7870/add-is-text-selected-flag-api January 24, 2025 04:05
@mcdurdin mcdurdin changed the title fix(windows): add api GetConext and TIPProcessKeyEx with IsTextSelected bool fix(windows): add api GetContext and TIPProcessKeyEx with IsTextSelected bool Jan 24, 2025
@@ -208,6 +209,7 @@ typedef struct tagKEYMAN64THREADDATA

PKEYMANPROCESSOUTPUTFUNC TIPProcessOutput;
PKEYMANGETCONTEXTFUNC TIPGetContext;
PKEYMANGETCONTEXTISSELECTEDFUNC TIPGetContextEx;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These names should really match -- should be either:

Suggested change
PKEYMANGETCONTEXTISSELECTEDFUNC TIPGetContextEx;
PKEYMANGETCONTEXTISSELECTEDFUNC TIPGetContextIsSelected;

or

Suggested change
PKEYMANGETCONTEXTISSELECTEDFUNC TIPGetContextEx;
PKEYMANGETCONTEXTEXFUNC TIPGetContextEx;

Comment on lines 209 to +215
_hr = E_FAIL; // TODO: use S_FALSE -> this tells _KeymanProcessKeystroke to pass keystroke on
}

if (!Keyman32Interface::TIPProcessKeyEx(_wParam, _lParam, ExtKeymanProcessOutput, ExtKeymanGetContextIsSelected, _fUpdate, _fPreserved)) {
SendDebugMessage(L"TIPProcessKeyEx did not handle the keystroke");
_hr = E_FAIL; // TODO: use S_FALSE -> this tells _KeymanProcessKeystroke to pass keystroke on
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code is puzzling to me -- why are you calling both TIPProcessKey and TIPProcessKeyEx? Shouldn't be either one or the other?

@dinakaranr
Copy link

dinakaranr commented Jan 24, 2025

Test Results

I tested this issue with the attached "18.0.177-alpha-test-13023" build(24/01/2025) on Windows 10 OS. Here is my observation.

  • TEST_DELETE_SELECTED_TEXT (passed):
  1. Installed the "Keyman-18.0.177.exe" file from this PR build.
  2. Installed the EuroLatin keyboard.
  3. Select the keyboard in the machine.
  4. Open the WordPad and Notepad.
  5. Type mySchool word
  6. Select School and type the backspace key.
  7. Verified that the "my" appeared.
  • TEST_BACKSPACE_NO_SELECTION (passed):
  1. Installed the "Keyman-18.0.177.exe" file from this PR build.
  2. Installed the EuroLatin keyboard.
  3. Select the keyboard in the machine.
  4. Open the WordPad and Notepad.
  5. Type mySchool word
  6. Move the cursor still to the right of "l"
  7. Type the backspace key.
  8. Verified that the "mySchoo" appeared.
  • TEST_SELECTION_TYPE_CHAR (passed):
  1. Installed the "Keyman-18.0.177.exe" file from this PR build.
  2. Installed the EuroLatin keyboard.
  3. Select the keyboard in the machine.
  4. Open the WordPad and Notepad.
  5. Type mySchool word
  6. Select School and type the "t" letter.
  7. Verified that the "myt" appeared.
    It works well. Thank you.
    Notes: I tested this issue in two Windows 10(1 Native machine, 1 virtual machine) and Windows 11. I observed that the native machine of Windows 10 had a letter appearing two times when typing one time.

@keymanapp-test-bot keymanapp-test-bot bot removed the user-test-required User tests have not been completed label Jan 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

3 participants