Skip to content
Draft
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
135 changes: 127 additions & 8 deletions windows/src/desktop/kmshell/main/UfrmBaseKeyboard.pas
Original file line number Diff line number Diff line change
Expand Up @@ -11,29 +11,35 @@ interface
TfrmBaseKeyboard = class(TfrmWebContainer)
procedure TntFormCreate(Sender: TObject);
private
FBaseKeyboardID: Integer;
procedure Footer_Cancel;
procedure Footer_OK(params: TStringList);
protected
procedure FireCommand(const command: WideString; params: TStringList); override;
end;

function ConfigureBaseKeyboard: Boolean;
function ConfigureBaseKeyboard(out BaseKeyboardID: Integer): Boolean;
function SetBaseKeyboard(WindowHandle: THandle; BaseKeyboardID: Integer): Boolean;
function MCompileBaseKeyboard(const BaseKeyboardIDText: string): Boolean;

implementation

{$R *.dfm}

uses
BaseKeyboards,
kmint;
ErrorControlledRegistry,
RegistryKeys,
keymanapi_TLB,
kmint,
utilkmshell;

function ConfigureBaseKeyboard: Boolean;
begin
with TfrmBaseKeyboard.Create(nil) do
function ConfigureBaseKeyboard(out BaseKeyboardID: Integer): Boolean;
begin with TfrmBaseKeyboard.Create(nil) do

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.

Suggested change
begin with TfrmBaseKeyboard.Create(nil) do
begin
with TfrmBaseKeyboard.Create(nil) do

try
Result := ShowModal = mrOk;
if Result then
kmcom.Apply;
BaseKeyboardID := FBaseKeyboardID;
finally
Free;
end;
Expand Down Expand Up @@ -65,9 +71,122 @@ procedure TfrmBaseKeyboard.Footer_OK(params: TStringList);
v: Integer;
begin
if not TryStrToInt('$'+params.Values['id'], v) then Exit;
kmcom.Options['koBaseLayout'].Value := v;
kmcom.Options.Apply;
FBaseKeyboardID := v;
ModalResult := mrOk;
end;

function MCompileBaseKeyboard(const BaseKeyboardIDText: string): Boolean;

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 think we should move this into its own unit

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This has changed a fair bit since this comment is it still valid. However, looking at it these 3 functions could be in there own unit around "Setting Base Keyboard" and are not tightly coupled to a form.

function SetBaseKeyboard(WindowHandle: THandle; BaseKeyboardID: Integer): Boolean;
function MCompileBaseKeyboard(const BaseKeyboardIDText: string): Boolean;
function CompileForBaseKeyboard(BaseKeyboardID: Integer): Boolean;

var
BaseKeyboardID: Integer;
PreviousBaseKeyboardID: Integer;
PreviousBaseKeyboardValue: string;
PreviousBaseKeyboardValueExists: Boolean;

procedure SavePreviousRegistryBaseKeyboardValue;
var
Reg: TRegistryErrorControlled;
begin
PreviousBaseKeyboardValueExists := False;
PreviousBaseKeyboardValue := '';

Reg := TRegistryErrorControlled.Create;
try
if Reg.OpenKeyReadOnly(SRegKey_KeymanEngine_CU) and Reg.ValueExists(SRegValue_UnderlyingLayout) then
begin
PreviousBaseKeyboardValueExists := True;
PreviousBaseKeyboardValue := Reg.ReadString(SRegValue_UnderlyingLayout);
end;
finally
Reg.Free;
end;
end;

procedure RestorePreviousBaseKeyboardValue;
var
Reg: TRegistryErrorControlled;
begin
Reg := TRegistryErrorControlled.Create;
try
if Reg.OpenKey(SRegKey_KeymanEngine_CU, True) then
if PreviousBaseKeyboardValueExists then
Reg.WriteString(SRegValue_UnderlyingLayout, PreviousBaseKeyboardValue)
else if Reg.ValueExists(SRegValue_UnderlyingLayout) then
Reg.DeleteValue(SRegValue_UnderlyingLayout);
finally
Reg.Free;
end;
end;

procedure ForceBaseLayoutChange;
var
Reg: TRegistryErrorControlled;
begin
// This is hacky, maybe just remove the registry value, however that
// would not force a recompile if the was the default base layout.
// Options.Apply re-compiles only when it observes a changed base layout.
// The caller may be repairing missing files for the already-selected layout.
Reg := TRegistryErrorControlled.Create;
try
if Reg.OpenKey(SRegKey_KeymanEngine_CU, True) then
Reg.WriteString(SRegValue_UnderlyingLayout, '00000000');
finally
Reg.Free;
end;

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.

Not comfortable with this

end;

begin
Result := False;
if not TryStrToInt('$' + BaseKeyboardIDText, BaseKeyboardID) or
not kmcom.SystemInfo.IsAdministrator then
Exit;

