Recent

Author Topic: [SOLVED] Event TComboBox.OnEditingDone fires to often (Focus problem?)  (Read 8879 times)

valdir.marcos

  • Hero Member
  • *****
  • Posts: 1106
Re: Event TComboBox.OnEditingDone fires to often (Focus problem?)
« Reply #15 on: July 14, 2018, 08:57:02 pm »
I personally think this is a bug. Switching tab in TPageControl should not cause the combobox to receive focus.
@wp: Do you also think that this is a bug?
[Sorry for stealing Handoko's answer.] Yes, it's a bug.
Changing TTabSheet in TPageControl should preserve focus inside TPageControl components. Focus would only get outside TPageControl if destine TTabSheet won't have any focusable component.
Today, we have to force similar behavior:
http://forum.lazarus.freepascal.org/index.php/topic,41874.html

wp

  • Hero Member
  • *****
  • Posts: 11830
Re: Event TComboBox.OnEditingDone fires to often (Focus problem?)
« Reply #16 on: July 14, 2018, 09:03:06 pm »
Very strange: If you move the PageControl to the top of the TabOrder (TabOrder = 0) then changing tabs keeps the focus with the PageControl.

The project in the attachment demonstrates the effect of changed TabOrder.
« Last Edit: July 14, 2018, 09:44:23 pm by wp »

Hartmut

  • Hero Member
  • *****
  • Posts: 739
Re: Event TComboBox.OnEditingDone fires to often (Focus problem?)
« Reply #17 on: July 14, 2018, 10:14:45 pm »
Very strange: If you move the PageControl to the top of the TabOrder (TabOrder = 0) then changing tabs keeps the focus with the PageControl.
The project in the attachment demonstrates the effect of changed TabOrder.
Ok, for me now it's clear that it is a bug, that the ComboBox receives the focus, when switching TabSheets in TPageControl, which occurs only on Windows, not on Linux Gtk2. I will write a bug report for this.

What is with the other problem, that this event is fired twice, as I described in reply #14, steps 1..5?
Quote
1) compile and start the program
2) enter e.g. 3 values into the ComboBox, so that the History contains 3 values
3) open the ComboBox History with the mouse
4) select a value with the arrow keys
5) press ENTER
Result: event TComboBox.OnEditingDone is fired twice.

If others think, that this is a bug too, I would write 1 bug report for both problems.

wp

  • Hero Member
  • *****
  • Posts: 11830
Re: Event TComboBox.OnEditingDone fires to often (Focus problem?)
« Reply #18 on: July 14, 2018, 10:20:32 pm »
What is with the other problem, that this event is fired twice, as I described in reply #14, steps 1..5?
Isn't my demo just covering this case?

Hartmut

  • Hero Member
  • *****
  • Posts: 739
Re: Event TComboBox.OnEditingDone fires to often (Focus problem?)
« Reply #19 on: July 14, 2018, 10:42:44 pm »
What is with the other problem, that this event is fired twice, as I described in reply #14, steps 1..5?
Isn't my demo just covering this case?
I think not, because your demo does not have a history for the combobox.

I expanded your demo by an TComboBox.OnEditingDone event and the combobox with a history. See attached project.
Quote
1) compile and start the program
2) enter e.g. 3 values into the ComboBox, so that the History contains 3 values
3) open the ComboBox History with the mouse
4) select a value with the arrow keys
5) press ENTER
Result: event TComboBox.OnEditingDone is fired twice, regardless which TabOrder is 0.

wp

  • Hero Member
  • *****
  • Posts: 11830
Re: Event TComboBox.OnEditingDone fires to often (Focus problem?)
« Reply #20 on: July 14, 2018, 11:36:28 pm »
But this is a completely different story. It does not have to do anything with the PageControl, it is related to the Combobox alone. Look at attached demo which contains only a combobox (plus a memo for registering the OnEditingDone events and a radiogroup for easy changing the Style of the Combobox): It fires the OnEditingDone event twice too if an item is selected from the dropdown with the ENTER key.

