From a1c1930ec1f9717223c9c6a975f1450dc084866f Mon Sep 17 00:00:00 2001
From: twinn1013 <81521099+twinn1013@users.noreply.github.com>
Date: Thu, 23 Jul 2026 04:46:37 +0000
Subject: [PATCH] fix: hang and wrong check states in data grid column
 selection on macOS

Fixes the reported hang (issue 2554): closing the column selection popup
with OK could run FormClose twice on macOS, freeing FCheckedColumns two
times. The resulting access violation raised the crash dialog inside a
paint cycle, freezing the application. FCheckedColumns is now freed in
FormDestroy, which runs exactly once.

While verifying that fix, a second bug in the same popup showed up:
checked columns were not applied correctly to the grid. The click
handler used ItemIndex to detect the toggled item, but on Cocoa the
check event fires before the list selection is updated, so the wrong
item was added or removed. The handler now syncs the check states of
all displayed items instead.
---
 source/column_selection.pas | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/source/column_selection.pas b/source/column_selection.pas
index 1bbdecb91..49d91b53f 100644
--- a/source/column_selection.pas
+++ b/source/column_selection.pas
@@ -150,20 +150,20 @@ procedure TfrmColumnSelection.editFilterButtonClick(Sender: TObject);
 }
 procedure TfrmColumnSelection.chklistColumnsClickCheck(Sender: TObject);
 var
-  i : Integer;
+  i, CheckedIndex : Integer;
   AllSelected, NoneSelected : Boolean;
-  FocusedItem: String;
-  FocusedItemIndex: Integer;
 begin
-  // Add or remove clicked item from list
-  if chklistColumns.ItemIndex > -1 then begin
-    FocusedItem := chklistColumns.Items[chklistColumns.ItemIndex];
-    if chklistColumns.Checked[chklistColumns.ItemIndex] then begin
-      FCheckedColumns.Add(FocusedItem)
+  // Sync check states of all displayed items into FCheckedColumns. Using
+  // ItemIndex to detect the clicked item would be wrong on macOS, where the
+  // check event fires before the list selection is updated. See issue 2554.
+  for i:=0 to chklistColumns.Items.Count-1 do begin
+    CheckedIndex := FCheckedColumns.IndexOf(chklistColumns.Items[i]);
+    if chklistColumns.Checked[i] then begin
+      if CheckedIndex = -1 then
+        FCheckedColumns.Add(chklistColumns.Items[i]);
     end else begin
-      FocusedItemIndex := FCheckedColumns.IndexOf(FocusedItem);
-      if FocusedItemIndex > -1 then
-        FCheckedColumns.Delete(FocusedItemIndex);
+      if CheckedIndex > -1 then
+        FCheckedColumns.Delete(CheckedIndex);
     end;
   end;
 
@@ -225,6 +225,7 @@ procedure TfrmColumnSelection.FormDestroy(Sender: TObject);
 begin
   AppSettings.WriteInt(asColumnSelectorWidth, ScaleFormToDesign(Width));
   AppSettings.WriteInt(asColumnSelectorHeight, ScaleFormToDesign(Height));
+  FCheckedColumns.Free;
 end;
 
 
@@ -243,8 +244,9 @@ procedure TfrmColumnSelection.FormDeactivate(Sender: TObject);
 procedure TfrmColumnSelection.FormClose(Sender: TObject; var Action:
     TCloseAction);
 begin
+  // FormClose can run twice when the form is closed by OK and afterwards
+  // deactivated - free FCheckedColumns in FormDestroy only. See issue 2554.
   Action := caFree;
-  FCheckedColumns.Free;
 end;