View Issue Details

IDProjectCategoryView StatusLast Update
0035458LazarusLCLpublic2019-12-19 17:57
ReporterZebu1erAssigned ToJuha Manninen 
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
PlatformIntelOSWindowsOS Version7 (NT6.1) x64
Product Version2.0.2Product Buildr60954 
Target VersionFixed in Version 
Summary0035458: Some data controls regressions since 1.8.x
DescriptionTDBLookupListBox : The selection disapears after selecting an item, but the focus remains on the item. Selecting with keybord or mouse does the same

TDBLookupComboBox : The progressive search fails, each character typed in replaces the previous selection.
Steps To ReproduceTDBLookupListBox : ListSource is set to a TDataSource

TDBLookupComboBox : AutoComplete property is set to true, the ListSource is set to a TDataSource
TagsNo tags attached.
Fixed in Revisionr61111
LazTarget-
WidgetsetWin32/Win64
Attached Files
  • BugTest.zip (128,885 bytes)
  • BugTest2.zip (128,754 bytes)
  • dblookuplistbox-unb.patch (1,566 bytes)
    Index: lcl/dbctrls.pp
    ===================================================================
    --- lcl/dbctrls.pp	(revision 61108)
    +++ lcl/dbctrls.pp	(working copy)
    @@ -473,6 +473,7 @@
         procedure KeyDown(var Key: Word; Shift: TShiftState); override;
         procedure Loaded; override;
         procedure UpdateData(Sender: TObject); override;
    +    function IsUnbound: boolean;
       public
         constructor Create(AOwner: TComponent); override;
         property KeyValue: Variant read GetKeyValue write SetKeyValue;
    Index: lcl/include/dblookuplistbox.inc
    ===================================================================
    --- lcl/include/dblookuplistbox.inc	(revision 61108)
    +++ lcl/include/dblookuplistbox.inc	(working copy)
    @@ -30,6 +30,11 @@
       FLookup.UpdateData(ItemIndex, FScrollListDataset);
     end;
     
    +function TDBLookupListBox.IsUnbound: boolean;
    +begin
    +  Result := (FDataLink.DataSource = nil) or (DataField = '');
    +end;
    +
     procedure TDBLookupListBox.ActiveChange(Sender: TObject);
     begin
       if FDataLink.Active then
    @@ -54,15 +59,18 @@
     procedure TDBLookupListBox.DoSelectionChange(User: Boolean);
     begin
       if User then
    -  begin
    -    if FDataLink.CanModify then
    +    if IsUnbound then
    +      UpdateData(Self)
    +    else
         begin
    -      FDataLink.Modified;
    -      FDataLink.UpdateRecord;
    -    end
    -    else
    -      DataChange(Self);
    -  end;
    +      if FDataLink.CanModify then
    +      begin
    +        FDataLink.Modified;
    +        FDataLink.UpdateRecord;
    +      end
    +      else
    +        DataChange(Self);
    +    end;
       inherited DoSelectionChange(User);
     end;
     
    

Relationships

related to 0034298 assignedJesus Reyes Autocomplete in DBLookupComboBox 
related to 0032408 closedJuha Manninen Lookup controls (TDBLookupComboBox, TDBLookupListBox) - ReadOnly property is ignored 
related to 0035069 resolvedJuha Manninen TDBLookupListBox. Items are not selectable (with mouse/keyboard) 

Activities

Zebu1er

2019-04-27 19:39

reporter  

BugTest.zip (128,885 bytes)

Juha Manninen

2019-04-30 14:50

developer   ~0115922

Can you please check which revision caused the regression.
 http://wiki.freepascal.org/How_do_I_create_a_bug_report#Regression_caused_by_a_certain_revision

Zebu1er

2019-04-30 17:36

reporter   ~0115925

I didn't install Lazarus from SVN. Both Lazarus v2.0.0 and 2.0.2 are affected. Version 1.8.4 wasn't affected.

Juha Manninen

2019-04-30 18:06

developer   ~0115928

> I didn't install Lazarus from SVN.

Why not? It is easy.

Zebu1er

2019-04-30 18:51

reporter   ~0115929

I installed the trunk version my own way, that is extracting SVN to a directory where I added fpc et mingw subdirectories from release 2.0.2 install... and I start lazarus.exe with --pcp=... option. It shows 2.1.0 in the title bar (no revision number included). The bug is still there !
No idea how to process : I think I gonna try to extract the couple of file that I suspect back to rev. 57972 (I think this is the one of version 1.8.4) and switch forth to actual trunck, file by file, and see when it fails...

Zebu1er

2019-04-30 19:40

reporter   ~0115930

TDBLookupListBox : As of rev. 55911 (made by a certain Juha :-P) DataChange as been called from DoSelectionChange
 when FDataLink.CanModify is false. DataChange forces the ItemIndex to -1 when FDatalink.Active is false