One OnEditingDone fires when the ENTER key is pressed - I did not yet find the code where this happens. But I think it sits deep inside the heart of the LCL - changing it will break everything...

The other OnEditingDone is created in the combobox method CloseUp:
Code: Pascal  [Select][+][-]
  1. procedure TCustomComboBox.CloseUp;
  2. begin
  3.   if [csLoading,csDestroying,csDesigning]*ComponentState<>[] then exit;
  4.   if (Style in [csSimple, csDropDown, csOwnerDrawEditableFixed, csOwnerDrawEditableVariable]) then
  5.     EditingDone;         // <--------- HERE!
  6.   if Assigned(FOnCloseUp) then FOnCloseUp(Self);
  7.   if FAutoSelect then
  8.    begin
  9.     SelectAll;
  10.     if (SelText = Text) then FAutoSelected := True;
  11.    end;//End if FAutoSelect
  12. end;
From the code it can be seen that the Style csDropDownList does not fire the second onEditingDone which in fact is observed in the demo. This EditingDone cannot be removed because a selection of an item from the dropdown with mouse would not fire the event. So, I am a bit clueless here what to do...

[EDIT]
Why is it harmful for your code when then OnEditingDone event fires twice?
« Last Edit: July 15, 2018, 11:11:20 am by wp »

Hartmut

  • Hero Member
  • *****
  • Posts: 739
Re: Event TComboBox.OnEditingDone fires to often (Focus problem?)
« Reply #21 on: July 15, 2018, 12:48:27 pm »
Look at attached demo which contains only a combobox (plus a memo for registering the OnEditingDone events and a radiogroup for easy changing the Style of the Combobox): It fires the OnEditingDone event twice too if an item is selected from the dropdown with the ENTER key.
I tried this demo and learned something new about ComboBoxes. And now I understand, why the Event is fired twice.

But I think, it's a bug, that 1 exit (via ENTER) fires an event twice. If the OnEditingDone-Event is already fired in procedure TCustomComboBox.CloseUp, then the 2nd event via ENTER in my opinion is a bug.
@wp: do you agree?

Quote
Why is it harmful for your code when then OnEditingDone event fires twice?
This event reads data from a SQL-DB, which can need some seconds.

The demo of wp inspired me to a solution, which works well:
 - set a flag in the TComboBox.OnEnter-Event
 - excecute the actions in TComboBox.OnEditingDone-Event only, if the flag is set
 - at the end of OnEditingDone-Event set the focus to another control, which forces the user to "enter" the ComboBox again, so that a new OnEnter-Event is fired
Now the OnEditingDone-Event is always fired twice (because of 'stealing' the focus), but this is now harmless because of the flag.

This is my code (project is attached):

Code: Pascal  [Select][+][-]
  1. unit Unit1;
  2.  
  3. {$mode objfpc}{$H+}
  4. {$apptype console} {neccessary for writeln}
  5.  
  6. interface
  7.  
  8. uses
  9.  Classes, SysUtils, Forms, Controls, Dialogs, StdCtrls, ComCtrls;
  10.  
  11. type
  12.  
  13.  TForm1 = class(TForm)
  14.   ComboBox1: TComboBox;
  15.   ListBox1: TListBox;
  16.   procedure ComboBox1Enter(Sender: TObject);
  17.   procedure ComboBox1EditingDone(Sender: TObject);
  18.  private
  19.  public
  20.  end;
  21.  
  22. var Form1: TForm1;
  23.  
  24. implementation
  25.  
  26. {$R *.lfm}
  27.  
  28. const enable_event: boolean = true;
  29.  
  30. procedure TForm1.ComboBox1Enter(Sender: TObject);
  31.    begin
  32.    writeln('OnEnter');
  33.    enable_event:=true; // enable the EditingDone-Event 1x
  34.    end;
  35.  
  36. procedure TForm1.ComboBox1EditingDone(Sender: TObject);
  37.    const nr: integer = 1;
  38.    var ee: boolean;
  39.    begin
  40.    writeln('OnEditingDone', nr, ' enable=', enable_event);
  41.    inc(nr);
  42.  
  43.    ee:=enable_event;    // memorize permission
  44.    enable_event:=false; // reset permission
  45.    if not ee then exit; // exit if no permission
  46.  
  47.    ListBox1.SetFocus;   // steal the focus from the ComboBox
  48.    end;
  49.  
  50. end.
