View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0032463 | Lazarus | LCL | public | 2017-09-24 11:05 | 2018-02-12 16:58 |
| Reporter | Michal Gawrycki | Assigned To | wp | ||
| Priority | normal | Severity | minor | Reproducibility | have not tried |
| Status | closed | Resolution | fixed | ||
| Product Version | 1.9 (SVN) | ||||
| Summary | 0032463: Regression - TDBLookupComboBox raise exception | ||||
| Description | Revision r55894 causes regression in TDBLookupComboBox. Value of combo text is assigned to data field in "TCustomComboBox.Change" function which is incorrect in case of DBLookupComboBox. I suggest moving field data assignment to "TDBComboBox.Change". | ||||
| Tags | No tags attached. | ||||
| Fixed in Revision | r56884, r56948 | ||||
| LazTarget | 1.8.2 | ||||
| Widgetset | |||||
| Attached Files |
| ||||
| related to | 0032383 | resolved | Jesus Reyes | TDbComboBox has problems to set data when item changes |
| related to | 0030587 | closed | Luiz Americo | TDBComboBox – field data is not updated if combo is not focused but element is changed by mouse wheel |
| related to | 0032943 | closed | Jesus Reyes | TDBLookupComboBox and csDropDown - restore correct ItemIndex |
| related to | 0033160 | closed | wp | Selected item in TDBComboBox when TDataSet are browse |
|
|
dbcombobox-change.patch (1,612 bytes)
Index: lcl/dbctrls.pp
===================================================================
--- lcl/dbctrls.pp (revision 55911)
+++ lcl/dbctrls.pp (working copy)
@@ -707,6 +707,7 @@
TDBComboBox = class(TCustomDBComboBox)
protected
+ procedure Change; override;
procedure DataChange(Sender: TObject); override;
procedure KeyDown(var Key: Word; Shift: TShiftState); override;
procedure KeyPress(var Key: char); override;
Index: lcl/include/customdbcombobox.inc
===================================================================
--- lcl/include/customdbcombobox.inc (revision 55911)
+++ lcl/include/customdbcombobox.inc (working copy)
@@ -34,15 +34,9 @@
procedure TCustomDBComboBox.Change;
begin
- FDataLink.Modified;
- try
- if FDataLink.CanModify then begin
- FDataLink.Field.AsString := Text;
- FDatalink.Modified;
- end;
- finally
- inherited Change;
- end;
+ if FDataLink.CanModify then
+ FDataLink.Modified;
+ inherited Change;
end;
function TCustomDBComboBox.GetReadOnly: Boolean;
Index: lcl/include/dbcombobox.inc
===================================================================
--- lcl/include/dbcombobox.inc (revision 55911)
+++ lcl/include/dbcombobox.inc (working copy)
@@ -17,6 +17,18 @@
FDataLink.Field.Text := Text;
end;
+procedure TDBComboBox.Change;
+begin
+ try
+ if FDataLink.CanModify then begin
+ FDataLink.Field.AsString := Text;
+ FDatalink.Modified;
+ end;
+ finally
+ inherited Change;
+ end;
+end;
+
procedure TDBComboBox.DataChange(Sender: TObject);
var
DataLinkField: TField;
|
|
|
Ok right. Thanks for the patch. Applied. |
|
|
This patch should be reverted because it crashes the DBCombobox: Load the demo attached to issue 0030587, click into the DBCombobox, and turn the mouse wheel --> crash. The crash happens in line FDataLink.Field.AsString := Text; of the newly inserted Change method, because the dataset is not yet in edit mode. Correct behavior could be achieved by checking FDatalink.Editing: procedure TDBComboBox.Change; begin try if FDataLink.CanModify and FDataLink.Editing then begin FDataLink.Field.AsString := Text; FDatalink.Modified; end; finally inherited Change; end; end; But this gives back the erroneous behavior which was fixed in issue 0030587. I found good behavior if both r55912 (this issue) and r55894 (issue 0032383) are reverted and TDBCombobox.Select (modified for issue 0030587) is changed to procedure TDBComboBox.Select; begin //avoid reseting text when calling select FDataLink.OnDataChange := nil; try if FDataLink.Edit then begin FDataLink.Modified; // <-- added FDataLink.UpdateData; inherited Select; end else begin // if cannot modify, let it reset FDatalink.Reset; DataChange(Self); end; finally FDataLink.OnDataChange := @DataChange; end; end; However, I am not sure if this is the correct solution because the IsModified flag should already have been set when the routine is entered. Unfortunately, no demo project has been added to issue 0032383. |
|
|
wp, please use your judgement to fix the issue. You know it better. |
|
|
Michael, I had to undo the patch partly because it crashes the combobox as described above (r56883, r56884). Please check whether the issue that you reported still exists. |
|
|
I can confirm that the reported problem does not occur after your changes. I found another one about DBLookupComboBox, but I'm not sure if it's caused by your changes so I'll check it and open a new issue if needed. |
|
|
Thank you. Before resolving this issue I'd ask you to check DBLookupCombobox. In r56948 I use the same code in TDBLookupCombobox.Select as in TDBCombobox.Select. Please check if you still see issues in DBLookupCombobox. For the future I'd ask you to always add sample projects to your reports which show the error. This little projects are a big help for the reviewer to test the patch and the effect of other related patches. |
|
|
My issue concerned that TDBLookupComboBox set value of "Text" property to the data field, which is incorrect in case of lookup contol (because there should be a key value from lookup table nor display text). So in this case it works ok (r56954). I usually attach an example project. But in this case the matter seemed obvious to me, because if I entered anything that was not on the lookup list in TDBLookupComboBox, I was getting an exception. |
|
|
Thank you for testing DBLookupCombobox. Please don't assume that anything is obvious. If it is obvious to you it may not be obvious to the reviewer because he maybe is used to work with the control in a different way. And even if it really were "obvious" then the demo would help another reviewer who comes back to this issue because of another issue (like me). With a sample project I know without going into details of the current report: this demo should not give an error any more if that other issue is fixed correctly. I am resolving this now as being fixed. Your turn to close it. Thank you again. |
|
|
Thank you for help. I close this report. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2017-09-24 11:05 | Michal Gawrycki | New Issue | |
| 2017-09-24 11:05 | Michal Gawrycki | File Added: dbcombobox-change.patch | |
| 2017-09-24 12:16 | Juha Manninen | Assigned To | => Juha Manninen |
| 2017-09-24 12:16 | Juha Manninen | Status | new => assigned |
| 2017-09-24 12:17 | Juha Manninen | Relationship added | related to 0032383 |
| 2017-09-24 12:26 | Juha Manninen | Fixed in Revision | => r55912 |
| 2017-09-24 12:26 | Juha Manninen | LazTarget | => - |
| 2017-09-24 12:26 | Juha Manninen | Note Added: 0103014 | |
| 2017-09-24 12:26 | Juha Manninen | Status | assigned => resolved |
| 2017-09-24 12:26 | Juha Manninen | Resolution | open => fixed |
| 2017-12-29 22:51 | wp | Note Added: 0105124 | |
| 2017-12-29 22:51 | wp | Status | resolved => assigned |
| 2017-12-29 22:51 | wp | Resolution | fixed => reopened |
| 2017-12-29 22:51 | wp | Relationship added | related to 0030587 |
| 2017-12-29 22:52 | wp | Note Edited: 0105124 | View Revisions |
| 2017-12-29 23:01 | wp | Note Edited: 0105124 | View Revisions |
| 2017-12-30 09:21 | wp | Note Edited: 0105124 | View Revisions |
| 2017-12-30 09:22 | Juha Manninen | Assigned To | Juha Manninen => wp |
| 2017-12-30 09:23 | Juha Manninen | Note Added: 0105134 | |
| 2017-12-30 20:59 | wp | Note Added: 0105166 | |
| 2017-12-30 20:59 | wp | Status | assigned => feedback |
| 2017-12-30 22:59 | wp | Note Edited: 0105166 | View Revisions |
| 2018-01-03 23:07 | Michal Gawrycki | Note Added: 0105303 | |
| 2018-01-03 23:07 | Michal Gawrycki | Status | feedback => assigned |
| 2018-01-04 08:03 | wp | Note Added: 0105312 | |
| 2018-01-04 08:03 | wp | Status | assigned => feedback |
| 2018-01-04 16:40 | Michal Gawrycki | Note Added: 0105326 | |
| 2018-01-04 16:40 | Michal Gawrycki | Status | feedback => assigned |
| 2018-01-04 17:09 | wp | Note Added: 0105330 | |
| 2018-01-04 17:11 | wp | Fixed in Revision | r55912 => r56884, r56948 |
| 2018-01-04 17:11 | wp | LazTarget | - => 1.8.2 |
| 2018-01-04 17:11 | wp | Status | assigned => resolved |
| 2018-01-04 17:11 | wp | Resolution | reopened => fixed |
| 2018-01-04 18:33 | Michal Gawrycki | Note Added: 0105336 | |
| 2018-01-04 18:33 | Michal Gawrycki | Status | resolved => closed |
| 2018-01-10 17:46 | Juha Manninen | Relationship added | related to 0032943 |
| 2018-02-12 16:58 | wp | Relationship added | related to 0033160 |