. That's probably the reason that makes the item deselect. No idea what's the right to change...

Zebu1er

2019-05-01 11:47

reporter   ~0115940

TDBLookupComboBox (lcl/include/dblookupcombobox.inc) : Over less than one year, some modifications of mainly jesus, triggered UpdateLookup during progressive search. When UnBound (either or both DataSource or DataField not set) function result is True, UpdateData is called, which calls UpdateLookup ... and then the acual typed text is replaced.
No idea of what to do to solve it !!!

mavika

2019-05-02 12:41

reporter   ~0115950

Probably duplicate? https://bugs.freepascal.org/view.php?id=34298#c115744

@Juha Manninen:
I tested it with version SVN 61105 also, and the problem is there.

Juha Manninen

2019-05-02 15:17

developer   ~0115955

> TDBLookupListBox : As of rev. 55911 (made by a certain Juha :-P) ...
Zebu1er, should I revert the change regarding TDBLookupListBox? The change was:

@@ -55,8 +55,13 @@ procedure TDBLookupListBox.DoSelectionChange(User: Boolean);
 begin
   if User then
   begin
- FDataLink.Modified;
- FDataLink.UpdateRecord;
+ if FDataLink.CanModify then
+ begin
+ FDataLink.Modified;
+ FDataLink.UpdateRecord;
+ end
+ else
+ DataChange(Self);
   end;
   inherited DoSelectionChange(User);
 end;

Michal Gawrycki

2019-05-02 15:49

reporter   ~0115958

Attached project is not valid because dataset for data is missing. There is only a dataset for lookup display. I attach the correct project.

BugTest2.zip (128,754 bytes)

Zebu1er

2019-05-02 17:42

reporter   ~0115962

Last edited: 2019-05-02 17:43

View 2 revisions

Juha Manninen : First of all : Is that true that DataSet has necessarily to be set, as pretends Michal Gawrycki ?

Michal Gawrycki

2019-05-02 18:49

reporter   ~0115963

I'm sorry, it looks like I made a mistake. I checked and Delphi allows this action. I will try to prepare the patch.

Zebu1er

2019-05-03 10:42

reporter   ~0115970

Juha Manninen : Michal Gawrycki confirmed the use case of no DataSource/DataField set.
That is, I can answer your question about TDBLookupListBox.
I don't think your changes need to be reversed by principle, however I don't know about the reason you did it.
We could introduce an IsUnbound function just like for TDBLookupComboBox. So then, using that function to determine if DataChange should be called. Or inside DataChange, using IsUnbound to determine how to act.
I'm not fixed on the right way, maybe you are ?
I won't be available since monday. Then I'l be able to propose a patch if you want ?

Michal Gawrycki

2019-05-03 13:35

reporter   ~0115971

> I don't think your changes need to be reversed by principle, however I don't know about the reason you did it.

The reason was the attempt to assign values to the read-only field, which resulted an exception.

> We could introduce an IsUnbound function just like for TDBLookupComboBox. So then, using that function to determine if DataChange should be called. Or inside DataChange, using IsUnbound to determine how to act.

Yes it's a good idea. We just need to call UpdateData instead of DataChange.

I attached a patch.

dblookuplistbox-unb.patch (1,566 bytes)
Index: lcl/dbctrls.pp
===================================================================
--- lcl/dbctrls.pp	(revision 61108)
+++ lcl/dbctrls.pp	(working copy)
@@ -473,6 +473,7 @@
     procedure KeyDown(var Key: Word; Shift: TShiftState); override;
     procedure Loaded; override;
     procedure UpdateData(Sender: TObject); override;
+    function IsUnbound: boolean;
   public
     constructor Create(AOwner: TComponent); override;
     property KeyValue: Variant read GetKeyValue write SetKeyValue;
Index: lcl/include/dblookuplistbox.inc
===================================================================
--- lcl/include/dblookuplistbox.inc	(revision 61108)
+++ lcl/include/dblookuplistbox.inc	(working copy)
@@ -30,6 +30,11 @@
   FLookup.UpdateData(ItemIndex, FScrollListDataset);
 end;
 
+function TDBLookupListBox.IsUnbound: boolean;
+begin
+  Result := (FDataLink.DataSource = nil) or (DataField = '');
+end;
+
 procedure TDBLookupListBox.ActiveChange(Sender: TObject);
 begin
   if FDataLink.Active then
@@ -54,15 +59,18 @@
 procedure TDBLookupListBox.DoSelectionChange(User: Boolean);
 begin
   if User then
-  begin
-    if FDataLink.CanModify then
+    if IsUnbound then
+      UpdateData(Self)
+    else
     begin
