chore(windows): remove legacy core and flag ⛏️#8593
Conversation
Removing the legacy code that was replaced by the core. Also removing the feature flag for core integeration.
User Test ResultsTest specification and instructions ✅ SUITE_BASELINE: Keyman for Windows Base Line Tests10 tests in 2 groups PASSED
✅ SUITE_CAPSLOCK:30 tests in 2 groups PASSED
✅ SUITE_TSF_APP:6 tests in 2 groups PASSED
🟥 SUITE_IMX_KEYBOARDS:
✅ SUITE_STORE_AND_CONTEXT:8 tests in 2 groups PASSED
Retesting TemplateTest Artifacts
|
| RefreshPreservedKeys(TRUE); | ||
| return TRUE; | ||
| // happy to use while(!done) pattern | ||
| fail: |
There was a problem hiding this comment.
I realised there was previously a possible memory leak here in not deleting _td_>lpActiveKeyboard I am using a goto here. If we don't like that I could maybe use while(!done) {} instead
There was a problem hiding this comment.
This LGTM although see my comment about removing Keyman_ForceKeyboard and Keyman_StopForcingKeyboard altogether 😁
SUITE_BASELINE: Keyman for Windows Base Line TestsGROUP_WIN10:
|
mcdurdin
left a comment
There was a problem hiding this comment.
LGTM. I wonder if there are other functions we can cleanup as a result of this (e.g. extended string management, etc). But those can come as separate work.
| LoadDLLs(&_td->lpKeyboards[i]); | ||
|
|
||
| LoadKeyboardOptions(&_td->lpKeyboards[i]); | ||
| LoadKeyboardOptionsREGCore(&_td->lpKeyboards[i], _td->lpKeyboards[i].lpCoreKeyboardState); |
There was a problem hiding this comment.
Curious why you didn't rename this LoadKeyboardOptionsREGCore given you renamed LoadlpKeyboardCore to LoadlpKeyboard?
There was a problem hiding this comment.
Good question. The LoadlpKeyboardCore was more about distinguishing it from the legacy equivalent function. The LoadKeyboardOptionsREGCore is more about loading registry options to Core. However, it made me think and go over all the functions with Core. I will create PR to clean this up. #8703
| */ | ||
| BOOL Globals::InitSettings() { | ||
| /* Check for common core vs windows core */ | ||
| f_CoreIntegration = Reg_GetDebugFlag(REGSZ_Flag_UseKeymanCore, TRUE); |
There was a problem hiding this comment.
The constant REGSZ_Flag_UseKeymanCore should also be deleted from the relevant .h file?
There was a problem hiding this comment.
ahh missed that thanks
| RefreshPreservedKeys(TRUE); | ||
| return TRUE; | ||
| // happy to use while(!done) pattern | ||
| fail: |
There was a problem hiding this comment.
This LGTM although see my comment about removing Keyman_ForceKeyboard and Keyman_StopForcingKeyboard altogether 😁
| _td->lpActiveKeyboard->IMDLLs = NULL; | ||
| _td->lpActiveKeyboard->KeyboardOptions = NULL;*/ |
There was a problem hiding this comment.
Let's get rid of this commented code
There was a problem hiding this comment.
As a follow-up PR, I think we should remove Keyman_ForceKeyboard altogether. It's totally legacy now, as anyone wanting to use a keyboard directly should host Keyman Core (as Keyman Developer Debugger does). We only use it in some really old manual tests (I checked) -- which we can leave alone at present.
That will be MUCH tidier, because the whole messy ownership of _td->lpActiveKeyboard problem goes away.
|
I think we can remove |
|
addins.cpp/addins.h should also be deleteable I think |
SUITE_CAPSLOCK:GROUP_WIN10:
|
| */ | ||
|
|
||
| if(mp->message == WM_UNICHAR && Addin_ShouldProcessUnichar(mp->hwnd)) | ||
| if(mp->message == WM_UNICHAR) |
There was a problem hiding this comment.
@mcdurdin I blindly removed addins - I don't know what they did. Should WM_UNICHAR be removed totally as it seems addins was the only function interested in the event. ?
There was a problem hiding this comment.
I think WM_UNICHAR is very much dead at this point. But perhaps it should be removed entirely in a separate PR (and also remove Keyman_ForceKeyboard in a separate PR too) -- that way we have PR# to reference them for potential future questions.
SUITE_TSF_APP:GROUP_WIN10:
|
SUITE_IMX_KEYBOARDS:GROUP_WIN10:
|
SUITE_STORE_AND_CONTEXT:GROUP_WIN10:
|
SUITE_BASELINE: Keyman for Windows Base Line TestsGROUP_WIN11:
|
SUITE_CAPSLOCK:GROUP_WIN11:
|
| WCHAR charCode; // I4582 | ||
| BOOL windowunicode; // I4287 | ||
| BOOL isDown; | ||
| LPKEYBOARD lpkb; |
…e-functions chore(window): rename core functions ⛏️
…unichar chore(windows): remove WM_UNICHAR completely
chore(windows): remove addins ⛏️
…e/windows/8646/remove-force-keyboard
…ce-keyboard chore(windows): remove keyman_forcekeyboard
Co-authored-by: Eberhard Beilharz <ermshiperete@users.noreply.github.com>
…legacy-keyman-core
|
The failed tests fail currently on Windows 11 with Keyman 15 and up. #8697 has been created to deal with this. Therefore can merge this PR. |
|
Changes in this pull request will be available for download in Keyman version 17.0.109-alpha |
Fixes: #5442
Removing the legacy code that was replaced by the core. Also removing the feature flag for core integration.
User Testing
This will require a full acceptance test just to insure coverage of all the functionality.
Keyman for Windows Acceptance Test Procedures
These test procedures are to be run before moving from alpha to beta, or beta to stable, or before PRs are merged into stable branches.
Results are annotated with results, use
>at the start of a new line under the checkbox/Test to note the result.User Testing
Gather Assets for Testing
app/windows/src/test/manual-tests/caps-lock-stores.Setup Steps
SUITE_BASELINE: Keyman for Windows Base Line Tests
Test cases
click to expand
TEST_INSTALL:
TEST_KEYBOARD_INSTALLATION_REMOTE:
TEST_INSTALL_PKG_DISK:
TEST_KEYBOARD_OUTPUT:
TEST_ON_SCREEN_KEYBOARD:
SUITE_CAPSLOCK:
Caps Lock
The test keyboard layouts are found in the keyman repo at
app/windows/src/test/manual-tests/caps-lock-stores. There is a project file for the 3 keyboards used in this test. The project file can be used to build the keyboard packages, but you can conveniently use the.kmpfile zipped and included respectively below.The test cases below expect the usage of the
capslock.kmp.zipkeyboard. That keyboard outputs pass or fail if following the test cases.Prerequisites before each test
capslock.kmp.Test cases
click to expand
TEST_CAPSLOCK-1: uppercase with virtual key
aExpected result:
pass.(with other keyboards uppercaseA)TEST_CAPSLOCK-2: lowercase with virtual key
bShiftExpected result:
pass.(with other keyboards lowercaseb)TEST_CAPSLOCK-3: capslock ignored for numbers
3ShiftExpected result:
pass.(with other keyboards#)TEST_CAPSLOCK-4: uppercase
cExpected result:
pass.(with other keyboards uppercaseC)TEST_CAPSLOCK-5: lowercase
dShiftExpected result:
pass.(with other keyboards lowercased)CapsAlwaysOff
For these tests, use a keyboard with the
caps_always_off.kmp.zipstore set. We call this keyboard capsalwaysoff below.Any keyboard with that store set will work; if you don't have one at hand you can use the
caps_always_off.kmpkeyboard. The caps_always_off.kmp keyboard will prevent switching caps lock on. As a sanity check to verify that Keyman is actually active, pressing the keyawill outputncaps_little_a, andShift+awill outputncaps_shift_A.Note: When testing in a virtual machine, use an on-screen keyboard (in VirtualBox: Input/Keyboard/Soft Keyboard) and observe the caps lock indicator of the on-screen keyboard. Using the hardware keyboard might show side effects with caps lock.
Prerequisites before each test
CapsAlwaysOffstore set, e.g.caps_always_off.kmp.Test cases
click to expand
TEST_CAPSOFF-1: sanity check
aExpected result:
ncaps_little_aTEST_CAPSOFF-2: caps lock stays off
CapsLockkeyaExpected result:
ncaps_little_aTEST_CAPSOFF-3: no caps lock while holding capslock key
CapsLockkeyaCapsLockkeyExpected result:
ncaps_little_aTEST_CAPSOFF-4: no caps lock while holding capslock key
CapsLockkeyShiftkeyaCapsLockandShiftkeysExpected result:
ncaps_shift_ATEST_CAPSOFF-5: switching turns off caps lock
aExpected result:
ncaps_little_aSHIFT: CapsOnOnly/ShiftFreesCaps
For these tests, use a keyboard with the
CapsOnOnlyandShiftFreesCapsstores set. We call this keyboard shift_frees_caps below.Any keyboard with these stores set will work; if you don't have one at hand you can use the
shift_frees_caps.kmp.zipkeyboard.The shift_frees_caps.kmp keyboard will enable caps lock by pressing the
CapsLockkey, and will turn capslock off by pressing theShiftkey. The keyboard outputs pass or fail if following the test cases.Note: When testing in a virtual machine, use an on-screen keyboard (in VirtualBox: Input/Keyboard/Soft Keyboard) and observe the caps lock indicator of the on-screen keyboard. Using the hardware keyboard might show side effects with caps lock. Except for TEST_CAPSONLY-5 which can only be reliably tested on a hardware keyboard on host OS (not a VM). For windows 10 and windows 11 with a virtual box vm-onscreen keyboard, the following happens. The VM soft keyboard does NOT actually send the Shift Shift Key Stroke through but rather will change the keys pressed for example if an
ais pressed the soft keyboard itself will change that key to aA. This means we can't Test TEST_CAPONLY-5 on a soft keyboard.Prerequisites before each test
CapsOnOnlyandShiftFreesCapsstores set, e.g.shift_frees_caps.kmp.Test cases
click to expand
TEST_CAPSONLY-1: no caps
1Expected result:
pass.TEST_CAPSONLY-2: caps
CapsLock2Expected result:
pass.TEST_CAPSONLY-3: caps doesn't toggle
CapsLockCapsLock6Expected result:
pass.TEST_CAPSONLY-4: shift turns off
CapsLockShift3ShiftExpected result:
pass.TEST_CAPSONLY-5: shift by itself turns off
Be aware of limitations when testing this on virtual machines as noted above.
CapsLockShiftExpected result:
SUITE_TSF_APP:
Background: Some windows applications use the windows TSF these include: MS Word, Firefox, Fieldworks and the windows search field on the taskbar.
Setup Text Services Framework(TSF) testing with Keyman for Windows
Install Keyman Developer version 15 or greater.
Open the Keyman Developers Test page in Firefox. It must be
Firefox Browseri. By default this is at
localhost:8008so you could type this location in to the firefox address bar.ii. You can also access it by pressing the
Test package on webbutton found in theCompiletab for any keyboard package.On the
Keymand Developer Keyboard Test Pageuse theKeyboarddropdown to selectsystem keyboardUse the Keyman for Windows icon located in the

System Trayto select the desired Keyman for the Windows keyboard.Typing text into the input field will now use the TSF with the keyboard selected in step 4.
Test cases: double processing
click to expand
To run these tests the EuroLatin (SIL) Keyboard from here.
GROUP_WIN10
GROUP_WIN11
TEST_DOUBLE_PROCESSING_FIREFOX:
Setup Text Services Framework(TSF) testing with Keyman for Windowabove selecting the EuroLatin Keyboard at step 4.abcdBackspaceabcTEST_DOUBLE_PROCESSING_SEARCHBAR:
abcdBackspaceabcTEST_DOUBLE_PROCESSING_NOTEPAD:
abcdabcdSUITE_IMX_KEYBOARDS
Test IMX keyboards still work after core integration. see tests suite on #5936 and also #6187
This is the keyboard to be tested. imsample.zip
If an IMTest keyboard is already installed you will need to uninstall it first, restart windows and then install the Imsample keyboard attached to this PR.
I have copied the tests from #5936 and made a few modifications.
The overall test we are aiming for is that in doing the tests below the keyboard does not cause a keyman or app crash.
Test cases
click to expand
IMSAMPLE_KEYBOARD_INLINE_MENU:
This keyboard uses the letters
aeomto allow IMX input. This keyboard is a bit flakey being a demo and not polished.It also takes some setting up the following registry keys need to be added and set once installed.
The location of the registry keys are at.
Computer\HKEY_CURRENT_USER\SOFTWARE\Keyman\Keyman Engine\Active Keyboards\imsampleand are of type REG_DWORD
The keys are
ShowIMWindowandShowIMWindowAlwaysthey both need to be set to 0.In this mode IM window is not shown rather the app text is updated with a menu when either
aeomis pressed.For example when type
hethe text will becomeh[1ɛ 2ɜ 3ə 4e 5ɘ]then you type the number3the text displayed should now behəTEST_IMSAMPLE_INPUT:
IMTesttryspacekey thenathen type 1 to get the 1st optiontry æTEST_IMSAMPLE_INPUT_CONT:
... contd from above
###optiontry æ###TEST_IMSAMPLE_BACKSPACE:
Using Notepad or Libreoffice
fe[]should be deleted.fIMSAMPLE_KEYBOARD_IM_WINDOW:
This keyboard uses the letters

aeomto allow IMX input. This time a IM window should display.The first time you use the keyboard with an application the registry keys will be created and by default set to 1.
If the IM window is not appearing you will need to verify the value of two registry keys.
The location of the registry keys are at.
Computer\HKEY_CURRENT_USER\SOFTWARE\Keyman\Keyman Engine\Active Keyboards\imsampleand are of type REG_DWORD
The keys are
ShowIMWindowandShowIMWindowAlwaysthey both need to be set to 1.Note for the IM window you need to use the mouse pointer to select the options, you can't type the number.
TEST_IMSAMPLE_INPUT_IMW:
IMTesttryspacekey thenathen click the 1st optiontry æTEST_IMSAMPLE_INPUT_IMW_CONT:
... contd from above
###optiontry æ###SIMPLIFIED_CHINESE:
This keyboard will display the IMX window as soon as a string the matches the pinyin for one or more characters are typed.
For all these tests install the Simplified Chinese Keyboard cs-pinyin.cmp found here
TEST_SIMPLIFIED_CHINESE_SINGLE:
Using Notepad or equivalent.
o- The IMX window should appearTEST_SIMPLIFIED_CHINESE_MULTIPLE:
Using Notepad or equivalent.
hanzi- The IMX window will appear and in the top left the lettershanziwill be presentTEST_SIMPLIFIED_CHINESE_BACKSPACE_1:
Using Notepad or equivalent.
This is to test the backspace occurring mid pinyin sequence before a character is output to the app
hanzi- The IMX window will appear and in the top left the lettershanziwill be presenthan.TEST_SIMPLIFIED_CHINESE_BACKSPACE_2:
Using Notepad or equivalent.
This is to test the backspace of already output characters.
hanzi- The IMX window will appear and in the top left the lettershanziwill be present.SUITE_STORE_AND_CONTEXT
Test the options keyboard stores and context updating as a result of keystroke input. Introduced after #7077 and #7809 there are more tests on those PRs.
Test cases
click to expand
The key referred to as "enter" is alternatively labelled "return" on some keyboards.
Keyboards for the following test are found in the zip file store_context_kbd.zip
TEST_21_OPTION_STORE:
Expected Result: "no foo.foo.no foo"
TEST_OUTPUT_KEYSTROKE
Expected Result: "abd3"
TEST_OUTPUT_KEYSTROKE_INVALID_CONTEXT
Expected Result:
"abc
3"
TEST_DEADKEY_AND_CONTEXT
Expected Result: "correct"