All Projects | FPC | Lazarus: Packages, Patches | Lazarus CCR | Mantis | fpGUI | fpcprojects: fpprofiler

Main | My View | View Issues | Change Log | Roadmap
  

View Issue Details [ Jump to Notes ] [ Issue History ] [ Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0021942LazarusLCLpublic2012-05-05 14:112012-05-21 00:57
ReporterG. Colla 
Assigned ToBart Broersma 
PrioritynormalSeveritymajorReproducibilityalways
StatusclosedResolutionfixed 
PlatformOSOS Version
Product Version1.1 (SVN)Product Build 
Target Version1.0.0Fixed in Version 
Summary0021942: A BitButton with Kind bkClose doesn't close the main form.
DescriptionIf in a form is inserted a BitButton of kind bkClose, clicking on it, or typing enter or space when the button has focus doesn't close the form.
Additional Informationin bitbtn.inc

TCustomBitBtn.Click

performs the following check:

if (FKind = bkClose) and (ModalResult = mrNone) then begin
    Form := GetParentForm(Self);
    if Form <> nil then begin
      Form.Close;
[...]

but setting the button kind to bkClose now sets ModalResult to mrClose, and this makes the test fail.

The fix can be either to revert the ModalResult for bkClose to mrNone, as it was, or to modify the test into:
 if (FKind = bkClose) or (ModalResult = mrClose)

TagsNo tags attached.
Fixed in Revision37299
LazTarget-
Widgetset
Attached Filesdiff file icon bitbtn.bkclose-close-non-modal-windows-fix.diff [^] (1,102 bytes) 2012-05-08 23:35 [Show Content]

- Relationships

-  Notes
(0059260)
Bart Broersma (developer)
2012-05-05 17:24
edited on: 2012-05-05 17:28

 Apparently (?) MainForm is not a modal form so setting ModalResult to any value doesn't close it.
How does delphi handle a TBitBtn with Kind=bkClose on the mainform?
Does this depend on the value of TBitBtn.ModalResult?
(I cannot test this)
(0059270)
G. Colla (reporter)
2012-05-05 20:36
edited on: 2012-05-06 00:11

 I have only Delphi Kylix sources at hand.
On a BitBtn click event it just checks the Kind: if Kind is bkClose, then it closes the parent form, without looking at ModalResult, which has little to do with close action.
IOW instead of

if (FKind = bkClose) and (ModalResult = mrNone) then begin

just

if (FKind = bkClose) then begin

However there are two different Delphi compatibility issues:

1) Bot for Delphi compatibility and for common sense, bkClose must close the form, be it modal or not and whatever the ModalResult has been assigned.

2) What should be the Delphi Compatible default ModalResult for a bkClose
Kind? From what I've gathered, Delphi is testing Kind (and therefore closing the form) before setting modal result, so mrNone is returned in any case.

See:
http://www.google.com/url?q=http://comments.gmane.org/gmane.comp.ide.lazarus.general/62384&sa=U&ei=5KClT4zTJs3E4gSquJDMCQ&ved=0CBIQFjAA&usg=AFQjCNEoUBL2zEmDd0aoWDOJOZOJdQt2vw [^]

I believe that the error has been that of changing the default ModalResult of bkClose from mrNone to mrClose, and adding a test for ModalResult = mrNone in TBitBtnClick.
This breaks Delphi compatibility, existing applications, and common sense.
(0059280)
Bart Broersma (developer)
2012-05-05 23:57

 Please do not look at the Delphi sources.
Until recently mrClose did not exist in Delphi, so Modal forms just closed, not returning mrClose as result.
This has changed now.
Someone with recent Delphi (XE?) should test this.
Put a TBitBtn on the mainform, set Kind = bkClose, ModalResult = mrClose
Build, run, click button.
Does mainform close?
Repeat this with Kind = bkClose, ModalResult = mrOk and ModalResult = mrNone.
Just a blackbox approach: test and report back.
(0059284)
G. Colla (reporter)
2012-05-06 00:41

 If mrClose is a recent addition, then I bet that the problem is elsewhere:
in

procedure TCustomForm.Close;
var
  CloseAction: TCloseAction;
  IsMainForm: Boolean;
begin
  if fsModal in FFormState then
    ModalResult := mrCancel
  else
[...]

ModalResult := mrCancel should be changed in ModalResult := mrClose

Let's wait for recent Delphi tests, but I'm convinced that this is all what Delphi people has done. It doesn't break any previous compatibility, it doesn't rely on bkClose ModalResult, and just adds a bit of information.
(0059288)
Luiz Americo (developer)
2012-05-06 02:35

 > Repeat this with Kind = bkClose, ModalResult = mrOk and ModalResult = mrNone.

