View Issue Details

IDProjectCategoryView StatusLast Update
0019903LazarusLCLpublic2019-04-22 17:30
ReporterMartin Friebe Assigned ToFelipe Monteiro de Carvalho  
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Product Version0.9.31 (SVN) 
Fixed in Version0.9.31 (SVN) 
Summary0019903: [patch] TPageControl.OnChange fires twice
Descriptionw32/Vista, Fedora, OSX

When clicking a tab, then TPageControl.OnChange fires twice
Additional InformationMight be related/duplicate to 0011813 (Which is not clear if it refers to OnChange or OnChanging)
Tagshas_patch, patch
Fixed in Revision32622
LazTarget0.99.0
WidgetsetGTK 2, Win32/Win64, Carbon
Attached Files

Relationships

has duplicate 0011813 resolvedFelipe Monteiro de Carvalho Lazarus Carbon - TPageControl OnChange event fires twice 
related to 0020442 resolvedFelipe Monteiro de Carvalho Packages PAge Control crashes into EReadError of OnPageChanged even on WinCE 

Activities

Anton

2011-08-06 08:43

reporter   ~0050531

Indeed, it is 2 event handlers: TPageControl.OnChange and TPageControl.OnPageChange. When I try to create OnChange handler, IDE also creates OnPageChanged handler and points it to TForm1.PageControl1Change too.
In run-time after page changing these events fire in he following order:
1) OnPageChanged (pagecontrol.inc:113 in inherited method)
2) OnChange (pagecontrol.inc:115)
When I delete in OI OnPageChanged handler it also deletes OnChange handler and vice versa.
An error is here: comctrls.pp:400
    property OnChange: TNotifyEvent read FOnPageChanged write FOnPageChanged;

Felipe Monteiro de Carvalho

2011-08-09 14:18

developer   ~0050648

Targetting to 0.99 since Anton proposed a fix

Anton

2011-08-09 16:58

reporter   ~0050658

I propose to make difference between OnPageChanged and OnChange events by introducing FOnChange field and
-property OnChange: TNotifyEvent read FOnPageChanged write FOnPageChanged;
+property OnChange: TNotifyEvent read FOnChange write FOnChange;

Flávio Etrusco

2011-08-10 21:08

developer   ~0050694

Last edited: 2011-08-10 21:15

I guess OnChange was added for compatibility, right? If so, why not add "stored False;" to OnPageChanged and remove the double invocation (remove TPageControl.DoChange), for a couple of releases, and then remove OnPageChanged altogether?

Felipe Monteiro de Carvalho

2011-08-12 08:56

developer   ~0050753

OnChange is the correct one from the VCL:

http://docs.embarcadero.com/products/rad_studio/delphiAndcpp2009/HelpUpdate2/EN/html/delphivclwin32/!!MEMBERTYPE_Events_ComCtrls_TPageControl.html

Flávio Etrusco

2011-08-12 09:43

developer   ~0050763

Yes, I saw this.
But will OnPageChanged stay? When the decision is made I'll be happy to provide a patch ;-)

2011-08-26 20:23

 

pagecontrol-fix-double-event.patch (1,313 bytes)   
diff --git lcl/comctrls.pp lcl/comctrls.pp
index 4149f25..0e4fbfe 100644
--- lcl/comctrls.pp
+++ lcl/comctrls.pp
@@ -508,7 +508,6 @@ type
                        State: TDragState; var Accept: Boolean); override;
     procedure DoRemoveDockClient(Client: TControl); override;
     function DoUndockClientMsg(NewTarget, Client: TControl):boolean; override;
-    procedure DoChange; override;
     function ChildClassAllowed(ChildClass: TClass): boolean; override;
   public
     constructor Create(TheOwner: TComponent); override;
@@ -574,7 +573,7 @@ type
     property OnMouseLeave;
     property OnMouseMove;
     property OnMouseUp;
-    property OnPageChanged;
+    property OnPageChanged stored False;
     property OnResize;
     property OnStartDock;
     property OnStartDrag;
diff --git lcl/include/pagecontrol.inc lcl/include/pagecontrol.inc
index 166e6bb..c6caa01 100644
--- lcl/include/pagecontrol.inc
+++ lcl/include/pagecontrol.inc
@@ -108,13 +108,6 @@ begin
   Result := inherited DoUndockClientMsg(NewTarget, Client);
 end;
 
