View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0032623 | Patches | LCL | public | 2017-10-29 09:00 | 2018-02-28 11:15 |
| Reporter | C Western | Assigned To | Juha Manninen | ||
| Priority | normal | Severity | minor | Reproducibility | random |
| Status | resolved | Resolution | fixed | ||
| Platform | i386 | OS | MACOSX | ||
| Product Version | 1.9 (SVN) | ||||
| Summary | 0032623: IDE crash on opening Tools, Options, User Defined Markup | ||||
| Description | On MacOSX/Carbon, Tools, Options, User Defined Markup crashes the IDE. Investigating this it seems the crash occurs in TCustomGrid.EditorPos because a bad value (from an unset variable) is used in FEditor.BoundsRect := CellR. I think the grid is empty at the time of the call, so CellR:=CellRect(FCol,FRow); simply does not set CellR. The attached patch is one possible workaround, forcing sane values. (An alternative aproach would be to act on the fail returns in ColRowToOffset.) | ||||
| Tags | No tags attached. | ||||
| Fixed in Revision | r56235 | ||||
| LazTarget | - | ||||
| Widgetset | Carbon | ||||
| Attached Files |
| ||||
| related to | 0033273 | closed | Juha Manninen | Lazarus | After revision 56235 a exception is called when a app with a DBGrid is closed |
|
|
grid.patch (667 bytes)
diff -uwNr --exclude=.svn --exclude=Makefile --exclude=Makefile.fpc --exclude=Makefile.compiled --exclude='*.rsj' --exclude='*.bak' --exclude='*.po' lazarus/lcl/grids.pas lazarus.w/lcl/grids.pas --- lazarus/lcl/grids.pas 2017-10-28 12:40:36.000000000 +0100 +++ lazarus.w/lcl/grids.pas 2017-10-28 19:21:39.000000000 +0100 @@ -3425,6 +3425,7 @@ function TCustomGrid.CellRect(ACol, ARow: Integer): TRect; begin //Result:=ColRowToClientCellRect(aCol,aRow); + Result := Rect(0,0,0,0); // Force sane values in case calls below fail ColRowToOffset(True, True, ACol, Result.Left, Result.Right); ColRowToOffSet(False,True, ARow, Result.Top, Result.Bottom); end; |
|
|
Widgetset = Carbon. However the patch is for LCL Grids code. The issue does not look like widgetset dependent.(?) |
|
|
I think it is indeed widgetset independent. (I selected Carbon because the crash only happens on a Mac for me.) |
|
|
ACol and ARow in CellRect must be out of range then. I think their validity should be checked explicitly and adjust the result accordingly. Another option is to add an Assert() for their validity and then make sure the method is not called with wrong values. It would be more robust. |
|
|
The root cause of the original crash may be that EditorPos shouldn't be called at all if there are no cells in the grid, but I am less sure about that. I attach an alternate patch that is more robust. |
|
|
grid.alt.patch (2,739 bytes)
diff -uwNr '--exclude=.svn' '--exclude=Makefile' '--exclude=Makefile.fpc' '--exclude=Makefile.compiled' '--exclude=*.rsj' '--exclude=*.bak' '--exclude=*.po' lazarus/lcl/grids.pas lazarus.w/lcl/grids.pas
--- lazarus/lcl/grids.pas 2017-09-25 08:48:02.487059888 +0100
+++ lazarus.w/lcl/grids.pas 2017-10-29 18:21:32.018523519 +0000
@@ -1258,6 +1258,7 @@
procedure AutoAdjustColumns; virtual;
procedure BeginUpdate;
function CellRect(ACol, ARow: Integer): TRect;
+ function CellRectValid(ACol, ARow: Integer; out ARect: TRect): Boolean;
function CellToGridZone(aCol,aRow: Integer): TGridZone;
procedure CheckPosition;
function ClearCols: Boolean;
@@ -3421,12 +3422,16 @@
end;
end;
-{ Returns a reactagle corresponding to a fisical cell[aCol,aRow] }
+{ Returns a reactagle corresponding to a physical cell[aCol,aRow] }
function TCustomGrid.CellRect(ACol, ARow: Integer): TRect;
begin
- //Result:=ColRowToClientCellRect(aCol,aRow);
- ColRowToOffset(True, True, ACol, Result.Left, Result.Right);
- ColRowToOffSet(False,True, ARow, Result.Top, Result.Bottom);
+ CellRectValid(ACol, ARow, Result);
+end;
+
+function TCustomGrid.CellRectValid(ACol, ARow: Integer; out ARect: TRect): Boolean;
+begin
+ Result := ColRowToOffset(True, True, ACol, ARect.Left, ARect.Right)
+ and ColRowToOffSet(False,True, ARow, ARect.Top, ARect.Bottom)
end;
// The visible grid Depends on TopLeft and ClientWidht,ClientHeight,
@@ -5291,7 +5296,7 @@
end;
end;
-function TCustomGrid.GetDefColWidth: integer;
+function TCustomGrid.GetDefColWidth: Integer;
begin
if FDefColWidth<0 then
begin
@@ -5302,7 +5307,7 @@
Result := FDefColWidth;
end;
-function TCustomGrid.GetDefRowHeight: integer;
+function TCustomGrid.GetDefRowHeight: Integer;
begin
if FDefRowHeight<0 then
begin
@@ -8207,6 +8220,7 @@
var
msg: TGridMessage;
CellR: TRect;
+ PosValid: Boolean;
begin
{$ifdef dbgGrid} DebugLn('Grid.EditorPos INIT');{$endif}
if HandleAllocated and (FEditor<>nil) then begin
@@ -8219,9 +8233,11 @@
FEditor.Dispatch(Msg);
// send editor bounds
- CellR:=CellRect(FCol,FRow);
+ PosValid := CellRectValid(FCol, FRow, CellR);
+ if not PosValid then // Can't position editor; ensure sane values
+ CellR := Rect(0,0,FEditor.Width, FEditor.Height);
- if (CellR.Top<FGCache.FixedHeight) or (CellR.Top>FGCache.ClientHeight) or
+ if not PosValid or (CellR.Top<FGCache.FixedHeight) or (CellR.Top>FGCache.ClientHeight) or
(UseRightToLeftAlignment and ((CellR.Right-1>FlipX(FGCache.FixedWidth)) or (CellR.Right<0))) or
(not UseRightToLeftAlignment and ((CellR.Left<FGCache.FixedWidth) or (CellR.Left>FGCache.ClientWidth)))
then
|
|
|
Thanks. I applied the patch with minor changes. I added an Assert() to CellRect and it did not trigger during my tests. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2017-10-29 09:00 | C Western | New Issue | |
| 2017-10-29 09:00 | C Western | File Added: grid.patch | |
| 2017-10-29 11:01 | Juha Manninen | LazTarget | => - |
| 2017-10-29 11:01 | Juha Manninen | Description Updated | View Revisions |
| 2017-10-29 11:03 | Juha Manninen | Note Added: 0103804 | |
| 2017-10-29 11:03 | Juha Manninen | Note Edited: 0103804 | View Revisions |
| 2017-10-29 11:07 | C Western | Note Added: 0103806 | |
| 2017-10-29 13:01 | Juha Manninen | Note Added: 0103807 | |
| 2017-10-29 20:58 | C Western | Note Added: 0103811 | |
| 2017-10-29 20:59 | C Western | File Added: grid.alt.patch | |
| 2017-10-29 22:11 | Juha Manninen | Assigned To | => Juha Manninen |
| 2017-10-29 22:11 | Juha Manninen | Status | new => assigned |
| 2017-10-29 22:22 | Juha Manninen | Fixed in Revision | => r56235 |
| 2017-10-29 22:22 | Juha Manninen | Note Added: 0103814 | |
| 2017-10-29 22:22 | Juha Manninen | Status | assigned => resolved |
| 2017-10-29 22:22 | Juha Manninen | Resolution | open => fixed |
| 2018-02-28 11:15 | Michl | Relationship added | related to 0033273 |