Delphi 7 (there's no mrClose)
bkClose and mrNone: close the form
bkClose and mrOk/mrCacel: DO NOT close the form
(0059308)
Bart Broersma (developer)
2012-05-06 14:05

 @G. Colla: can you try this implementation?

procedure TCustomBitBtn.Click;
var
  Form : TCustomForm;
begin
  { A TBitBtn with Kind = bkClose should
    - Close the ParentForm if ModalResult = mrNone.
      It should not set ParentForm.ModalResult in this case
    - Close a non-modal ParentForm if ModalResult in [mrNone, mrClose]
    - In all other cases it should behave like any other TBitBtn
  }
  if (FKind = bkClose) then
  begin
    Form := GetParentForm(Self);
    if (Form <> nil) then
    begin
      if (ModalResult = mrNone) or
         ((ModalResult = mrClose) and not (fsModal in Form.FormState)) then
      begin
        Form.Close;
        Exit;
      end;
    end;
  end;
  inherited Click;
end;
(0059417)
Bart Broersma (developer)
2012-05-08 23:36

 Target 1.0 for review of patch.
(0059448)
G. Colla (reporter)
2012-05-09 21:43

 Sorry for the delay, but I'm traveling abroad, with poor connections.
I'll test ASAP, but looking at code, I'm pretty sure that it'll be fine.
(0059512)
G. Colla (reporter)
2012-05-12 13:44

 Lazarus 1.1 r37254M FPC 2.6.0 i386-linux-gtk 2
with proposed patch.

Test results:

BitBtn Kind= bkClose.

1) In non modal form.
  a) Modal result set to mrClose (default): Form is closed.
  b) Modal result set to mrNone: Form is closed.
  c) Modal result set to mrOk: Form is not closed

2) In modal form
  a) Click on BitBtn: Form is closed. Modal result= mrClose
  b) Close Form with ALT-F4. Form is closed. Modal result=mrCancel
  c) Close form with X on top corner. Form is closed. Modal Result= mrCancel

Same tests with Qt WS (just in case). No difference whatsoever.

For a normal usage it always works as expected. I don't know about new Delphi compatibility of 2)-b) and 2)-c) but in any case it would be a very minor issue IMHO.
(0059660)
Bart Broersma (developer)
2012-05-17 09:03

 Please close if OK.

- Issue History
Date Modified Username Field Change
2012-05-05 14:11 G. Colla New Issue
2012-05-05 17:24 Bart Broersma Note Added: 0059260
2012-05-05 17:28 Bart Broersma Note Edited: 0059260
2012-05-05 20:36 G. Colla Note Added: 0059270
2012-05-05 23:57 Bart Broersma Note Added: 0059280
2012-05-06 00:00 Bart Broersma Status new => assigned
2012-05-06 00:00 Bart Broersma Assigned To => Bart Broersma
2012-05-06 00:11 G. Colla Note Edited: 0059270
2012-05-06 00:41 G. Colla Note Added: 0059284
2012-05-06 02:35 Luiz Americo Note Added: 0059288
2012-05-06 14:05 Bart Broersma Note Added: 0059308
2012-05-08 23:33 Bart Broersma Assigned To Bart Broersma =>
2012-05-08 23:35 Bart Broersma File Added: bitbtn.bkclose-close-non-modal-windows-fix.diff
2012-05-08 23:36 Bart Broersma LazTarget => -
2012-05-08 23:36 Bart Broersma Note Added: 0059417
2012-05-08 23:36 Bart Broersma Target Version => 1.0.0
2012-05-09 14:17 Maxim Ganetsky Status assigned => acknowledged
2012-05-09 21:43 G. Colla Note Added: 0059448
2012-05-12 13:44 G. Colla Note Added: 0059512
2012-05-16 23:02 Bart Broersma Status acknowledged => assigned
2012-05-16 23:02 Bart Broersma Assigned To => Bart Broersma
2012-05-17 09:03 Bart Broersma Fixed in Revision => 37299
2012-05-17 09:03 Bart Broersma Status assigned => resolved
2012-05-17 09:03 Bart Broersma Resolution open => fixed
2012-05-17 09:03 Bart Broersma Note Added: 0059660
2012-05-21 00:57 G. Colla Status resolved => closed

Main | My View | View Issues | Change Log | Roadmap
  

MantisBT 1.2.12[^]
Copyright © 2000 - 2012 MantisBT Group
Powered by Mantis Bugtracker