fix(windows): set basekeyboard as current user not the admin user on a elevated process. - #16162
fix(windows): set basekeyboard as current user not the admin user on a elevated process.#16162rc-swag wants to merge 4 commits into
Conversation
User Test ResultsTest specification and instructions
Results TemplateTest Artifacts |
mcdurdin
left a comment
There was a problem hiding this comment.
I don't think this is correct. The base keyboard dialog needs to run elevated -- because when you change the base keyboard, Keyman needs to mcompile each of the installed keyboards against the new base keyboard:
keyman/windows/src/engine/kmcomapi/com/options/keymanoptions.pas
Lines 133 to 135 in 1536a5f
The problem here is that the base layout setting is saved against the Admin user, but needs to be saved against the current user. This probably is best solved by splitting the admin component -- mcompiling -- out of the TKeymanOptions.Apply function, and running it as a separate step from the Base Keyboard dialog. Then the Base Keyboard dialog does not show elevated, but just elevates when OK is clicked, if it detects that new mcompiles need to be run, and does that as a kmshell -mcompile <basekeylayoutid> call? (implementation calls: TKeymanKeyboardInstalled.UpdateBaseLayout for each installed keyboard).
TKPRecompileMnemonicKeyboard then needs a parameter for the base layout, rather than reading it from the context options:
So some plumbing required, sadly.
This change may not have been the prefered change but it worked. It does run as an elevated but the process is created as a current user. |
|
Did you verify that this worked with a baselayout you had never selected previously? How could the mcompiled .kmx files be written to the C:\ProgramData folder if the base layout steps are run non-elevated? One reasonably straightforward way to address this, for starting as a non-elevated user:
Also:
|
Need to verify that the relevant files are saved in ProgramData too and that the base keyboard is mapped as expected! |
Test Prerequisites
Test Specs
Test Results
Note The Keyman Configuration window becomes unclickable until I click out of the app and click back. It does not show unresponding.
|
Note, if a crash dialog appears, FAIL the test. Also, please Copy to Clipboard and paste it into the test report. TEST_BASE_KEYBOARD_CURRENT_USER (FAIL): a crash dialog appeared |
2d6a4ee to
2c48025
Compare
The original change, changed the InKeymanKyboardInstalled UpdateBaseLayout inteface and it didn't need to. This change restores it.
|
Test-bot: retest TEST_BASE_KEYBOARD_CURRENT_USER |
Test Specs
Test Results
|

Fixes: #15152
This was quicker fix then I thought the more through issue 15154 still remains to catch discover all the cases the non-admin user configuration changes that may been attempting.
Build-bot: release:windows
User Testing
TEST_BASE_KEYBOARD_CURRENT_USER
Login into Windows with and account that is a "standard" user and does not have "Administrator" rights.
Install the Keyman from this PR
Open Keyman Configuration -> Keyboard Layouts
Install a keyboard for example sil_ipa
Open Keyman Configuration -> Options
Check
C:\ProgramData\Keyman\Keyman Engine\Keyboard\_Package\sil_ipadon't delete the basesil_ipa.kmxbut delete any with KLIDs for examplesil_ipa-00000409-d.kmxClick Base Keyboard, change the Base Keyboard to German. You will need to enter the login details for a Admin user.
Confirm the Keyboard changes for the current user and not the Admin user used for the elevated processs.
Check
C:\ProgramData\Keyman\Keyman Engine\Keyboard\_Package\sil_ipathere should now be asil_ipa-????0407-d.kmxandsil_ipa-????0407.kmxTEST_BASE_KEYBOARD_CURRENT_USER_KMX_EXISTS
After completing the steps in
TEST_BASE_KEYBOARD_CURRENT_USERCheck German mcomplied kmx is still there i.e.
C:\ProgramData\Keyman\Keyman Engine\Keyboard\_Package\sil_ipathere should now be asil_ipa-????0407-d.kmxandsil_ipa-????0407.kmx6.Click Base Keyboard, change the Base Keyboard to German. You should Not be asked to enter a admin user.