fix(developer): prevent two touch layout editors opening for the same file

The keyboard editor has a complex edit state machine, which has
grown a lot over time. This is a minimal patch to address one specific
edge case scenario on that state machine, without any attempt to improve
the state machine overall. The biggest change here is bubbling failure
up to the main form so that it can destroy (aka Release, which is an
asynchronous destroy) the editor window if it fails to load completely.

Fixes: #11715
Fixes: KEYMAN-DEVELOPER-1JC
This commit is contained in:
Marc Durdin 2024-06-06 10:45:44 +07:00
parent ce0dbd800c
commit 33d7dca155
3 changed files with 89 additions and 63 deletions

View file

@ -453,13 +453,13 @@ type
procedure ConfirmSaveOfOldEditorWindows;
procedure ConfirmSaveOfOldEditorWindow(FeatureID: TKeyboardParser_FeatureID;
FModified: Boolean; const FOldFilename: string; DoSave: TProc; DoLoad: TProc<String>);
procedure LoadFeature(ID: TKeyboardParser_FeatureID);
function LoadFeature(ID: TKeyboardParser_FeatureID): Boolean;
function FeatureTab(kf: TKeyboardParser_FeatureID): TTabSheet;
procedure InitFeatureTab(ID: TKeyboardParser_FeatureID);
procedure FeatureModified(Sender: TObject);
function SaveFeature(ID: TKeyboardParser_FeatureID): Boolean;
procedure SelectTouchLayoutTemplate(APromptChange: Boolean);
procedure LoadTouchLayout; // I4034
function LoadTouchLayout: Boolean; // I4034
function GetFontInfo(Index: TKeyboardFont): TKeyboardFontInfo; // I4057
procedure SetFontInfo(Index: TKeyboardFont; const Value: TKeyboardFontInfo); // I4057
@ -1628,7 +1628,7 @@ begin
FLayoutSetup := FOldLayoutSetup;
end;
procedure TfrmKeymanWizard.LoadFeature(ID: TKeyboardParser_FeatureID);
function TfrmKeymanWizard.LoadFeature(ID: TKeyboardParser_FeatureID): Boolean;
begin
if FKeyboardParser.Features.ContainsKey(ID) then
begin
@ -1651,7 +1651,8 @@ begin
end;
kfTouchLayout:
begin
LoadTouchLayout; // I4034
if not LoadTouchLayout then
Exit(False);
end;
else
begin
@ -1667,6 +1668,7 @@ begin
end;
end;
FFeature[ID].Modified := False;
Result := True;
end;
function TfrmKeymanWizard.SaveFeature(ID: TKeyboardParser_FeatureID): Boolean;
@ -2007,7 +2009,12 @@ begin
LoadSettings;
for kf in FKeyboardParser.Features.Keys do
LoadFeature(kf);
begin
if not LoadFeature(kf) then
begin
Exit(False);
end;
end;
if FKeyboardParser.IsComplex then // I4557
pagesLayout.ActivePage := pageLayoutCode;
@ -3163,18 +3170,17 @@ begin
FFeature[kfTouchLayout].Modified := True;
end;
procedure TfrmKeymanWizard.LoadTouchLayout; // I4034
function TfrmKeymanWizard.LoadTouchLayout: Boolean; // I4034
begin
if pagesTouchLayout.ActivePage = pageTouchLayoutDesign then
begin
if not frameTouchLayout.Load(FFeature[kfTouchLayout].Filename, False, False) then
begin
pagesTouchLayout.ActivePage := pageTouchLayoutCode;
frameTouchLayoutSource.LoadFromFile(FFeature[kfTouchLayout].Filename, tffUTF8);
end;
Result := frameTouchLayout.Load(FFeature[kfTouchLayout].Filename, False, False);
end
else
begin
frameTouchLayoutSource.LoadFromFile(FFeature[kfTouchLayout].Filename, tffUTF8);
Result := True;
end;
end;
procedure TfrmKeymanWizard.SaveTouchLayout; // I3885

View file

@ -1477,8 +1477,12 @@ begin
if n >= 0 then
Result.ProjectFile := FGlobalProject.Files[n];
(Result as frmClass).OpenFile(FFileName);
LockWindowUpdate(0);
if not (Result as frmClass).OpenFile(FFileName) then
begin
Result.Release;
end;
end;
procedure TfrmKeymanDeveloper.HelpTopic(s: string);

View file

