View Issue Details

IDProjectCategoryView StatusLast Update
0033528LazarusLCLpublic2018-09-19 12:12
ReporterSerge Anvarov Assigned ToBart Broersma  
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Product Version1.9 (SVN) 
Target Version1.8.6Fixed in Version1.8.6 
Summary0033528: The Position property of the TFindDialog or TReplaceDialog components is ignored
DescriptionThe Position, Left and Top properties of TFindDialog (or TReplaceDialog) are ignored. The dialog is always displayed in the center of the main form.
In Delphi these properties are taken into account.
Steps To ReproduceOn an empty project, put the TFindDialog component and the TButton with event:
procedure TForm1.Button1Click(Sender: TObject);
begin
  FindDialog1.Position := Point(100, 100);
  FindDialog1.Execute;
end;

The same for the TReplaceDialog.
TagsNo tags attached.
Fixed in Revisionr59031, r59048
LazTarget1.8.6
Widgetset
Attached Files

Relationships

related to 0034297 closedBart Broersma Issues with testing TFindDialog and TReplaceDialog in form designer 

Activities

Serge Anvarov

2018-03-28 16:58

reporter  

finddialog.diff (1,613 bytes)   
Index: lcl/include/finddialog.inc
===================================================================
--- lcl/include/finddialog.inc	(revision 57570)
+++ lcl/include/finddialog.inc	(working copy)
@@ -81,7 +81,6 @@
   BorderIcons := [biSystemMenu, biHelp];
   Caption := 'Find';
   //OnCreate := @FormCreate;
-  Position := poMainFormCenter;
 
   LCLVersion := '1.3';
 
@@ -520,9 +519,6 @@
     HelpButton.OnClick := @HelpClick;
     CancelButton.OnClick := @CancelClick;
     PopupMode := pmAuto;
-    // Init local property with defaut value
-    FFormTop  := top;
-    FFormLeft := Left;
   end;
 end;
 
@@ -576,13 +572,13 @@
 end;
 
 procedure TFindDialog.CalcPosition(aForm:Tform);
-Var
-  MfBound : Trect;
-
 begin
-  MfBound := Application.MainForm.BoundsRect;
-  FFormTop  := MfBound.Top + (((MfBound.Bottom - MfBound.Top)  - aForm.Height) Div 2);
-  FFormLeft := MfBound.Left + (((MfBound.Right - MfBound.Left) - aForm.Width) Div 2);
+  if (FFormLeft = 0) and (FFormTop = 0) then // Self.Position not set
+  begin
+    aForm.Position := poMainFormCenter;
+    FFormTop := aForm.Top;
+    FFormLeft := aForm.Left;
+  end;
   aForm.Top := FFormTop;
   aForm.Left := FFormLeft;
 end;
Index: lcl/include/replacedialog.inc
===================================================================
--- lcl/include/replacedialog.inc	(revision 57570)
+++ lcl/include/replacedialog.inc	(working copy)
@@ -73,7 +73,6 @@
   AutoSize := True;
   BorderIcons := [biSystemMenu, biHelp];
   Caption := 'Replace Text';
-  Position := poMainFormCenter;
   LCLVersion := '1.3';
 
   TextLabel := TLabel.Create(Self);
finddialog.diff (1,613 bytes)   

Serge Anvarov

2018-03-28 16:58

reporter   ~0107444

Patch added

Bart Broersma

2018-03-28 17:23

developer   ~0107445

(0,0) as indicator for not setting Left/Top?
Is that how Delphi does it?

B.t.w.: what about other TCommonDialog descendents?

Serge Anvarov

2018-03-28 19:49

reporter   ~0107447

Last edited: 2018-03-28 19:50

View 2 revisions

Yes (0,0) is default value for fields - InitInstance behavior. Delphi default is (-1,-1), but it uses WinAPI FindText function, witch has own default position.
I do not know about TCommonDialog, because these fields (FFormTop, FFormLeft) are defined only in TFindDialog.

Bart Broersma

2018-03-28 23:12

developer   ~0107454

(-1,-1) to me seems a better value to indicate no Top/Left is set, and then use poMainFormCenter.

Bart Broersma

2018-09-16 14:56

developer   ~0110786

Please test and close if OK.
Dialog is centered if both Top and Left < 0, which is wat Delphi 7 seems to do.

Serge Anvarov

2018-09-17 17:28

reporter   ~0110827

I don't like your correction at all.
Compare {bad} code
[code]
  if (Self.Position.x < 0) and (Self.Position.y < 0) then
  begin
    MfBound := Application.MainForm.BoundsRect;
    FFormTop := MfBound.Top + (((MfBound.Bottom - MfBound.Top) - aForm.Height) Div 2);
    FFormLeft := MfBound.Left + (((MfBound.Right - MfBound.Left) - aForm.Width) Div 2);
  end
  else
  begin
    FFormLeft := Position.x;
    FFormTop := Position.y;
  end;
