Skip to content

fix(windows): set basekeyboard as current user not the admin user on a elevated process.#16162

Open
rc-swag wants to merge 3 commits into
masterfrom
fix/windows/15152/basekeyboard-user-not-admin
Open

fix(windows): set basekeyboard as current user not the admin user on a elevated process.#16162
rc-swag wants to merge 3 commits into
masterfrom
fix/windows/15152/basekeyboard-user-not-admin

Conversation

@rc-swag

@rc-swag rc-swag commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

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

  1. Login into Windows with and account that is a "standard" user and does not have "Administrator" rights.
  2. Install the Keyman from this PR
  3. Open Keyman Configuration -> Options
  4. Click Base Keyboard you will need to enter the login details for a Admin user. Change the Base Keyboard
  5. Confirm the Keyboard changes for the current user and not the Admin user used for the elevated processs

@rc-swag rc-swag self-assigned this Jun 30, 2026
@github-project-automation github-project-automation Bot moved this to Todo in Keyman Jun 30, 2026
@keymanapp-test-bot keymanapp-test-bot Bot added the user-test-missing User tests have not yet been defined for the PR label Jun 30, 2026
@keymanapp-test-bot

keymanapp-test-bot Bot commented Jun 30, 2026

Copy link
Copy Markdown

User Test Results

Test specification and instructions

  • TEST_BASE_KEYBOARD_CURRENT_USER (OPEN)
Results Template
# Test Results

* **TEST_BASE_KEYBOARD_CURRENT_USER (OPEN):** notes

Test Artifacts

@mcdurdin mcdurdin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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:

if FOldBaseLayout <> Get_Items('koBaseLayout').Value then
for I := 0 to Context.Keyboards.Count - 1 do // I4169
(Context.Keyboards.Items[I] as IIntKeymanKeyboardInstalled).UpdateBaseLayout;

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:

with Context as TKeymanContext do
BaseKeyboardID := (Options as IKeymanOptions).Items['koBaseLayout'].Value;

So some plumbing required, sadly.

@keyman-server keyman-server modified the milestones: A19S32, A19S33 Jul 4, 2026
@rc-swag

rc-swag commented Jul 6, 2026

Copy link
Copy Markdown
Contributor Author

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:

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.
I tested it last week and it changed the base keyboard for the non admin user. Not the admin. I will look into it again.

@keymanapp-test-bot keymanapp-test-bot Bot added user-test-required User tests have not been completed and removed user-test-missing User tests have not yet been defined for the PR labels Jul 6, 2026
@mcdurdin

mcdurdin commented Jul 6, 2026

Copy link
Copy Markdown
Member

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:

  1. CU: The Base Keyboard dialog itself should not be elevated - launch from the current process.
  2. CU: After clicking OK, verify if any mcompile is needed by looking to see if the needed .kmx files are already present in ProgramData.
  3. CU: If mcompile is required, then wait for elevated kmshell -mcompile <baseklid> should be called to run the mcompile process.
  4. LM: kmshell -mcompile will need to temporarily set the option[koBaseLayout] for the admin user in order to run the process. This will cause mcompile to be run. Reset option afterwards.
  5. CU: After the wait for elevated process step completes, set option[koBaseLayout] for current user.

Also:

  • If Keyman Configuration is started as an elevated user, skip steps 3 and 4.
  • Need to update kmcomapi to only run mcompile if elevated

@mcdurdin

mcdurdin commented Jul 6, 2026

Copy link
Copy Markdown
Member

5. Confirm the Keyboard changes for the current user and not the Admin user used for the elevated processs

Need to verify that the relevant files are saved in ProgramData too and that the base keyboard is mapped as expected!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

bug(windows): base keyboard user setting not set when using the current user is not an administrator

3 participants