SavePreviousRegistryBaseKeyboardValue;
PreviousBaseKeyboardID := kmcom.Options['koBaseLayout'].Value;
kmcom.Options['koBaseLayout'].Value := BaseKeyboardID;
try
if PreviousBaseKeyboardID = BaseKeyboardID then
ForceBaseLayoutChange;
kmcom.Options.Apply;
Result := True;
finally
kmcom.Options['koBaseLayout'].Value := PreviousBaseKeyboardID;
RestorePreviousBaseKeyboardValue;
end;

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.

We can update the kmcom API and I think that might be better than side-effecting our way to a solution?

end;

function BaseKeyboardNeedsMCompile(BaseKeyboardID: Integer): Boolean;
var
I: Integer;
Keyboard: IKeymanKeyboardInstalled;
BaseFileName: string;
BaseKeyboardIDHex: string;
begin
BaseKeyboardIDHex := IntToHex(BaseKeyboardID, 8);
for I := 0 to kmcom.Keyboards.Count - 1 do
begin
Keyboard := kmcom.Keyboards.Items[I];
BaseFileName := Keyboard.Filename;
if FileExists(BaseFileName) and
(not FileExists(ChangeFileExt(BaseFileName, '') + '-' + BaseKeyboardIDHex + '.kmx') or
not FileExists(ChangeFileExt(BaseFileName, '') + '-' + BaseKeyboardIDHex + '-d.kmx')) then
Exit(True);
end;
Result := False;
end;

function SetBaseKeyboard(WindowHandle: THandle; BaseKeyboardID: Integer): Boolean;
var
MCompileResult: Boolean;
begin
MCompileResult := True;
Result := False;
if BaseKeyboardNeedsMCompile(BaseKeyboardID) and not kmcom.SystemInfo.IsAdministrator then
MCompileResult := WaitForElevatedConfiguration(WindowHandle, '-mcompile ' + IntToHex(BaseKeyboardID, 8)) = 0;

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.

This means if isadmin then mcompile will never be called? Don't we need:

Suggested change
if BaseKeyboardNeedsMCompile(BaseKeyboardID) and not kmcom.SystemInfo.IsAdministrator then
MCompileResult := WaitForElevatedConfiguration(WindowHandle, '-mcompile ' + IntToHex(BaseKeyboardID, 8)) = 0;
if BaseKeyboardNeedsMCompile(BaseKeyboardID) then
begin
if not kmcom.SystemInfo.IsAdministrator then
MCompileResult := WaitForElevatedConfiguration(WindowHandle, '-mcompile ' + IntToHex(BaseKeyboardID, 8)) = 0;
else
MCompileResult := ... call directly?
end;

if not MCompileResult then
Exit;
kmcom.Options['koBaseLayout'].Value := BaseKeyboardID;
kmcom.Options.Apply; // This will trigger a recompile if needed
Result := True;
end;

end.
11 changes: 9 additions & 2 deletions windows/src/desktop/kmshell/main/UfrmMain.pas
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ implementation
Keyman.Configuration.UI.UfrmStartInstall,
RegistryKeys,
SupportXMLRenderer,
UfrmBaseKeyboard,
UfrmChangeHotkey,
UfrmHTML,
UfrmInstallKeyboardFromWeb,
Expand Down Expand Up @@ -660,9 +661,15 @@ procedure TfrmMain.cefBeforeBrowseSync(Sender: TObject; const Url: string;
------------------------------------------------------------------------------}

procedure TfrmMain.Options_BaseKeyboard; // I4169
var
BaseKeyboardID: Integer;
begin
WaitForElevatedConfiguration(Handle, '-basekeyboard');
// Refresh will be triggered by elevated process
if ConfigureBaseKeyboard(BaseKeyboardID) then
begin
SetBaseKeyboard(Handle, BaseKeyboardID);
DoRefresh;
end;

end;