@ -90,7 +90,7 @@ type
procedure cefCommand(Sender: TObject; const command: string; params: TStringList);
procedure cefLoadEnd(Sender: TObject);
procedure RegisterSource;
procedure RegisterSources(const AState: string);
procedure CharMapDragDrop(Sender, Source: TObject; X, Y: Integer);
procedure CharMapDragOver(Sender, Source: TObject; X, Y: Integer;
State: TDragState; var Accept: Boolean);
@ -238,10 +238,12 @@ begin
modWebHttpServer.AppSource.UnregisterSource(FFilename+'#state');
end;
procedure TframeTouchLayoutBuilder.RegisterSource;
procedure TframeTouchLayoutBuilder.RegisterSources(const AState: string);
begin
if FFilename <> '' then
modWebHttpServer.AppSource.RegisterSource(FFilename, FSavedLayoutJS);
if (FFileName <> '') and (AState <> '') then
modWebHttpServer.AppSource.RegisterSource(FFilename + '#state', AState, True);
end;
procedure TframeTouchLayoutBuilder.ImportFromKVK(const KVKFileName: string); // I3945
@ -332,71 +334,85 @@ begin
end;
UnregisterSources;
try
if ALoadFromString then
if ALoadFromString then
begin
FNewLayoutJS := AFilename;
FFilename := GetNextFilename;
end
else
begin
if ALoadFromTemplate or (AFileName = '') or not FileExists(AFileName) then
begin
FNewLayoutJS := AFilename;
FBaseFileName := FTemplateFileName;
FFilename := GetNextFilename;
end
else
begin
if ALoadFromTemplate or (AFileName = '') or not FileExists(AFileName) then
begin
FBaseFileName := FTemplateFileName;
FFilename := GetNextFilename;
end
else
begin
FBaseFileName := AFileName;
FFilename := AFileName;
end;
with TStringList.Create do
try
LoadFromFile(FBaseFileName, TEncoding.UTF8);
FNewLayoutJS := Text;
finally
Free;
end;
FBaseFileName := AFileName;
FFilename := AFileName;
end;
FTouchLayout := TTouchLayout.Create; // I3642
with TStringList.Create do
try
if not FTouchLayout.Load(FNewLayoutJS) then
LoadFromFile(FBaseFileName, TEncoding.UTF8);
FNewLayoutJS := Text;
finally
Free;
end;
end;
FTouchLayout := TTouchLayout.Create; // I3642
try
if not FTouchLayout.Load(FNewLayoutJS) then
begin
FLastError := FTouchLayout.LoadError; // I4083
FLastErrorOffset := FTouchLayout.LoadErrorOffset; // I4083
FFilename := FLastFilename;
RegisterSources(FState);
Exit(False);
end
else
begin
if (FSavedLayoutJS <> '') and ALoadFromTemplate then
begin
FLastError := FTouchLayout.LoadError; // I4083
FLastErrorOffset := FTouchLayout.LoadErrorOffset; // I4083
FFilename := FLastFilename;
Exit(False);
FOldLayout := TTouchLayout.Create;
try
FOldLayout.Load(FSavedLayoutJS);
if FTouchLayout.Merge(FOldLayout)
then FSavedLayoutJS := FTouchLayout.Save(False)
else FSavedLayoutJS := FNewLayoutJS;
finally
FOldLayout.Free;
end;
end
else
begin
if (FSavedLayoutJS <> '') and ALoadFromTemplate then
begin
FOldLayout := TTouchLayout.Create;
try
FOldLayout.Load(FSavedLayoutJS);
if FTouchLayout.Merge(FOldLayout)
then FSavedLayoutJS := FTouchLayout.Save(False)
else FSavedLayoutJS := FNewLayoutJS;
finally
FOldLayout.Free;
end;
end
else
FSavedLayoutJS := FNewLayoutJS;
end;
finally
FTouchLayout.Free;
FSavedLayoutJS := FNewLayoutJS;
end;
finally
RegisterSource;
if (FFileName <> '') and (FState <> '') then
modWebHttpServer.AppSource.RegisterSource(FFilename + '#state', FState, True);
FTouchLayout.Free;
end;
if (FFileName <> '') and modWebHttpServer.AppSource.IsSourceRegistered(FFileName) then
begin
// If two .kmn files are loaded which both reference the same
// .keyman-touch-layout file, it's safest to just block it. This is a rare
// scenario, as most keyboard projects have a single .kmn, and it usually
// indicates a project may be in a bit of chaos anyway.
ShowMessage(
'The touch layout is already opened for editing in another keyboard '+
'editor. Please close the other keyboard editor before opening this one '+
'again.');
// We want to prevent this window unregistering the sources it doesn't own
// when it is destroyed immediately after this, which we can do by blanking
// the filename.
FFileName := '';
Exit(False);
end;
RegisterSources(FState);
try
DoLoad;
except