[/code]
with
[code]
  if (FFormLeft < 0) and (FFormTop < 0) then
  begin
    aForm.Position := poMainFormCenter;
    FFormTop := aForm.Top;
    FFormLeft := aForm.Left;
  end;
[/code]
1. if ... => 8 procedure calls
2. MfBound := ... => unsafe calculation (TForm do it better, with check for nil), additional var
3. FFormLeft := Position.x; => 4 procedure call
    FFormTop := Position.y; => 4 procedure call

Withal in constructor
[code]
Position := Point(-1,-1);
[/code]
with 5 proc call and
[code]
FFormLeft := -1;
FFormTop := -1
[/code]

Let's take my correction, taking into account "-1".

Bart Broersma

2018-09-17 20:58

developer   ~0110831

Last edited: 2018-09-17 21:12

View 2 revisions

Unassigning.
Your comments about {bad} code have nothing to do with the original issue.

Serge Anvarov

2018-09-17 21:46

reporter   ~0110836

This is not a reason to do it carelessly. I added a thorough patch and it was ignored.

Bart Broersma

2018-09-17 22:01

developer   ~0110837

Last edited: 2018-09-17 22:02

View 2 revisions

While I can see your point about replacing Position := Point(-1,-1) with assignments to FFormTop/FFromLeft, the MFBound parts IMO should go inot a separate issue (and revision).

Also, since this form reacts to user interaction, a few cycles doesn't really matter. Code readability does however.

I'll commit the MFBound part seperately.
I think that code predates the poMainFormCenter define?

PS. The tone of your post just put me off. Hence I unassigned. (It's been a hard day at work here)

Bart Broersma

2018-09-17 22:16

developer   ~0110838

Committed the second part of your fix (poMainFormCenter) in r59049.

Bart Broersma

2018-09-17 22:17

developer   ~0110839

Please test and close if OK.

Serge Anvarov

2018-09-18 16:42

reporter   ~0110850

I apologize for my tone. Thanks, it's all right.

Issue History

Date Modified Username Field Change
2018-03-28 16:57 Serge Anvarov New Issue
2018-03-28 16:58 Serge Anvarov File Added: finddialog.diff
2018-03-28 16:58 Serge Anvarov Note Added: 0107444
2018-03-28 17:23 Bart Broersma LazTarget => -
2018-03-28 17:23 Bart Broersma Note Added: 0107445
2018-03-28 17:23 Bart Broersma Assigned To => Bart Broersma
2018-03-28 17:23 Bart Broersma Status new => feedback
2018-03-28 19:49 Serge Anvarov Note Added: 0107447
2018-03-28 19:49 Serge Anvarov Status feedback => assigned
2018-03-28 19:50 Serge Anvarov Note Edited: 0107447 View Revisions
2018-03-28 23:12 Bart Broersma Note Added: 0107454
2018-09-16 14:56 Bart Broersma Fixed in Revision => r59031
2018-09-16 14:56 Bart Broersma LazTarget - => 1.8.6
2018-09-16 14:56 Bart Broersma Note Added: 0110786
2018-09-16 14:56 Bart Broersma Status assigned => resolved
2018-09-16 14:56 Bart Broersma Fixed in Version => 1.8.6
2018-09-16 14:56 Bart Broersma Resolution open => fixed
2018-09-16 14:56 Bart Broersma Target Version => 1.8.6
2018-09-17 17:28 Serge Anvarov Note Added: 0110827
2018-09-17 17:28 Serge Anvarov Status resolved => assigned
2018-09-17 17:28 Serge Anvarov Resolution fixed => reopened
2018-09-17 20:58 Bart Broersma Note Added: 0110831
2018-09-17 20:58 Bart Broersma Assigned To Bart Broersma =>
2018-09-17 20:58 Bart Broersma Status assigned => new
2018-09-17 21:12 Bart Broersma Note Edited: 0110831 View Revisions
2018-09-17 21:46 Serge Anvarov Note Added: 0110836
2018-09-17 22:01 Bart Broersma Note Added: 0110837
2018-09-17 22:02 Bart Broersma Note Edited: 0110837 View Revisions
2018-09-17 22:02 Bart Broersma Assigned To => Bart Broersma
2018-09-17 22:02 Bart Broersma Status new => assigned
2018-09-17 22:07 Bart Broersma Fixed in Revision r59031 => r59031, r59048
2018-09-17 22:16 Bart Broersma Note Added: 0110838
2018-09-17 22:17 Bart Broersma Note Added: 0110839
2018-09-17 22:17 Bart Broersma Status assigned => resolved
2018-09-17 22:17 Bart Broersma Resolution reopened => fixed
2018-09-18 16:42 Serge Anvarov Note Added: 0110850
2018-09-18 16:42 Serge Anvarov Status resolved => closed
2018-09-19 12:12 wp Relationship added related to 0034297