procedure TfrmMain.Options_SettingsManager;
Expand Down
16 changes: 15 additions & 1 deletion windows/src/desktop/kmshell/main/initprog.pas
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ function Main(Owner: TComponent = nil): TModalResult;
fmKeyboardWelcome, // I2569
fmKeyboardPrint, // I2329
fmBaseKeyboard, // I4169
fmMCompile,
fmUpgradeMnemonicLayout, // I4553
fmRepair,
fmKeepInTouch,
Expand Down Expand Up @@ -262,6 +263,13 @@ function Init(var FMode: TKMShellMode; KeyboardFileNames: TStrings; var FSilent,
else if s = '-bd' then FMode := fmBackgroundDownload
else if s = '-an' then FMode := fmApplyInstallNow
else if s = '-basekeyboard' then FMode := fmBaseKeyboard // I4169
else if s = '-mcompile' then
begin
FMode := fmMCompile;
Inc(i);
if i > ParamCount then Exit;
FQuery := ParamStr(i);
end
else if s = '-nowelcome' then FNoWelcome := True
else if s = '-kw' then FMode := fmKeyboardWelcome // I2569
else if s = '-kp' then FMode := fmKeyboardPrint // I2329
Expand Down Expand Up @@ -393,6 +401,7 @@ procedure RunKMCOM(FMode: TKMShellMode; KeyboardFileNames: TStrings; FSilent, FF
kdl: IKeymanDefaultLanguage;
FIcon: string;
FMutex: TKeymanMutex; // I2720
BaseKeyboardID: Integer;
function FirstKeyboardFileName: WideString;
begin
if KeyboardFileNames.Count = 0
Expand Down Expand Up @@ -540,7 +549,12 @@ procedure RunKMCOM(FMode: TKMShellMode; KeyboardFileNames: TStrings; FSilent, FF
end;

fmBaseKeyboard: // I4169
if ConfigureBaseKeyboard
if ConfigureBaseKeyboard(BaseKeyboardID) and SetBaseKeyboard(0, BaseKeyboardID)
then ExitCode := 0
else ExitCode := 1;

fmMCompile:
if MCompileBaseKeyboard(FQuery)
then ExitCode := 0
else ExitCode := 1;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,14 +152,19 @@ procedure TKeymanKeyboardInstalled.Uninstall;
end;

procedure TKeymanKeyboardInstalled.UpdateBaseLayout; // I4169

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This could now be removed

var
BaseKeyboardID: Integer;
begin
if FRegKeyboard.MnemonicLayout and FileExists(FRegKeyboard.KeymanFile) then // I4615
begin
BaseKeyboardID := (Context.Options as IKeymanOptions).Items['koBaseLayout'].Value;
Comment thread
rc-swag marked this conversation as resolved.
with TKPRecompileMnemonicKeyboard.Create(Context) do
try
Execute(FRegKeyboard.KeymanFile, FRegKeyboard.PackageName);
Execute(FRegKeyboard.KeymanFile, FRegKeyboard.PackageName, BaseKeyboardID);
finally
Free;
end;
end;
end;

function TKeymanKeyboardInstalled.Get_Copyright: WideString;
Expand Down
22 changes: 13 additions & 9 deletions windows/src/engine/kmcomapi/com/options/keymanoptions.pas
Original file line number Diff line number Diff line change
@@ -1,18 +1,18 @@
(*
Name: keymanoptions
Copyright: Copyright (C) SIL International.
Documentation:
Description:
Documentation:
Description:
Create Date: 20 Jun 2006

Modified Date: 6 Feb 2015
Authors: mcdurdin
Related Files:
Dependencies:
Related Files:
Dependencies:

Bugs:
Todo:
Notes:
Bugs:
Todo:
Notes:
History: 20 Jun 2006 - mcdurdin - Initial version
01 Aug 2006 - mcdurdin - Add AutoRefershKeyman call
12 Aug 2008 - mcdurdin - Avoid crash with missing options
Expand Down Expand Up @@ -67,6 +67,7 @@ implementation
ErrorControlledRegistry,
RegistryKeys,
Glossary,
isadmin,
Keyman.System.BaseKeyboard,
KeymanOptionNames,
keymanerrorcodes;
Expand Down Expand Up @@ -112,7 +113,9 @@ function TKeymanOptions.IndexOf(const ID: WideString): Integer;

procedure TKeymanOptions.Apply;
var
I, FOldBaseLayout: Integer;
I: Integer;
FOldBaseLayout: Integer;
FNewBaseLayout: Integer;
begin
with TRegistryErrorControlled.Create do // I3717
try
Expand All @@ -130,7 +133,8 @@ procedure TKeymanOptions.Apply;

FInternalOptions.Save(Context);

if FOldBaseLayout <> Get_Items('koBaseLayout').Value then
FNewBaseLayout := Get_Items('koBaseLayout').Value;
if IsAdministrator and (FOldBaseLayout <> FNewBaseLayout) then

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.

Why are we testing for IsAdministrator here?

for I := 0 to Context.Keyboards.Count - 1 do // I4169
(Context.Keyboards.Items[I] as IIntKeymanKeyboardInstalled).UpdateBaseLayout;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ procedure TKPInstallKeyboard.Execute(const FileName, PackageID: string; FInstall
FExitCode: Integer;
FKVKName: WideString;
FCreatedIcon: Boolean;
BaseKeyboardID: Integer;
begin
KL.MethodEnter(Self, 'Execute', [FileName,PackageID,ikPartOfPackage in FInstallOptions ,Force]);
try
Expand Down Expand Up @@ -248,9 +249,11 @@ procedure TKPInstallKeyboard.Execute(const FileName, PackageID: string; FInstall
// Recompile a mnemonic layout to the user's selected base layout
if ki.MnemonicLayout then // I4169
begin
with Context as TKeymanContext do
BaseKeyboardID := (Options as IKeymanOptions).Items['koBaseLayout'].Value;

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.

will this still get the admin value of base layout? do we need to be able to pass baselayout as a param to elevated kmshell?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

hmm yes I think we do. I just realised I have it resolved this case where it is on the install of new keyboard. We need get the users baselayout

with TKPRecompileMnemonicKeyboard.Create(Context) do
try
Execute(FDestFileName, PackageID);
Execute(FDestFileName, PackageID, BaseKeyboardID);
finally
Free;
end;
Expand Down
Loading