View Issue Details

IDProjectCategoryView StatusLast Update
0015390LazarusLCLpublic2012-04-19 08:03
ReporterStephano Assigned ToZeljan Rikalo  
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionno change required 
Product Version0.9.29 (SVN) 
Summary0015390: Hiding a modal form should not close it
DescriptionHiding a modal form closes it:

If the form hides itself in its OnShow event, a segmentation fault occurs when we try to access any of the form's components (use the attached test project as is).

If the form hides itself after its OnShow event, the form simply closes (comment/uncomment the "Self.Hide" in the attached test project to test).
TagsNo tags attached.
Fixed in Revision35525,35533,36902
LazTarget1.2
Widgetset
Attached Files

Activities

2009-12-19 13:28

 

ModalHide.zip (98,516 bytes)

Vincent Snijders

2009-12-19 20:45

manager   ~0033188

To me it seems that hiding a modal form is a way to make your app unresponsive, no input can be given to app.

What is the purpose of an hidden modal form? Closing is seems a good thing.

Stephano

2009-12-20 18:57

developer   ~0033208

1- A modal form may hide itself to show another modal form. Once the 2nd modal form is closed, the 1st modal form shows itself:
with TForm2.create(self) do
try
  Self.Hide;
  ShowModal;
finally
  Self.Show;
  Free;
end;

2- A modal form may hide itself if the usage requires it to do so, such as in the case of a graphical application that has a pseudo command line mode.

3- IIRC, Delphi allows hiding of the modal form :)

Zeljan Rikalo

2012-01-07 11:01

developer   ~0055546

Last edited: 2012-01-07 21:02

Maybe Delphi allows it but Delphi works only on windows, so such implementation could raise problems under carbon and x11. I'm for closing as "not fixable".
UPDATE: maybe there's a chance to fix it in ellegant way .. I'll check some possibilities.

Zeljan Rikalo

2012-02-06 15:39

developer   ~0056584

Updated category without widgetsets since it's pure LCL problem for a few years.

2012-02-06 16:06

 

modalformhide_issue15390.diff (2,674 bytes)   
Index: lcl/include/customform.inc
===================================================================
--- lcl/include/customform.inc	(revision 35180)
+++ lcl/include/customform.inc	(working copy)
@@ -2100,8 +2100,6 @@
 ------------------------------------------------------------------------------}
 procedure TCustomForm.Hide;
 begin
-  if (fsModal in FormState) and (ModalResult = 0) then
-    ModalResult := mrCancel;
   Visible := False;
 end;
 
@@ -2724,6 +2722,7 @@
   DisabledList: TList;
   SavedFocusState: TFocusState;
   ActiveWindow: HWnd;
+  HiddenModalForm: Boolean;
 begin
   if Self = nil then
     raise EInvalidOperation.Create('TCustomForm.ShowModal Self = nil');
@@ -2740,7 +2739,7 @@
   if GetCapture <> 0 then
     SendMessage(GetCapture, LM_CANCELMODE, 0, 0);
   ReleaseCapture;
-
+  HiddenModalForm := False;
   Application.ModalStarted;
   try
     Include(FFormState, fsModal);
@@ -2783,22 +2782,34 @@
             if ModalResult<>0 then break;
           end;
           Application.Idle(true);
-        until False;
+          HiddenModalForm := not Visible and (ModalResult = 0);
+        until False or HiddenModalForm;
 
+        if HiddenModalForm then
+          ModalResult := 0;
         Result := ModalResult;
         if HandleAllocated and (GetActiveWindow <> Handle) then
           ActiveWindow := 0;
         RestoreFocusedForm;
       finally
         Screen.EnableForms(DisabledList);
-        { guarantee execution of widgetset CloseModal }
-        TWSCustomFormClass(WidgetSetClass).CloseModal(Self);
-        Hide;
-        // free handles to save resources and to reduce overhead in the interfaces
-        // for bookkeeping changing between Show and ShowModal.
-        // (e.g.: the gtk interface creates some specials on ShowModal, so the
-        //  combination ShowModal, Close, Show makes problems.)
-        DestroyHandle;
+
+        if HiddenModalForm then
+          TWSCustomFormClass(WidgetSetClass).CloseModal(Self)
+        else
+        begin
+          { guarantee execution of widgetset CloseModal }
+          TWSCustomFormClass(WidgetSetClass).CloseModal(Self);
+          Hide;
+          // only here we set modal result
+          if ModalResult = 0 then
+            ModalResult := mrCancel;
+          // free handles to save resources and to reduce overhead in the interfaces
+          // for bookkeeping changing between Show and ShowModal.
+          // (e.g.: the gtk interface creates some specials on ShowModal, so the
+          //  combination ShowModal, Close, Show makes problems.)
+          DestroyHandle;
+        end;
       end;
     finally
       RestoreFocusState(SavedFocusState);
