View Issue Details

IDProjectCategoryView StatusLast Update
0026189LazarusLCLpublic2014-10-03 18:19
Reportertintinux Assigned ToBart Broersma  
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
PlatformWindows OSWindows 
Product Version1.2.2 
Target Version1.2.6 
Summary0026189: After deleting a row in a StringGrid, AutoAdd rows is not working
DescriptionHi

I have found a small issue in Lazarus 1.2.2 with StringGrid.

This occurs with Options goAutoAddRows and goAlwaysShowEditor, and can be reproduced with the attached example.

1) Press Keydown 4 times. A new empty row is created : OK (except OnSelectCell is called twice, not an important issue).
2) Click button "Delete line". The row is deleted and the previous line is selected : OK.
3) Press Keydown again, no arrow key is efficient, no row is created : KO

It is necessary to use the mouse to click in another cell, to reactivate Keys.

Regards

Tintinux
Tagspatch
Fixed in Revisionr46414
LazTarget1.2.6
WidgetsetWin32/Win64
Attached Files

Activities

tintinux

2014-05-19 09:43

reporter  

StringGrid_delete.zip (1,925 bytes)

tintinux

2014-05-21 13:48

reporter  

tintinux

2014-05-21 13:56

reporter   ~0075163

Please download the second attachment (StringGrid_delete_2.zip).
The first one was missing a SetFocus which could explain the issue.

---

If you add the following steps, the issue is more important :

4) Click in the first column of the last Row
5) Press KeyUp : the last row is deleted and shouldn't

Bart Broersma

2014-09-07 21:00

developer   ~0076966

.lpi is missing, so I cannot test.

Mike Thompson

2014-09-15 16:00

developer  

Mike Thompson

2014-09-15 16:02

developer   ~0077259

I've uploaded StringGrid_Delete_3.zip, which contains a .lpi, and confirmed that the issue repeats under Win7/Lazarus 1.3 SVN 45863 (I know it's stale, but it's all I've got access to for now)

Mike Thompson

2014-09-15 16:28

developer   ~0077260

Last edited: 2014-09-17 21:55

View 3 revisions

Making this harder to debug was the following line in the attached example:
  StringGrid1.Options := StringGrid1.Options - [goAutoAddRows];
I recommend commenting that out during debugging.

The issue is that FAutoRowInserted in TCustomGrid is not being cleared if the outside code calls DeleteColRow on the newly inserted Row at a time when its status is still only temporary.

I'm not in a position to generate an uptodate DIFF file, or to test against latest trunk. I will be in a few weeks time, so will revisit this then. In the meantime, I'm leaving this here as a note to myself (or others if they're interested)

In grids.pp, I can fix this by adding the following code
procedure TCustomGrid.DoOPDeleteColRow(IsColumn: Boolean; index: Integer);
  procedure doDeleteRow;
{add to last line in doDeleteRow}
    If FRowAutoInserted And (Index=FixedRows+(RowCount-1)) Then
      FRowAutoInserted := False;

In all my testing to date, DoOPDeleteColRow is only called when TStringGrid.DeleteColRow is called.

As this change is in TCustomGrid, I know this will need testing on all descended grids. To date I've only tested on TStringGrid.

Mike Thompson

2014-09-28 21:59

developer  

grids.pas.patch (436 bytes)   
Index: lcl/grids.pas
===================================================================
--- lcl/grids.pas	(revision 46342)
+++ lcl/grids.pas	(working copy)
@@ -5916,6 +5916,9 @@
     FGCache.AccumHeight.Delete(Index);
     ColRowDeleted(False,Index);
     FixPosition(False, Index);
+
+    If FRowAutoInserted And (Index=FixedRows+(RowCount-1)) Then
+      FRowAutoInserted := False;
   end;
 begin
   CheckIndex(IsColumn,Index);
grids.pas.patch (436 bytes)   

Mike Thompson

2014-09-28 21:59

developer  

GridsTestHarness.zip (3,823 bytes)

Mike Thompson

2014-09-28 22:05

developer   ~0077785

Last edited: 2014-09-28 22:08

View 2 revisions

Attached patch "grids.pas.patch" resolves this issue.

Attached Test Project "GridsTestHarness.zip" has a TStringGrid, TDrawGrid & TDBGrid which are all populated at runtime.

Patch works fine with TStringGrid and doesn't break TDBGrid (TDBGrid uses a different mechanism for automatically adding rows anyway).

I couldn't get goAutoAddRows to work with TDrawGrid, so couldn't test if this patch worked for TDrawGrid. But I now don't think TDrawGrid supports goAutoAddRows.

(There's an exception raised at close of the testharness if you add a record to the TDBGrid, but this is due to the TSdfDataset, not the grid - I can't work out how to rollback the add operation).

Bart Broersma

2014-09-28 23:37

developer   ~0077789

I'm currently busy with other things, but I'll look into it when I have the time.

Bart Broersma

2014-10-03 18:13

developer   ~0077973

Applied, thanks for the patch.
Please test and close if OK.

Issue History

Date Modified Username Field Change
2014-05-19 09:43 tintinux New Issue
2014-05-19 09:43 tintinux File Added: StringGrid_delete.zip
2014-05-20 08:37 Jesus Reyes Assigned To => Jesus Reyes
2014-05-20 08:37 Jesus Reyes Status new => assigned
2014-05-21 13:48 tintinux File Added: StringGrid_delete_2.zip
2014-05-21 13:56 tintinux Note Added: 0075163
2014-09-07 20:00 Jesus Reyes LazTarget => 1.4
2014-09-07 20:00 Jesus Reyes Assigned To Jesus Reyes => Bart Broersma
2014-09-07 20:00 Jesus Reyes Target Version => 1.4
2014-09-07 21:00 Bart Broersma Note Added: 0076966
2014-09-07 21:00 Bart Broersma Status assigned => feedback
2014-09-15 16:00 Mike Thompson File Added: StringGrid_Delete_3.zip
2014-09-15 16:02 Mike Thompson Note Added: 0077259
2014-09-15 16:28 Mike Thompson Note Added: 0077260
2014-09-15 16:29 Mike Thompson Note Edited: 0077260 View Revisions
2014-09-17 21:55 Mike Thompson Note Edited: 0077260 View Revisions
2014-09-28 21:59 Mike Thompson File Added: grids.pas.patch
2014-09-28 21:59 Mike Thompson File Added: GridsTestHarness.zip
2014-09-28 22:05 Mike Thompson Note Added: 0077785
2014-09-28 22:05 Mike Thompson Tag Attached: patch
2014-09-28 22:08 Mike Thompson Note Edited: 0077785 View Revisions
2014-09-28 23:37 Bart Broersma Note Added: 0077789
2014-10-03 18:13 Bart Broersma Fixed in Revision => r46414
2014-10-03 18:13 Bart Broersma LazTarget 1.4 => 1.2.6
2014-10-03 18:13 Bart Broersma Note Added: 0077973
2014-10-03 18:13 Bart Broersma Status feedback => resolved
2014-10-03 18:13 Bart Broersma Resolution open => fixed
2014-10-03 18:13 Bart Broersma Target Version 1.4 => 1.2.6