View Issue Details

IDProjectCategoryView StatusLast Update
0037768LazarusLCLpublic2020-09-23 03:45
ReporterJoeny Ang Assigned Towp  
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Product Version2.1 (SVN) 
Summary0037768: [Patch] TRadioGroup: onEnter/onExit triggered twice
DescriptiononEnter/onExit is being triggered twice on a TRadioGroup.

Not sure why, but the individual TRadioButton's onEnter/onExit events are assigned handlers that will call DoEnter/DoExit of TCustomRadioGroup.
Steps To ReproduceSee test project
TagsNo tags attached.
Fixed in Revision63898
LazTarget-
Widgetset
Attached Files

Activities

Joeny Ang

2020-09-18 04:08

reporter  

tradiogroup-double-onenter-onexit-fix.patch (321 bytes)   
--- lcl/include/radiogroup.inc.63887
+++ lcl/include/radiogroup.inc
@@ -146,12 +146,12 @@
 
 procedure TCustomRadioGroup.ItemEnter(Sender: TObject);
 begin
-  DoEnter;
+
 end;
 
 procedure TCustomRadioGroup.ItemExit(Sender: TObject);
 begin
-  DoExit;
+
 end;
 
 procedure TCustomRadioGroup.ItemResize(Sender: TObject);

wp

2020-09-18 16:46

developer   ~0125619

Last edited: 2020-09-18 17:01

View 2 revisions

Checked with Delphi, and found that it fires the OnEnter and OnExit events only when the RadioGroup itself is entered/exited. Lazarus, on the other hand, also fires when a radio button is selected within the Radiogroup.

Which behaviour is correct? The Delphi way seems logical, but there is also some logics in the Lazarus way. And what it against applying the patch is that it will break some users' code relying on the events working exactly in the current way.

Calling DoEnter/DoExit in ItemEnter/ItemExit has been added intentionally by Mattias in r7581 back in 2006 (commit note: "TRadioGroup now delegates OnEnter/OnExit from items to itself"). At the top of the include file of that commit there a possibly related note:

 "Possible improvements:
       - The current implementation often recreates the group even
         if it might not be neccessary. This could be solved if with
         an approach like Marc Weustink suggested:

         "Why not on SetColumn/SetItems/FItems.Onchange create the necessary
         checkboxes and align them. This way the RadioGroup is just a control
         with other controls in it. It doesn't matter if the the gtk control is
         created or not.
         If not created and you already have added checkboxes, they will be
         created when the groupbox is created and will be destroyed when the
         groupbox is destroyed. This way you internally allways deal with
         TCheckboxes and you dont have to mess with creating/destroying them.
         Besides that, you dont have to recreate the control on every change."
  
         On the other side this might have the following disadvantages:
           - requires some work to find out which buttons to add/delete
           - the TButtonList and the group property of affected buttons
             have to be updated according to the new order of buttons
           - works only if the interface library supports reordering of
             radiobuttons"

I am not sure if all this is still valid today. Therefore, I would like you to investigate wheter there are any issue with your patch in any widgetset. I tested in Windows, where the patch seems to work fine.

wp

2020-09-18 18:08

developer   ~0125623

Last edited: 2020-09-19 23:38

View 5 revisions

I tested also on a VM with cocoa, and another one with gtk2 and qt5, and it looks fine in all these cases.

[EDIT]
qt works, too (Mint)

Joeny Ang

2020-09-19 03:41

reporter   ~0125636

Last edited: 2020-09-19 04:19

View 2 revisions

Tested also on GTK2 and QT5 under GNOME, and Win32/64 (WinXP, Win10) on VMs, all looks ok.

wp

2020-09-20 19:08

developer   ~0125677

Last edited: 2020-09-20 19:21

View 2 revisions

Lets's give it a try.

Maybe there are users who evaluate the OnEnter/OnExit events fired so far when ItemIndex changes. I added OnItemEnter and OnItemExit events as a replacement.
I dropped an incompatibility note at https://wiki.freepascal.org/Lazarus_2.2.0_release_notes#TRadioGroup.27s_OnEnter_and_OnExit_events.

Please test again, and close if ok.

Joeny Ang

2020-09-23 03:45

reporter   ~0125769

All good. Thanks :)

Issue History

Date Modified Username Field Change
2020-09-18 04:08 Joeny Ang New Issue
2020-09-18 04:08 Joeny Ang File Added: tradiogroup-double-onenter-onexit-fix.patch
2020-09-18 04:08 Joeny Ang File Added: tradiogroup-onenter-onexit-test-01.zip
2020-09-18 16:46 wp Note Added: 0125619
2020-09-18 16:46 wp Assigned To => wp
2020-09-18 16:46 wp Status new => feedback
2020-09-18 16:46 wp LazTarget => -
2020-09-18 17:01 wp Note Edited: 0125619 View Revisions
2020-09-18 18:08 wp Note Added: 0125623
2020-09-18 23:10 wp Note Edited: 0125623 View Revisions
2020-09-18 23:11 wp Note Edited: 0125623 View Revisions
2020-09-19 03:41 Joeny Ang Note Added: 0125636
2020-09-19 03:41 Joeny Ang Status feedback => assigned
2020-09-19 04:19 Joeny Ang Note Edited: 0125636 View Revisions
2020-09-19 23:38 wp Note Edited: 0125623 View Revisions
2020-09-19 23:38 wp Note Edited: 0125623 View Revisions
2020-09-20 19:08 wp Status assigned => resolved
2020-09-20 19:08 wp Resolution open => fixed
2020-09-20 19:08 wp Fixed in Revision => 63898
2020-09-20 19:08 wp Note Added: 0125677
2020-09-20 19:21 wp Note Edited: 0125677 View Revisions
2020-09-23 03:45 Joeny Ang Status resolved => closed
2020-09-23 03:45 Joeny Ang Note Added: 0125769