-procedure TPageControl.DoChange;
-begin
-  inherited DoChange;
-  if Assigned(OnChange) then
-    OnChange(Self);
-end;
-
 function TPageControl.ChildClassAllowed(ChildClass: TClass): boolean;
 begin
   Result:=(ChildClass<>nil) and (ChildClass.InheritsFrom(PageClass));

Flávio Etrusco

2011-08-26 20:27

developer   ~0051166

FWIW I attached a patch.
@Felipe I was actually going to cancel assigning the bug to you, but Mantis doesn't display an intermediate page like all other of its actions :-/

Felipe Monteiro de Carvalho

2011-10-02 17:43

developer   ~0052381

Your patch is wrong. We should do like Anton proposed:

-property OnChange: TNotifyEvent read FOnPageChanged write FOnPageChanged;
 +property OnChange: TNotifyEvent read FOnChange write FOnChange;

And deprecate OnPageChanged

Felipe Monteiro de Carvalho

2011-10-02 18:04

developer   ~0052383

Even better, I removed OnPageChanged. It will be annoying in the short run, but in the long run it will be good, because 2 events for the same thing is not a good thing.

I documented the incompatibility here:

http://wiki.lazarus.freepascal.org/Lazarus_0.99.0_release_notes#TCustomPageControl.2FTPageControl.OnPageChanged_were_removed

Issue History

Date Modified Username Field Change
2011-08-05 16:53 Martin Friebe New Issue
2011-08-05 16:53 Martin Friebe LazTarget => -
2011-08-05 16:53 Martin Friebe Relationship added related to 0011813
2011-08-06 08:43 Anton Note Added: 0050531
2011-08-09 14:18 Felipe Monteiro de Carvalho LazTarget - => 0.99.0
2011-08-09 14:18 Felipe Monteiro de Carvalho Note Added: 0050648
2011-08-09 14:18 Felipe Monteiro de Carvalho Status new => confirmed
2011-08-09 16:58 Anton Note Added: 0050658
2011-08-10 21:08 Flávio Etrusco Note Added: 0050694
2011-08-10 21:15 Flávio Etrusco Note Edited: 0050694
2011-08-11 05:44 Flávio Etrusco Additional Information Updated
2011-08-11 05:51 Flávio Etrusco Widgetset => GTK 2, Win32/Win64, Carbon
2011-08-12 08:56 Felipe Monteiro de Carvalho Note Added: 0050753
2011-08-12 09:43 Flávio Etrusco Note Added: 0050763
2011-08-26 20:23 Flávio Etrusco File Added: pagecontrol-fix-double-event.patch
2011-08-26 20:24 Flávio Etrusco Status confirmed => assigned
2011-08-26 20:24 Flávio Etrusco Assigned To => Felipe Monteiro de Carvalho
2011-08-26 20:27 Flávio Etrusco Note Added: 0051166
2011-09-03 06:04 Flávio Etrusco Assigned To Felipe Monteiro de Carvalho =>
2011-09-03 06:04 Flávio Etrusco Summary TPageControl.OnChange fires twice => [patch] TPageControl.OnChange fires twice
2011-09-03 06:05 Flávio Etrusco Tag Attached: has_patch
2011-09-03 06:05 Flávio Etrusco Tag Attached: patch
2011-09-03 06:05 Flávio Etrusco Status assigned => acknowledged
2011-10-02 17:43 Felipe Monteiro de Carvalho Note Added: 0052381
2011-10-02 17:55 Felipe Monteiro de Carvalho Relationship deleted related to 0011813
2011-10-02 17:55 Felipe Monteiro de Carvalho Relationship added has duplicate 0011813
2011-10-02 18:04 Felipe Monteiro de Carvalho Fixed in Revision => 32622
2011-10-02 18:04 Felipe Monteiro de Carvalho Status acknowledged => resolved
2011-10-02 18:04 Felipe Monteiro de Carvalho Fixed in Version => 0.9.31 (SVN)
2011-10-02 18:04 Felipe Monteiro de Carvalho Resolution open => fixed
2011-10-02 18:04 Felipe Monteiro de Carvalho Assigned To => Felipe Monteiro de Carvalho
2011-10-02 18:04 Felipe Monteiro de Carvalho Note Added: 0052383
2011-10-12 07:20 Felipe Monteiro de Carvalho Relationship added related to 0020442
2019-04-22 17:30 Martin Friebe Status resolved => closed