-      FDataLink.Modified;
-      FDataLink.UpdateRecord;
-    end
-    else
-      DataChange(Self);
-  end;
+      if FDataLink.CanModify then
+      begin
+        FDataLink.Modified;
+        FDataLink.UpdateRecord;
+      end
+      else
+        DataChange(Self);
+    end;
   inherited DoSelectionChange(User);
 end;
 

Juha Manninen

2019-05-03 15:58

developer   ~0115972

Last edited: 2019-05-03 16:00

View 2 revisions

I applied the patch in r61111. Thanks.
I am not an expert with data aware controls although I earlier applied the patch in 0032408. Then it looked OK and did not break anything.
My Linux test environment may behave slightly differently than your Windows system. Now I must trust you guys for testing this feature. If it works then I will resolve the issue. If there are problems that need expertize, I will assign this to Jesus Reyes.

How about the related issue 0034298? Does it get solved together with this one? Is it clearly a duplicate?

Michal Gawrycki

2019-05-03 17:46

reporter   ~0115978

I think it is still not implemented correctly. IMHO the only correct solution for unbounded lookup controls is to explicitly indicate active record in lookp dataset. We just assume that active record of lookup dataset is always selected in lookup control. It should also react to changing active record in the lookup dataset. Otherwise, the selected element in lookup control will not match the active record in lookup dataset.
For example, when you run a test project, no element in the lookuplistbox is selected, but dataset points to a record that does not reflect actual state of lookup control.

This requires much more work, especially in the case of TDBLookupComboBox. I'm not sure if it's worth the effort. After all, these are data-aware controls. I suggested that delphi has similar functionality but has the same problems with inconsistency.

Juha Manninen

2019-05-11 10:07

developer   ~0116127

Assigning to Jesus.

wp

2019-12-16 11:18

developer   ~0119878

Reminder sent to: Jesus Reyes

Hi Jesus, is it ok to "resolve" this issue? To me it seems so. Forum users have been reporting the same issue occasionally.

Juha Manninen

2019-12-19 17:57

developer   ~0119959

Resolving. r61111 looks good to me and wp at least.

Issue History

Date Modified Username Field Change
2019-04-27 19:08 Zebu1er New Issue
2019-04-27 19:39 Zebu1er File Added: BugTest.zip
2019-04-30 14:50 Juha Manninen Note Added: 0115922
2019-04-30 17:36 Zebu1er Note Added: 0115925
2019-04-30 18:06 Juha Manninen Note Added: 0115928
2019-04-30 18:51 Zebu1er Note Added: 0115929
2019-04-30 19:40 Zebu1er Note Added: 0115930
2019-05-01 11:47 Zebu1er Note Added: 0115940
2019-05-02 12:41 mavika Note Added: 0115950
2019-05-02 15:09 Juha Manninen Relationship added related to 0034298
2019-05-02 15:11 Juha Manninen Relationship added related to 0032408
2019-05-02 15:17 Juha Manninen Note Added: 0115955
2019-05-02 15:49 Michal Gawrycki File Added: BugTest2.zip
2019-05-02 15:49 Michal Gawrycki Note Added: 0115958
2019-05-02 17:42 Zebu1er Note Added: 0115962
2019-05-02 17:43 Zebu1er Note Edited: 0115962 View Revisions
2019-05-02 18:49 Michal Gawrycki Note Added: 0115963
2019-05-03 10:42 Zebu1er Note Added: 0115970
2019-05-03 13:35 Michal Gawrycki File Added: dblookuplistbox-unb.patch
2019-05-03 13:35 Michal Gawrycki Note Added: 0115971
2019-05-03 15:43 Juha Manninen Assigned To => Juha Manninen
2019-05-03 15:43 Juha Manninen Status new => assigned
2019-05-03 15:58 Juha Manninen Note Added: 0115972
2019-05-03 16:00 Juha Manninen Note Edited: 0115972 View Revisions
2019-05-03 16:00 Juha Manninen Status assigned => feedback
2019-05-03 16:00 Juha Manninen LazTarget => -
2019-05-03 17:46 Michal Gawrycki Note Added: 0115978
2019-05-11 10:07 Juha Manninen Assigned To Juha Manninen => Jesus Reyes
2019-05-11 10:07 Juha Manninen Status feedback => assigned
2019-05-11 10:07 Juha Manninen Note Added: 0116127
2019-12-16 11:08 wp Relationship added related to 0035069
2019-12-16 11:18 wp Note Added: 0119878
2019-12-19 17:57 Juha Manninen Assigned To Jesus Reyes => Juha Manninen
2019-12-19 17:57 Juha Manninen Status assigned => resolved
2019-12-19 17:57 Juha Manninen Resolution open => fixed
2019-12-19 17:57 Juha Manninen Fixed in Revision => r61111
2019-12-19 17:57 Juha Manninen Widgetset Win32/Win64 => Win32/Win64
2019-12-19 17:57 Juha Manninen Note Added: 0119959