View Issue Details

IDProjectCategoryView StatusLast Update
0019903LazarusLCLpublic2019-04-22 19:30
ReporterMartin FriebeAssigned ToFelipe Monteiro de Carvalho 
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Product Version0.9.31 (SVN)Product Build 
Target VersionFixed 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
  • 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));
    

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 10: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 16:18

developer   ~0050648

Targetting to 0.99 since Anton proposed a fix

Anton

2011-08-09 18: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 (notifications not working)

2011-08-10 23:08

developer   ~0050694

Last edited: 2011-08-10 23: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 10: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
Yes, I saw this.
But will OnPageChanged stay? When the decision is made I'll be happy to provide a patch ;-)

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