modalformhide_issue15390.diff (2,674 bytes)   

Zeljan Rikalo

2012-02-06 16:10

developer   ~0056585

Last edited: 2012-02-06 16:11

@Stephano, can you test attached patch ? I've tested it with gtk,gtk2 and qt under linux and win32 under wine and it works fine (no harm for normal modal forms), AND IT'LL work correct with such example, BUT IF YOU SAY SO:
try
  Self.Hide;
  ShowModal;
finally
  Self.ShowModal; // note this ShowModal again
  Free;
end;
it won't work. Self.ShowModal after Self.Hide won't work since newly created form taken modal loop and fsModal stays setted up on Self.
You can play with patch ... maybe removing fsModal in .Hide could fix things, anyway I'm not sure that this will be implemented in 1.0 since it can raise a lot of new bugs.

UPDATE: I've tested things with your example which works correct with this
patch , but also lazarus ide with gtk2 and qt ws - no sideeffects.

Zeljan Rikalo

2012-02-06 17:35

developer   ~0056589

Not blocker, postponed. If passes all tests it's easy to commit into 1.0.

Zeljan Rikalo

2012-02-21 09:36

developer   ~0056953

Please test and close if ok.
I've tested your example "as is" + removed Self.Hide from Form2.OnShow, and make it hidden when button "Hide" is clicked. Everything works fine.

Zeljan Rikalo

2012-02-21 10:20

developer   ~0056954

Reopened, must fix TCustomForm.Hide to be Delphi compatibile first.
It should not change ModalResult of fsModal there.

Zeljan Rikalo

2012-02-21 12:26

developer   ~0056966

Please test and close if ok.

Justin Smyth

2012-04-16 13:56

reporter   ~0058651

I've just gone from Lazarus-0.9.31-35368-fpc-2.6.1-20120215-win32 to Lazarus-1.1-36784-fpc-2.6.1-20120415-win32 and have noticed when a hide is called the showmodal exits. ( caused by this mod. i commented it out and the problem no longer contiues.)

Stephano

2012-04-16 16:29

developer   ~0058652

I tested with Lazarus 1.1 r36794 FPC 2.6.1 i386-linux-gtk 2.
ShowModal now produces a non modal form in the sense that the parent form (if not hidden) can be dragged/resized with the mouse. Otherwise, the behaviour is modal.

Furthermore, hiding the modal form still closes it. Pls test using the new test project.

2012-04-16 16:30

 

ModalHideTest2.zip (94,807 bytes)

Zeljan Rikalo

2012-04-16 17:30

developer   ~0058654

@Stephano, your example is wrong.
you cannot say:
procedure TForm1.Button1Click(Sender: TObject);
var
  Form2: TForm2;
begin
  Form2 := TForm2.Create(Self);
  try
    Self.Hide;
    Form2.ShowModal;
  finally
    Self.Show;
    Form2.Free; // <<----- see problem ?
  end;
end;

You have been started modal form and then you freed it , so how do you expect it to be shown in OnTimer event ?
Comment Form2.Free; and it'll work as you expected.

2012-04-17 13:55

 

TestShowModal.zip (3,478 bytes)

Justin Smyth

2012-04-17 13:57

reporter   ~0058675

I've uploaded some source code which shows the bug.

code behind my button on the main form

  frmShowModal := TfrmShowModal.Create(self);
  frmShowModal.ShowModal;
  Messagedlg('frmShowModal Exited',mtwarning,[mbok],0);
  frmShowModal.free;


and in the child form on the either formshow or formactivate i simply turn on a timer ie Timer1.Enabled := true;

and in the timer

  Timer1.Enabled:=false;
  Hide;

Zeljan Rikalo

2012-04-17 14:30

developer   ~0058676

Not bug, I've explained it on mailing list. Problem is wrong perception of what is modal and how it works.

Justin Smyth

2012-04-17 22:41

reporter   ~0058692

I beg to differ it's a bug. unless lazarus is breaking compatibility with delphi as Delphi does state showmodal only closes when you close the form not when you hide the form.

Zeljan Rikalo

2012-04-18 05:50

developer   ~0058693

