Lazarus

Programming => Widgetset => Cocoa => Topic started by: VTwin on January 14, 2020, 12:22:55 am

Title: [SOLVED] TCheckbox bug
Post by: VTwin on January 14, 2020, 12:22:55 am
The TCheckbox.OnChange event is incorrectly called if TCheckBox.Checked is True in IDE.

Title: Re: TCheckbox bug
Post by: skalogryz on January 14, 2020, 02:32:32 am
please try r62545
Title: Re: TCheckbox bug
Post by: VTwin on January 14, 2020, 04:11:37 pm
Excellent, it is working in trunk. Thanks!!
Title: Re: [SOLVED] TCheckbox bug
Post by: VTwin on January 14, 2020, 05:21:26 pm
Fixed in fixes too!  :)
Title: Re: TCheckbox bug - not quite solved
Post by: VTwin on January 14, 2020, 05:49:59 pm
Unfortunately now it is not possible to change checkbox.checked in the IDE. Checked ;) both in trunk and fixes.

 :(
Title: Re: TCheckbox bug - not quite solved
Post by: skalogryz on January 15, 2020, 04:45:39 am
svn up
Title: Re: TCheckbox bug - not quite solved
Post by: VTwin on January 15, 2020, 06:15:29 pm
Thanks, I can now change checked true/false in the IDE. :)

However, if it is unchecked (false) in the IDE, and I have:

Code: Pascal  [Select][+][-]
  1. procedure TForm1.FormShow(Sender: TObject);
  2. begin
  3.   CheckBox1.Checked := true;
  4. end;