« Last Edit: July 15, 2018, 12:52:08 pm by Hartmut »

wp

  • Hero Member
  • *****
  • Posts: 11830
Re: Event TComboBox.OnEditingDone fires to often (Focus problem?)
« Reply #22 on: July 15, 2018, 01:16:38 pm »
But I think, it's a bug, that 1 exit (via ENTER) fires an event twice. If the OnEditingDone-Event is already fired in procedure TCustomComboBox.CloseUp, then the 2nd event via ENTER in my opinion is a bug.
@wp: do you agree?
In principle, yes, and if you want you can report it to bugtracker, maybe somebody with good knowledge of the LCL picks it up and fixes it. I won't touch it myself because the OnEditingDone via ENTER originates from deep inside the LCL, the bug may even depend on the widgetset; therefore, I am rather sure that it may introduce a cascade of other bugs if not done correctly.

It is much safer to fix - well, maybe, work around - the issue by using the flag that you propose.

BTW: What do you think that no OnEditingDone event is created when the combobox style is csDropdownList and an item is picked from the dropdown using a mouse click? On the other hand, the event IS created when the item is selected with the ENTER key. Pretty much inconsistent in my eyes... What does "editing" mean in case a combobox with style csDropDownList? You cannot type anything in here, so no edit state? Then the ENTER key does not make sense either to fire OnEditingDone. But you still can SELECT an item from the combo. Is this editing? And suddenly things get complicated again...

And the confusion with OnEditingDone is nothing compared to the OnClick and OnChange events of many LCL controls. The only thing you can be sure of that practically every controls behaves differently. Normally the OnClick fires when the user changes the control by a click, but sometimes also by code. OnChange, sometimes, duplicates OnClick, sometimes not. etc.

But we are in good company: Delphi's VCL is not more consistant at all.


Hartmut

  • Hero Member
  • *****
  • Posts: 739
Re: Event TComboBox.OnEditingDone fires to often (Focus problem?)
« Reply #23 on: July 15, 2018, 02:41:20 pm »
But I think, it's a bug, that 1 exit (via ENTER) fires an event twice. If the OnEditingDone-Event is already fired in procedure TCustomComboBox.CloseUp, then the 2nd event via ENTER in my opinion is a bug.
@wp: do you agree?
In principle, yes, and if you want you can report it to bugtracker
I will create a bug report.

Quote
BTW: What do you think that no OnEditingDone event is created when the combobox style is csDropdownList and an item is picked from the dropdown using a mouse click? On the other hand, the event IS created when the item is selected with the ENTER key. Pretty much inconsistent in my eyes...
I agree. Maybe the problem is, that Delphi was a template to the LCL and if the original is inconsistent, the copy is difficult to be better.

For me (as an "advanced beginner" to the LCL) events are often difficult to handle. Normally I must do a lot of tries to find out, which event is the right one for my purpose and how to handle it.

Hartmut

  • Hero Member
  • *****
  • Posts: 739
Re: Event TComboBox.OnEditingDone fires to often (Focus problem?)
« Reply #24 on: July 15, 2018, 03:37:24 pm »
This is a summary about this Topic:
 - 3 problems with event TComboBox.OnEditingDone firing to often have been discussed
 - for every problem a solution was found
 - 2 bugs were identified

