View Issue Details

IDProjectCategoryView StatusLast Update
0021708LazarusLCLpublic2015-07-12 01:30
ReporterZoran VučenovićAssigned ToJuha Manninen 
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Product Version0.9.31 (SVN)Product Build 
Target VersionFixed in Version1.1 (SVN) 
Summary0021708: WinControl.CanFocus returns True when ParentForm is not visible.
DescriptionIf the control itself has properties Visible and Enabled set to true, but its parent form is not visible, CanFocus method returns true, but trying to invoke SetFocus raises the exception.
Additional InformationIf C is TWinControl (any TWinControl's descendant) then if C.ParentForm.Visible is False, this code will raise the exception:

...
  if C.CanFocus then // returns True!
    C.SetFocus; // raises
...

I believe that CanFocus should return True only when SetFocus will not raise the exception, so I'm uploading the patch which solves the problem.
TagsNo tags attached.
Fixed in Revisionr36803, r49526
LazTarget-
WidgetsetWin32/Win64
Attached Files
  • wincontrol.inc.patch (460 bytes)
    Index: lcl/include/wincontrol.inc
    ===================================================================
    --- lcl/include/wincontrol.inc	(revision 36724)
    +++ lcl/include/wincontrol.inc	(working copy)
    @@ -3606,7 +3606,7 @@
       Result := False;
       //Verify that every parent is enabled and visible before returning true.
       Form := GetParentForm(Self);
    -  if Form <> nil then
    +  if (Form <> nil) and Form.CanFocus then
       begin
         Control := Self;
         repeat
    
    wincontrol.inc.patch (460 bytes)
  • canfocus.patch (2,222 bytes)
    Index: forms.pp
    ===================================================================
    --- forms.pp	(revision 49287)
    +++ forms.pp	(working copy)
    @@ -603,7 +603,6 @@
                                    Raw: boolean = false;
                                    WithThemeSpace: boolean = true); override;
         procedure Release;
    -    function CanFocus: Boolean; override;
         procedure SetFocus; override;
         function SetFocusedControl(Control: TWinControl): Boolean ; virtual;
         procedure SetRestoredBounds(ALeft, ATop, AWidth, AHeight: integer);
    Index: include/customform.inc
    ===================================================================
    --- include/customform.inc	(revision 49287)
    +++ include/customform.inc	(working copy)
    @@ -2128,14 +2128,6 @@
         Free;
     end;
     
    -function TCustomForm.CanFocus: Boolean;
    -begin
    -  if Parent = nil then
    -    Result := IsControlVisible and Enabled
    -  else
    -    Result := inherited CanFocus;
    -end;
    -
     {------------------------------------------------------------------------------
            TCustomForm Method CloseQuery
     ------------------------------------------------------------------------------}
    Index: include/wincontrol.inc
    ===================================================================
    --- include/wincontrol.inc	(revision 49287)
    +++ include/wincontrol.inc	(working copy)
    @@ -3610,22 +3610,16 @@
     function TWinControl.CanFocus: Boolean;
     var
       Control: TWinControl;
    -  Form: TCustomForm;
     begin
    -  Result := False;
    -  //Verify that every parent is enabled and visible before returning true.
    -  Form := GetParentForm(Self);
    -  if Form <> nil then
    +  Control := Self;
    +  while Assigned(Control) do
       begin
    -    Control := Self;
    -    repeat
    -      if Control = Form then break;
    -      // test all except the Form if it is visible and enabled
    -      if not (Control.IsControlVisible and Control.Enabled) then Exit;
    -      Control := Control.Parent;
    -    until False;
    -    Result := True;
    +    // test if all are visible and enabled
    +    if not (Control.IsControlVisible and Control.Enabled) then
    +      Exit(False);
    +    Control := Control.Parent;
       end;
    +  Result := True;
     end;
     
     {------------------------------------------------------------------------------
    
    canfocus.patch (2,222 bytes)
  • cansetfocus.patch (4,926 bytes)
    Index: docs/xml/lcl/controls.xml
    ===================================================================
    --- docs/xml/lcl/controls.xml	(revision 49517)
    +++ docs/xml/lcl/controls.xml	(working copy)
    @@ -11138,16 +11138,48 @@
           </element>
           <!-- function Visibility: public -->
           <element name="TWinControl.CanFocus">
    -        <short>Is this control allowed to receive the focus?</short>
    -        <descr>A control can get the focus only when all of its Parents except the form are Visible and Enabled.
    -While CanFocus checks all control parents it does not check whether a form control is placed on can have focus.</descr>
    +        <short>Is this control allowed to receive the focus when parent form is visible?</short>
    +        <descr>
    +          <p>Checks if the control can get focus when parent form is visible, i.e. if all its parents except the form are visible and enabled.
    +          </p>
    +          <p>A possible usage:</p>
    +          <pre>if FormFoo.EditFoo.CanFocus then
    +  FormFoo.ActiveControl := FormFoo.EditFoo;
    +          </pre>
    +          <p>Please note that this function returns True also if the parent form isn't visible and therefore a consecutive SetFocus call could throw an exception. Use <var>CanSetFocus</var> in this case.
    +          </p>
    +        </descr>
             <errors/>
    -        <seealso/>
    +        <seealso>
    +          <link id="TWinControl.CanSetFocus"/>
    +        </seealso>
           </element>
           <element name="TWinControl.CanFocus.Result">
             <short/>
           </element>
           <!-- function Visibility: public -->
    +      <element name="TWinControl.CanSetFocus">
    +        <short>Is this control allowed to receive the focus?</short>
    +        <descr>
    +          <p>Checks if the control can get focus, i.e. if all its parents are visible and enabled.
    +          </p>
    +          <p>A possible usage:</p>
    +          <pre>if MyControl.CanSetFocus then
    +  MyControl.SetFocus;
    +          </pre>
    +          <p><var>CanSetFocus</var> should be prefered over <var>CanFocus</var> if used in CanSetFocus/SetFocus combination
    +          because it checks also if the parent form can receive focus and thus prevents the "cannot focus an invisible window" LCL exception.
    +          </p>
    +        </descr>
    +        <errors/>
    +        <seealso>
    +          <link id="TWinControl.CanFocus"/>
    +        </seealso>
    +      </element>
    +      <element name="TWinControl.CanSetFocus.Result">
    +        <short/>
    +      </element>
    +      <!-- function Visibility: public -->
           <element name="TWinControl.GetControlIndex">
             <short>Finds the index value for the given control,
               in <link id="TWinControl.Controls">Controls[]</link>.
    Index: lcl/controls.pp
    ===================================================================
    --- lcl/controls.pp	(revision 49517)
    +++ lcl/controls.pp	(working copy)
    @@ -2164,6 +2164,7 @@
         destructor Destroy; override;
         procedure DockDrop(DragDockObject: TDragDockObject; X, Y: Integer); virtual;
         function CanFocus: Boolean; virtual;
    +    function CanSetFocus: Boolean; virtual;
         function GetControlIndex(AControl: TControl): integer;
         procedure SetControlIndex(AControl: TControl; NewIndex: integer);
         function Focused: Boolean; virtual;
    Index: lcl/include/wincontrol.inc
    ===================================================================
    --- lcl/include/wincontrol.inc	(revision 49517)
    +++ lcl/include/wincontrol.inc	(working copy)
    @@ -3604,8 +3604,8 @@
     
     {------------------------------------------------------------------------------
       TWinControl CanFocus
    -  
    -  
    +
    +
     ------------------------------------------------------------------------------}
     function TWinControl.CanFocus: Boolean;
     var
    @@ -3629,6 +3629,35 @@
     end;
     
     {------------------------------------------------------------------------------
    +  TWinControl CanSetFocus
    +
    +  CanSetFocus should be prefered over CanFocus if used in CanSetFocus/SetFocus
    +  combination
    +
    +  if MyControl.CanSetFocus then
    +    MyControl.SetFocus;
    +
    +  because it checks also if the parent form can receive focus and thus prevents
    +  the "cannot focus an invisible window" LCL exception.
    +------------------------------------------------------------------------------}
    +function TWinControl.CanSetFocus: Boolean;
    +var
    +  Control: TWinControl;
    +begin
    +  Control := Self;
    +  while True do
    +  begin
    +    // test if all are visible and enabled
    +    if not (Control.IsControlVisible and Control.Enabled) then
    +      Exit(False);
    +    if not Assigned(Control.Parent) then
    +      Break;
    +    Control := Control.Parent;
    +  end;
    +  Result := Control is TCustomForm;//the very top parent must be a form
    +end;
    +
    +{------------------------------------------------------------------------------
       TWinControl CreateSubClass
     ------------------------------------------------------------------------------}
     procedure TWinControl.CreateSubClass(var Params: TCreateParams;
    
    cansetfocus.patch (4,926 bytes)

Activities

2012-04-11 13:32

 

wincontrol.inc.patch (460 bytes)
Index: lcl/include/wincontrol.inc
===================================================================
--- lcl/include/wincontrol.inc	(revision 36724)
+++ lcl/include/wincontrol.inc	(working copy)
@@ -3606,7 +3606,7 @@
   Result := False;
   //Verify that every parent is enabled and visible before returning true.
   Form := GetParentForm(Self);
-  if Form <> nil then
+  if (Form <> nil) and Form.CanFocus then
   begin
     Control := Self;
     repeat
wincontrol.inc.patch (460 bytes)

Paul Ishenin

2012-04-12 05:34

manager   ~0058519

LCL behaves the same way as delphi do:
http://qc.embarcadero.com/wc/qcmain.aspx?d=32663

Zoran Vučenović

2012-04-12 08:49

developer   ~0058521

Last edited: 2012-04-12 08:50

Thank you, Paul, then it is correct behaviour. Strange is that CanFocus returns False if the control does not have ParentForm, but if it does have it, the function return true even when this form is disabled or hidden.

Then I believe that the documentation is misleading now.
The help entry for CanClose (http://lazarus-ccr.sourceforge.net/docs/lcl/controls/twincontrol.canfocus.html ) now says: "A control can get the focus only when all of its Parents are Visible and Enabled."
It should be changed to something like "A control can get the focus only when all its Parents, but not considering its parent form, are visible and enabled. CanFocus returns True even when its parent form is not visible and enabled".

I have to admit that I don't understand well how documentation is changed, so I can't provide patch for help file here. Would you update the help?

Paul Ishenin

2012-04-16 03:03

manager   ~0058637

Ok. I changed the docs.

Please close if ok.

Ondrej Pokorny

2015-06-08 12:47

developer   ~0084301

Since I don't see changed docs (http://lazarus-ccr.sourceforge.net/docs/lcl/controls/twincontrol.canfocus.html says the same "A control can get the focus only when all of its Parents are Visible and Enabled."), I'd like to discuss this issue again.

I know that Lazarus behaves here the same way like Delphi but many Delphi developers consider this a Delphi bug that Embarcadero just don't care about. And Lazarus should not repeat Delphi bugs just because they are in Delphi as well.

Basically, the current CanShow implementation fails to fulfill its basic purpose: prevent the "cannot be focused" exception in every case. As a result, every developer has to write and use his own CanShow replacement.

Also the Embarcadero docs are wrong about it. http://docwiki.embarcadero.com/Libraries/XE8/en/Vcl.Controls.TWinControl.CanFocus say "[CanShow] Indicates whether a control can receive focus. [...] CanFocus returns true if both the control and its parent(s) have their Visible and Enabled properties set to true.". This statement does not correspond with the current implementation.

Sample code:
procedure TForm1.Button1Click(Sender: TObject);
var
  xMF: TForm;
  xMemo: TMemo;
begin
  xMF := TForm.CreateNew(TComponent(Sender));
  try
    xMF.SetBounds(100, 100, 700, 500);
    xMF.WindowState := wsMaximized;
    xMemo := TMemo.Create(xMF);
    xMemo.Parent := xMF;
    if xMemo.CanFocus then // The result of CanFocus is True. But it should be False.
      xMemo.SetFocus; // Exception occurs
    xMF.Show;
  finally
    xMF.Free;
  end;
end;

Juha Manninen

2015-06-08 14:34

developer   ~0084303

I will open a mailing list thread to get more opinions.
Is the patch still valid?

Ondrej Pokorny

2015-06-08 15:01

developer  

canfocus.patch (2,222 bytes)
Index: forms.pp
===================================================================
--- forms.pp	(revision 49287)
+++ forms.pp	(working copy)
@@ -603,7 +603,6 @@
                                Raw: boolean = false;
                                WithThemeSpace: boolean = true); override;
     procedure Release;
-    function CanFocus: Boolean; override;
     procedure SetFocus; override;
     function SetFocusedControl(Control: TWinControl): Boolean ; virtual;
     procedure SetRestoredBounds(ALeft, ATop, AWidth, AHeight: integer);
Index: include/customform.inc
===================================================================
--- include/customform.inc	(revision 49287)
+++ include/customform.inc	(working copy)
@@ -2128,14 +2128,6 @@
     Free;
 end;
 
-function TCustomForm.CanFocus: Boolean;
-begin
-  if Parent = nil then
-    Result := IsControlVisible and Enabled
-  else
-    Result := inherited CanFocus;
-end;
-
 {------------------------------------------------------------------------------
        TCustomForm Method CloseQuery
 ------------------------------------------------------------------------------}
Index: include/wincontrol.inc
===================================================================
--- include/wincontrol.inc	(revision 49287)
+++ include/wincontrol.inc	(working copy)
@@ -3610,22 +3610,16 @@
 function TWinControl.CanFocus: Boolean;
 var
   Control: TWinControl;
-  Form: TCustomForm;
 begin
-  Result := False;
-  //Verify that every parent is enabled and visible before returning true.
-  Form := GetParentForm(Self);
-  if Form <> nil then
+  Control := Self;
+  while Assigned(Control) do
   begin
-    Control := Self;
-    repeat
-      if Control = Form then break;
-      // test all except the Form if it is visible and enabled
-      if not (Control.IsControlVisible and Control.Enabled) then Exit;
-      Control := Control.Parent;
-    until False;
-    Result := True;
+    // test if all are visible and enabled
+    if not (Control.IsControlVisible and Control.Enabled) then
+      Exit(False);
+    Control := Control.Parent;
   end;
+  Result := True;
 end;
 
 {------------------------------------------------------------------------------
canfocus.patch (2,222 bytes)

Ondrej Pokorny

2015-06-08 15:02

developer   ~0084304

@Juha: thanks, it's a great idea to open a mailing list thread.

Although the patch solves the issue, I don't consider it being "clean" - it depends on overridden TCustomForm.CanFocus and is unnecessarily complicated. I supplied another cleaner patch (canfocus.patch).

Ondrej Pokorny

2015-07-11 21:56

developer  

cansetfocus.patch (4,926 bytes)
Index: docs/xml/lcl/controls.xml
===================================================================
--- docs/xml/lcl/controls.xml	(revision 49517)
+++ docs/xml/lcl/controls.xml	(working copy)
@@ -11138,16 +11138,48 @@
       </element>
       <!-- function Visibility: public -->
       <element name="TWinControl.CanFocus">
-        <short>Is this control allowed to receive the focus?</short>
-        <descr>A control can get the focus only when all of its Parents except the form are Visible and Enabled.
-While CanFocus checks all control parents it does not check whether a form control is placed on can have focus.</descr>
+        <short>Is this control allowed to receive the focus when parent form is visible?</short>
+        <descr>
+          <p>Checks if the control can get focus when parent form is visible, i.e. if all its parents except the form are visible and enabled.
+          </p>
+          <p>A possible usage:</p>
+          <pre>if FormFoo.EditFoo.CanFocus then
+  FormFoo.ActiveControl := FormFoo.EditFoo;
+          </pre>
+          <p>Please note that this function returns True also if the parent form isn't visible and therefore a consecutive SetFocus call could throw an exception. Use <var>CanSetFocus</var> in this case.
+          </p>
+        </descr>
         <errors/>
-        <seealso/>
+        <seealso>
+          <link id="TWinControl.CanSetFocus"/>
+        </seealso>
       </element>
       <element name="TWinControl.CanFocus.Result">
         <short/>
       </element>
       <!-- function Visibility: public -->
+      <element name="TWinControl.CanSetFocus">
+        <short>Is this control allowed to receive the focus?</short>
+        <descr>
+          <p>Checks if the control can get focus, i.e. if all its parents are visible and enabled.
+          </p>
+          <p>A possible usage:</p>
+          <pre>if MyControl.CanSetFocus then
+  MyControl.SetFocus;
+          </pre>
+          <p><var>CanSetFocus</var> should be prefered over <var>CanFocus</var> if used in CanSetFocus/SetFocus combination
+          because it checks also if the parent form can receive focus and thus prevents the "cannot focus an invisible window" LCL exception.
+          </p>
+        </descr>
+        <errors/>
+        <seealso>
+          <link id="TWinControl.CanFocus"/>
+        </seealso>
+      </element>
+      <element name="TWinControl.CanSetFocus.Result">
+        <short/>
+      </element>
+      <!-- function Visibility: public -->
       <element name="TWinControl.GetControlIndex">
         <short>Finds the index value for the given control,
           in <link id="TWinControl.Controls">Controls[]</link>.
Index: lcl/controls.pp
===================================================================
--- lcl/controls.pp	(revision 49517)
+++ lcl/controls.pp	(working copy)
@@ -2164,6 +2164,7 @@
     destructor Destroy; override;
     procedure DockDrop(DragDockObject: TDragDockObject; X, Y: Integer); virtual;
     function CanFocus: Boolean; virtual;
+    function CanSetFocus: Boolean; virtual;
     function GetControlIndex(AControl: TControl): integer;
     procedure SetControlIndex(AControl: TControl; NewIndex: integer);
     function Focused: Boolean; virtual;
Index: lcl/include/wincontrol.inc
===================================================================
--- lcl/include/wincontrol.inc	(revision 49517)
+++ lcl/include/wincontrol.inc	(working copy)
@@ -3604,8 +3604,8 @@
 
 {------------------------------------------------------------------------------
   TWinControl CanFocus
-  
-  
+
+
 ------------------------------------------------------------------------------}
 function TWinControl.CanFocus: Boolean;
 var
@@ -3629,6 +3629,35 @@
 end;
 
 {------------------------------------------------------------------------------
+  TWinControl CanSetFocus
+
+  CanSetFocus should be prefered over CanFocus if used in CanSetFocus/SetFocus
+  combination
+
+  if MyControl.CanSetFocus then
+    MyControl.SetFocus;
+
+  because it checks also if the parent form can receive focus and thus prevents
+  the "cannot focus an invisible window" LCL exception.
+------------------------------------------------------------------------------}
+function TWinControl.CanSetFocus: Boolean;
+var
+  Control: TWinControl;
+begin
+  Control := Self;
+  while True do
+  begin
+    // test if all are visible and enabled
+    if not (Control.IsControlVisible and Control.Enabled) then
+      Exit(False);
+    if not Assigned(Control.Parent) then
+      Break;
+    Control := Control.Parent;
+  end;
+  Result := Control is TCustomForm;//the very top parent must be a form
+end;
+
+{------------------------------------------------------------------------------
   TWinControl CreateSubClass
 ------------------------------------------------------------------------------}
 procedure TWinControl.CreateSubClass(var Params: TCreateParams;
cansetfocus.patch (4,926 bytes)

Ondrej Pokorny

2015-07-11 21:58

developer   ~0084910

cansetfocus.patch: As discussed in the mailing list, let's not change CanFocus but add a new function CanSetFocus with whole parent tree check.

I also updated the docs.

Juha Manninen

2015-07-12 01:30

developer   ~0084911

Added new CanSetFocus with whole parent tree check. Original CanFocus was not changed.
Please test.

Issue History

Date Modified Username Field Change
2012-04-11 13:32 Zoran Vučenović New Issue
2012-04-11 13:32 Zoran Vučenović File Added: wincontrol.inc.patch
2012-04-11 13:32 Zoran Vučenović Widgetset => Win32/Win64
2012-04-11 13:36 Zeljan Rikalo Status new => assigned
2012-04-11 13:36 Zeljan Rikalo Assigned To => Paul Ishenin
2012-04-12 05:34 Paul Ishenin Note Added: 0058519
2012-04-12 08:49 Zoran Vučenović Note Added: 0058521
2012-04-12 08:50 Zoran Vučenović Note Edited: 0058521
2012-04-16 03:03 Paul Ishenin Fixed in Revision => 36803
2012-04-16 03:03 Paul Ishenin LazTarget => -
2012-04-16 03:03 Paul Ishenin Status assigned => resolved
2012-04-16 03:03 Paul Ishenin Fixed in Version => 1.1 (SVN)
2012-04-16 03:03 Paul Ishenin Resolution open => fixed
2012-04-16 03:03 Paul Ishenin Note Added: 0058637
2015-06-08 12:47 Ondrej Pokorny Note Added: 0084301
2015-06-08 14:34 Juha Manninen Assigned To Paul Ishenin => Juha Manninen
2015-06-08 14:34 Juha Manninen Note Added: 0084303
2015-06-08 14:34 Juha Manninen Status resolved => assigned
2015-06-08 14:34 Juha Manninen Resolution fixed => reopened
2015-06-08 15:01 Ondrej Pokorny File Added: canfocus.patch
2015-06-08 15:02 Ondrej Pokorny Note Added: 0084304
2015-07-11 21:56 Ondrej Pokorny File Added: cansetfocus.patch
2015-07-11 21:58 Ondrej Pokorny Note Added: 0084910
2015-07-12 01:30 Juha Manninen Fixed in Revision 36803 => r36803, r49526
2015-07-12 01:30 Juha Manninen Note Added: 0084911
2015-07-12 01:30 Juha Manninen Status assigned => resolved
2015-07-12 01:30 Juha Manninen Resolution reopened => fixed