The Change event is triggered. :(
Title: Re: TCheckbox bug - not quite solved
Post by: lucamar on January 15, 2020, 07:35:17 pm
Quote from: VTwin on January 15, 2020, 06:15:29 pm
[..] if it is unchecked (false) in the IDE, and I have:
Code: Pascal  [Select][+][-]
  1. procedure TForm1.FormShow(Sender: TObject);
  2. begin
  3.   CheckBox1.Checked := true;
  4. end;
The Change event is triggered.

I'm curious, why do you think it shouldn't? After all it's supposed to be triggered on any property change.

Or is it because you change it in code vs. user action?
Title: Re: TCheckbox bug - not quite solved
Post by: VTwin on January 15, 2020, 07:52:00 pm
Quote from: lucamar on January 15, 2020, 07:35:17 pm
I'm curious, why do you think it shouldn't? After all it's supposed to be triggered on any property change.

Or is it because you change it in code vs. user action?

It is my understanding, and observation, that a control should respond to a user action, and not trigger a change event if assigned a value in code. For, example, if you set a ComboBox item index in code, that should not (and does not) trigger a change event.

I have many dialog boxes with controls that are set from stored preferences. In Carbon there was a persistent bug where some controls would trigger a change event when set in code. My code was littered with code like:

Code: Pascal  [Select][+][-]
  1. procedure TForm1.ComboBox1Change(Sender: TObject);
  2. begin
  3.   {$IFDEF DARWIN}
  4.   if fSettingControls then
  5.     exit; // because the control is being set in code to a saved preference value
  6.   {$ENDIF}
  7.   // now do something that should be done when the user changes the control
  8.   ...
  9. end;

This was never a problem in Linux or Windows. I have recently been happily deleting all of these defines as the Cocoa widgetset has now surpassed Carbon.
Title: Re: TCheckbox bug - not quite solved
Post by: skalogryz on January 15, 2020, 08:22:28 pm
Quote from: VTwin on January 15, 2020, 06:15:29 pm
However, if it is unchecked (false) in the IDE, and I have:

The Change event is triggered. :(
That's expected actually.

Quote from: VTwin on January 15, 2020, 07:52:00 pm
It is my understanding, and observation, that a control should respond to a user action, and not trigger a change event if assigned a value in code. For, example, if you set a ComboBox item index in code, that should not (and does not) trigger a change event.
The answer is "inconsistency".

For example in Web-browser (JS) world "onchange" is triggered due to a user action. Changing the value via the software will not trigger the event.

In Cocoa API, "onchange" is triggered due to a user action. Changing the value via the software will not trigger the event.

In WinAPI world (inherited by Delphi, inherited by LCL), changing the value triggers the same "OnChange" event as user action does. BUT in WinAPI, there's typically a way to suppress the notification.
Specifically the checkbox:
Code: Pascal  [Select][+][-]
  1. class procedure TWin32WSCustomCheckBox.SetState(const ACustomCheckBox: TCustomCheckBox; const NewState: TCheckBoxState);
  2. var
  3.   Flags: WPARAM;
  4. begin
  5.   case NewState of
  6.     cbChecked: Flags := Windows.WParam(BST_CHECKED);
  7.     cbUnchecked: Flags := Windows.WParam(BST_UNCHECKED);
  8.   else
  9.     Flags := Windows.WParam(BST_INDETERMINATE);
  10.   end;
  11.   //Pass SKIP_LMCHANGE through lParam to avoid the OnChange event be fired
  12.   Windows.SendMessage(ACustomCheckBox.Handle, BM_SETCHECK, Flags, SKIP_LMCHANGE);
  13. end;
  14.  
Yet LCL itself would send out the notification.

Code: Pascal  [Select][+][-]
  1. procedure TCustomCheckBox.SetChecked(Value : Boolean);
  2. var
  3.   OldState: TCheckBoxState;
  4. begin
  5.   OldState := FState;
  6.   if Value then
  7.     FState := cbChecked
  8.   else
  9.     FState := cbUnChecked;
  10.   //debugln('TCustomCheckBox.SetChecked ',dbgsname(Self),' ',dbgs(ord(FState)));
  11.   if FState <> OldState then
  12.   begin
  13.     if Action is TCustomAction then
  14.       TCustomAction(Action).Checked := FState = cbChecked;
  15.     ApplyChanges;
  16.     //some widgetsets (gtk*) does not allow to uncheck a radio button
  17.     //only call OnChange if effectivelly changed
  18.     FState := RetrieveState;
  19.     if FState <> OldState then
  20.       DoClickOnChange;
  21.   end;
  22. end;
  23.  

Most of LCL Controls would trigger the notification, either way. And the documentation should be referred to.

In case of the check box, you can avoid the notification by using
Code: Pascal  [Select][+][-]
  1.   checkbox1.State:=csChecked;
  2.  
instead of using checked property
Title: Re: TCheckbox bug - not quite solved
Post by: VTwin on January 15, 2020, 09:46:11 pm
Thanks for the illuminating answer skalogryz. It has left me puzzled though.

Quote from: skalogryz on January 15, 2020, 08:22:28 pm
The answer is "inconsistency".

I figured the controls should behave the same on each platform. :(

Quote from: skalogryz on January 15, 2020, 08:22:28 pm
For example in Web-browser (JS) world "onchange" is triggered due to a user action. Changing the value via the software will not trigger the event.

In Cocoa API, "onchange" is triggered due to a user action. Changing the value via the software will not trigger the event.

In WinAPI world (inherited by Delphi, inherited by LCL), changing the value triggers the same "OnChange" event as user action does.

So, Lazarus is emulating the Windows/Delphi model? From my experiences (but I should test), Linux and Windows seem not to  send an OnChange when set in code, while some of Carbon's did. I figured it was a Carbon bug.  Maybe it is a Windows/Linux bug instead? %)

Quote from: skalogryz on January 15, 2020, 08:22:28 pm
BUT in WinAPI, there's typically a way to suppress the notification.

Yet LCL itself would send out the notification.

Most of LCL Controls would trigger the notification, either way. And the documentation should be referred to.

So LCL does (or is supposed to) send an OnChange when a control state is set in code?

Quote from: skalogryz on January 15, 2020, 08:22:28 pm
In case of the check box, you can avoid the notification by using
Code: Pascal  [Select][+][-]
  1.   checkbox1.State:=csChecked;
  2.  
instead of using checked property

That does indeed work, and is good to know.

So, I guess I should assume that setting a control state in code does (or is supposed to) send an OnChange  event and work around that, or be careful to check the docs and always use a method such as "State := csChecked".

The JS/Cocoa model makes more sense to me, but as long as I know what is supposed to happen I can deal with it.

Thanks again for your explanations and quick fixes, they are much appreciated.
Title: Re: TCheckbox bug - not quite solved
Post by: VTwin on January 15, 2020, 10:05:16 pm
I just remembered that I use a helper class for all these dialog boxes, so changing:

Code: Pascal  [Select][+][-]
  1. procedure TVCustomCheckBoxHelper.SetBool(const b: boolean);
  2. begin
  3.   Checked := b;
  4. end;
  5.  

to:

Code: Pascal  [Select][+][-]
  1. procedure TVCustomCheckBoxHelper.SetBool(const b: boolean);
  2. begin
  3.   if b then
  4.    State := csChecked
  5.   else
  6.    State := csUnchecked
  7. end;

is a quick fix that should take care of it.  :)
Title: Re: TCheckbox bug - not quite solved
Post by: skalogryz on January 15, 2020, 10:16:54 pm
Quote from: VTwin on January 15, 2020, 09:46:11 pm
So, Lazarus is emulating the Windows/Delphi model? From my experiences (but I should test), Linux and Windows seem not to  send an OnChange when set in code, while some of Carbon's did. I figured it was a Carbon bug.  Maybe it is a Windows/Linux bug instead? %)
I would encourage you to play around and see how (and if) things are different.

But the common rule is "follow Windows widgetset (as it is most likely does what Delphi does)", no matter if it's poor design or not.
Title: Re: TCheckbox bug - not quite solved
Post by: VTwin on January 15, 2020, 10:20:12 pm
Quote from: skalogryz on January 15, 2020, 10:16:54 pm
But the common rule is "follow Windows widgetset (as it is most likely does what Delphi does)", no matter if it's poor design or not.

Will do!


I found this bug report:

https://bugs.freepascal.org/view.php?id=33076

that suggests changing TCheckBox.State should trigger an OnChange event. :o

That appears to contradict. Is the bug report incorrect? 
Title: Re: [SOLVED] TCheckbox bug
Post by: skalogryz on January 15, 2020, 11:26:49 pm
ahaha... I've just tried Delphi (10.3) and VCL TCheckBox doesn't have OnChange event at all!

now you see why it would ok for a LCL developer to suffer from a substance abuse :)


But technically the report is incorrect.

If we're seeking Delphi compatibility, THEN:
changing State via code should trigger "OnClick" event.
changing Checked via code should trigger "OnClick" event
(you can check that, and if it's not this way, you can surely file a bug report with the reference to Delphi compatibility)

Triggering event of OnChange is so solely in LCL responsibility. So, it should be strictly documented (in LCL documentation) and/or Requested For Comments (with the future documentation).

If it's documented, we can accepted the current behavior as expected and desired, and the bug report was created without any base. (solely on the developer's expectations). And should be closed as "no changes required".

P.S. The most sad thing for me, personally, is that LCL is out there for about 20 years now. But we're still have to handle such basic stuff as checkbox notifications.
Title: Re: [SOLVED] TCheckbox bug
Post by: skalogryz on January 15, 2020, 11:40:37 pm
Just for the record, but not the reference.

Delphi FMX checkbox doesn't seem to support "mixed" state at all. (so there's no "State" property, only "IsChecked")