Problem #1 (introduced in the 1st post) was, that if you have a TPageControl with some TabSheets, that by clicking on the TabSheets the focus can be received wrongly by a TComboBox (if the ComboBox has TabOrder=0). If you then click on the next TabSheet, the focus leaves the ComboBox and event TComboBox.OnEditingDone is fired wrongly.
This is a bug. I created a bug report (#33991). This bug is not on Linux with Gtk2.

A solution came in reply #12 by setting a skip-event-variable in event PageControl1Change.

Problem #2 (introduced in the 1st post) was, that event TComboBox.OnEditingDone was always fired twice. The reason was my fault, because I "stealed" the focus from inside the TComboBox.OnEditingDone event at it's end. Therefore the ComboBox was exited twice and the event was fired twice.

A solution came in reply #4 by using a timer, if it is neccessary to switch the focus to another control.
Another solution came in reply # 21 by using an event-enable-flag, which is set in the TComboBox.OnEnter event.

Problem #3 (introduced in reply #14 as a variant of problem #2) was, that event TComboBox.OnEditingDone was fired twice, when you press ENTER in the ComboBox. wp found out in reply #20 why that is the case.
This is a bug too. I created a bug report (#33992).

A solution came in reply # 21 by using an event-enable-flag, which is set in the TComboBox.OnEnter event.

Thanks a lot again to wp and Handoko for their valuable help! I'm happy that all 3 problems found a good solution.
« Last Edit: July 15, 2018, 06:12:20 pm by Hartmut »

Ondrej Pokorny

  • Full Member
  • ***
  • Posts: 220
Problem #1 - not a bug, this is "as designed"
Problem #2 - not a bug, this is "as designed"
Problem #3 - yes, a bug but IMO it's not worth to fix it

---

I had the same idea recently - to fire OnEditingDone only if something changed and also refactor the OnEditingDone event in general (see r58337, r58339). Quickly I realized that it is not the way to go and I reverted my changes. I decided so by looking at how OnEditingDone is used in the IDE and the comments in the code about it. Another fact is that you cannot do this generally for every custom control.

OnEditingDone is an event that is fired when "editing state" finished. It doesn't mean that something actually has changed. Therefore if you use OnEditingDone you always have to track the value you use from the control (from TEdit is't Text, but from a custom control it can be anything) and execute your action only if the value changed.

If you do this (detect change) problem #3 isn't that bad.
« Last Edit: July 16, 2018, 07:16:36 am by Ondrej Pokorny »

Ondrej Pokorny

  • Full Member
  • ***
  • Posts: 220
Generally speaking OnEditingDone is a "helper" event without much priority that can be fired at various points (whenever the control decides) and doesn't guarantee that something actually changed. So it's not worth to pollute the LCL code with workarounds to fix issue #33992.

The user himself is responsible for change detection in OnEditingDone.

We should document the meaning of OnEditingDone better in http://lazarus-ccr.sourceforge.net/docs/lcl/controls/tcontrol.oneditingdone.html so that this confusion doesn't happen.

Hartmut

  • Hero Member
  • *****
  • Posts: 739
OnEditingDone is an event that is fired when "editing state" finished. It doesn't mean that something actually has changed. Therefore if you use OnEditingDone you always have to track the value you use from the control ... and execute your action only if the value changed.
If you do this (detect change) problem #3 isn't that bad.
In my case I need the TComboBox.OnEditingDone event also, if nothing was changed. That means, that the same actions have to take place, if someone enters or selects the same value than before e.g. from the ComboBox-History. So this (detect change) is not a solution for my problem #3. But I have a good solution (see reply #21), so my problem #3 is solved.

Generally speaking OnEditingDone ... doesn't guarantee that something actually changed. So it's not worth to pollute the LCL code with workarounds to fix issue #33992.
I understand that to fix issue #33992 is a difficult decision because of the risk to create new bugs. As wp wrote, this person must have very good knowledge of the LCL.

 

TinyPortal © 2005-2018