I dont't know what delphi does, but under lazarus
ShowModal runs it's own loop, when form is hidden it goes out of that loop - that's all, form is still allocated and you can use it again.
So, eg. autocreated form can be used many times by using ShowModal; Hide; ShowModal; Hide; Show; Hide; ShowModal; Hide etc etc .... when you call Free (like it's called in attached examples) that form is destroyed and formclose is called.

2012-04-18 09:20

 

modalloopdelphicompat.diff (871 bytes)   
Index: lcl/include/customform.inc
===================================================================
--- lcl/include/customform.inc	(revision 36892)
+++ lcl/include/customform.inc	(working copy)
@@ -2909,12 +2909,13 @@
             (ModalResult = 0) and not Visible then
           begin
             DebugLn('NOTICE: TCustomForm.ShowModal() modal form invisible in loop ',dbgsName(Self));
-            ModalResult := mrCancel;
+            // ModalResult := mrCancel;
             // CloseModal; do not process CloseModal stuff,
             // it can trigger OnCloseQuery and that's not our intention
             // since we are here because programmer called Hide of modal form.
+            Exclude(FFormState, fsModal);
             Application.Idle(HasVisibleForms);
-            break;
+            // break;
           end;
 
           Application.Idle(true);
modalloopdelphicompat.diff (871 bytes)   

Zeljan Rikalo

2012-04-18 09:20

developer   ~0058703

Please test with attached patch - modalloopdelphicompat.diff

Stephano

2012-04-18 09:30

developer   ~0058704

Last edited: 2012-04-18 09:38

Lazarus LCL help: "Show this form as modal - ie control cannot be resumed by another form until the current form is closed"

Embarcadero online help: "ShowModal does not return until the form closes"

Edit: This note was posted before seeing Zeljan's last post above

Stephano

2012-04-18 09:51

developer   ~0058706

Tested modalloopdelphicompat.diff patch with project test3:

- If Form2 is hidden for 1 sec by pressing btnHide, and then closed by clicking on the [x] button (close/maximize/minimize/restore), Form2 is hidden and the application stalls.

- If Form2 is closed by pressing the btnClose (modal result=mrClose), everything is fine.

2012-04-18 09:51

 

test3.zip (2,781 bytes)

Justin Smyth

2012-04-18 11:58

reporter   ~0058711

with that patch it now works. sorry for being a pain in the arse about this. it works the way i'd expect it to

tho can you explain why you do this ?

Exclude(FFormState, fsModal);

in the loop ?

Stephano

2012-04-18 12:46

developer   ~0058713

@Justin, modalloopdelphicompat.diff patch suffers from a bug as mentioned in my note above. Please use the mailing list for discussions.

Zeljan Rikalo

2012-04-18 18:14

developer   ~0058719

@Justin, because if you try to ShowModal your "live but hidden" form next time it will raise error about fsModal is already in FFormState.
@Stephano: I didn't test your example yet - no spare time, but I'll give my best next 2-3 days.

2012-04-18 18:39

 

modalloopdelphicompat_2.diff (925 bytes)   
Index: lcl/include/customform.inc
===================================================================
--- lcl/include/customform.inc	(revision 36897)
+++ lcl/include/customform.inc	(working copy)
@@ -2909,12 +2909,13 @@
             (ModalResult = 0) and not Visible then
           begin
             DebugLn('NOTICE: TCustomForm.ShowModal() modal form invisible in loop ',dbgsName(Self));
-            ModalResult := mrCancel;
+            // ModalResult := mrCancel;
             // CloseModal; do not process CloseModal stuff,
             // it can trigger OnCloseQuery and that's not our intention
             // since we are here because programmer called Hide of modal form.
-            Application.Idle(HasVisibleForms);
-            break;
+            // Exclude(FFormState, fsModal);
+            // Application.Idle(HasVisibleForms);
+            // break;
           end;
 
           Application.Idle(true);

Zeljan Rikalo

2012-04-18 18:41

developer   ~0058720

Please test attached modalloopdelphicompat_2.diff.That's how it can be for delphi compatibility. Note that I will fight for current compatibility ;), or add property to TCustomForm which will define behaviour of hiding modal form via Hide(). Seem that both works fine.

Stephano

2012-04-18 19:18

developer   ~0058727

modalloopdelphicompat_2.diff seems to be OK.

As for the behaviour you expect, I suggest to use Close instead of Hide. That would eliminate the need for cumbersome additional properties.

Zeljan Rikalo

2012-04-19 06:03

developer   ~0058743

Yes, I've spotted it later ... anyone can test win32 with this patch ?

Zeljan Rikalo

2012-04-19 06:09

developer   ~0058744

Please test and close if ok.

Stephano

2012-04-19 08:02

developer   ~0058746

Great!

Tested on Ubuntu(GTK2) and Win32
Lazarus 1.1 r36902 FPC 2.6.1 i386-linux-gtk 2

Issue History

Date Modified Username Field Change
2009-12-19 13:28 Stephano New Issue
2009-12-19 13:28 Stephano File Added: ModalHide.zip
2009-12-19 13:28 Stephano Widgetset => GTK 2
2009-12-19 20:45 Vincent Snijders Note Added: 0033188
2009-12-19 20:46 Vincent Snijders LazTarget => -
2009-12-19 20:46 Vincent Snijders Status new => acknowledged
2009-12-20 18:57 Stephano Note Added: 0033208
2011-07-04 19:03 Zeljan Rikalo LazTarget - => 1.0
2012-01-07 11:01 Zeljan Rikalo Note Added: 0055546
2012-01-07 21:02 Zeljan Rikalo Note Edited: 0055546
2012-02-04 10:17 Zeljan Rikalo Status acknowledged => assigned
2012-02-04 10:17 Zeljan Rikalo Assigned To => Zeljan Rikalo
2012-02-06 15:39 Zeljan Rikalo Widgetset GTK 2 =>
2012-02-06 15:39 Zeljan Rikalo Note Added: 0056584
2012-02-06 16:06 Zeljan Rikalo File Added: modalformhide_issue15390.diff
2012-02-06 16:10 Zeljan Rikalo Note Added: 0056585
2012-02-06 16:10 Zeljan Rikalo Status assigned => feedback
2012-02-06 16:11 Zeljan Rikalo Note Edited: 0056585
2012-02-06 17:35 Zeljan Rikalo LazTarget 1.0 => 1.2
2012-02-06 17:35 Zeljan Rikalo Note Added: 0056589
2012-02-21 09:36 Zeljan Rikalo Fixed in Revision => 35525
2012-02-21 09:36 Zeljan Rikalo Status feedback => resolved
2012-02-21 09:36 Zeljan Rikalo Resolution open => fixed
2012-02-21 09:36 Zeljan Rikalo Note Added: 0056953
2012-02-21 10:20 Zeljan Rikalo Note Added: 0056954
2012-02-21 10:20 Zeljan Rikalo Status resolved => acknowledged
2012-02-21 12:26 Zeljan Rikalo Fixed in Revision 35525 => 35525,35533
2012-02-21 12:26 Zeljan Rikalo Status acknowledged => resolved
2012-02-21 12:26 Zeljan Rikalo Note Added: 0056966
2012-04-16 13:56 Justin Smyth Note Added: 0058651
2012-04-16 16:29 Stephano Status resolved => assigned
2012-04-16 16:29 Stephano Resolution fixed => reopened
2012-04-16 16:29 Stephano Note Added: 0058652
2012-04-16 16:30 Stephano File Added: ModalHideTest2.zip
2012-04-16 17:30 Zeljan Rikalo Status assigned => resolved
2012-04-16 17:30 Zeljan Rikalo Resolution reopened => no change required
2012-04-16 17:30 Zeljan Rikalo Note Added: 0058654
2012-04-17 13:55 Justin Smyth File Added: TestShowModal.zip
2012-04-17 13:57 Justin Smyth Note Added: 0058675
2012-04-17 14:30 Zeljan Rikalo Note Added: 0058676
2012-04-17 22:41 Justin Smyth Note Added: 0058692
2012-04-18 05:50 Zeljan Rikalo Note Added: 0058693
2012-04-18 09:20 Zeljan Rikalo File Added: modalloopdelphicompat.diff
2012-04-18 09:20 Zeljan Rikalo Note Added: 0058703
2012-04-18 09:20 Zeljan Rikalo Status resolved => feedback
2012-04-18 09:30 Stephano Note Added: 0058704
2012-04-18 09:38 Stephano Note Edited: 0058704
2012-04-18 09:51 Stephano Note Added: 0058706
2012-04-18 09:51 Stephano File Added: test3.zip
2012-04-18 11:58 Justin Smyth Note Added: 0058711
2012-04-18 12:46 Stephano Note Added: 0058713
2012-04-18 18:14 Zeljan Rikalo Note Added: 0058719
2012-04-18 18:39 Zeljan Rikalo File Added: modalloopdelphicompat_2.diff
2012-04-18 18:41 Zeljan Rikalo Note Added: 0058720
2012-04-18 19:18 Stephano Note Added: 0058727
2012-04-19 06:03 Zeljan Rikalo Note Added: 0058743
2012-04-19 06:09 Zeljan Rikalo Fixed in Revision 35525,35533 => 35525,35533,36902
2012-04-19 06:09 Zeljan Rikalo Status feedback => resolved
2012-04-19 06:09 Zeljan Rikalo Note Added: 0058744
2012-04-19 08:02 Stephano Note Added: 0058746
2012-04-19 08:03 Stephano Status resolved => closed