Changing IsChecked via code triggers OnChange event, but doesn't trigger OnClick event.
Changing checked via mouse click triggers both OnChange and OnClick events.
Title: Re: [SOLVED] TCheckbox bug
Post by: VTwin on January 16, 2020, 01:14:43 am
Wow, thanks. That is confusing, I'm having a drink now myself. ;)

I've explored options for cross-platform development, and Lazarus is the best that I have found, especially as a one person team trying to do everything myself. As an end user, I'm just trying to understand what the expected behavior is, and report bugs as I can.

It does seem that the expected control behavior for controls should be well defined by now. Maybe Delphi's choices are not always the best for Lazarus. In my mind, if Lazarus had alternate options for setting controls that always follow JS/Cocoa convention, and the Delphi compatible options followed Delphi/Windows that would be a good thing.

Just my 2 cents. That and about $3 will get you a cup of Starbucks coffee in my country.

Cheers,
VTwin
Title: Re: [SOLVED] TCheckbox bug
Post by: VTwin on January 16, 2020, 01:20:05 am
Quote from: skalogryz on January 15, 2020, 11:40:37 pm
Just for the record, but not the reference.

Delphi FMX checkbox doesn't seem to support "mixed" state at all. (so there's no "State" property, only "IsChecked")

Changing IsChecked via code triggers OnChange event, but doesn't trigger OnClick event.
Changing checked via mouse click triggers both OnChange and OnClick events.

I suppose that is a good example where sticking strictly with Delphi is not the best option. Windows, Linux, and macOS support mixed state checkboxes. I'd say Delphi -> Lazarus compatibility is more important than Lazarus -> Delphi, but just my opinion.
TinyPortal © 2005-2018