CPTableView Drag and drop in void causes a crash #1857

Closed
primalmotion opened this Issue Mar 14, 2013 · 10 comments

Comments

Projects
None yet
4 participants
@primalmotion
Member

primalmotion commented Mar 14, 2013

Context:
CPTableView with a few lines
Frame sufficiently big to see blank lines
Start to drag and drop from the void
--> crash coming from _updateSelectionWithMouseAtRow:

Uncaught CPInvalidArgumentException: Range {-1, 8} is out of bounds.

Visible in Tests/Manual/TableView/DataView (or anywhere else actually as soon as you implement the drag and drop methods, or you allow multiple selection)

Discussed with @cacaodev

@primalmotion

This comment has been minimized.

Show comment Hide comment
@primalmotion

primalmotion Mar 14, 2013

Member

+#accepted

Member

primalmotion commented Mar 14, 2013

+#accepted

@primalmotion

This comment has been minimized.

Show comment Hide comment
@primalmotion

primalmotion Mar 14, 2013

Member

I'll add that you need to actually end the drag on an existing row

Member

primalmotion commented Mar 14, 2013

I'll add that you need to actually end the drag on an existing row

@cappbot

This comment has been minimized.

Show comment Hide comment
@cappbot

cappbot Mar 17, 2013

Milestone: Someday. Label: #accepted. What's next? A reviewer should examine this issue.

cappbot commented Mar 17, 2013

Milestone: Someday. Label: #accepted. What's next? A reviewer should examine this issue.

@CyrilPeponnet

This comment has been minimized.

Show comment Hide comment
@CyrilPeponnet

CyrilPeponnet Mar 29, 2013

Could be related :
When using CPMenu and delegate on Outlineview/tableview, a right click on a void section lead to a crash with same exception : Uncaught CPInvalidArgumentException: Range {-1, 1} is out of bounds.

Could be related :
When using CPMenu and delegate on Outlineview/tableview, a right click on a void section lead to a crash with same exception : Uncaught CPInvalidArgumentException: Range {-1, 1} is out of bounds.

@CyrilPeponnet

This comment has been minimized.

Show comment Hide comment
@CyrilPeponnet

CyrilPeponnet Apr 10, 2013

More details for right click, it's when you implement in the delegate :

var itemRow = [aTableView rowAtPoint:aRow];
if ([aTableView selectedRow] != aRow)
[aTableView selectRowIndexes:[CPIndexSet indexSetWithIndex:aRow] byExtendingSelection:NO];

So, you have an item selected, and you right click on a void row -> crash as above.

More details for right click, it's when you implement in the delegate :

var itemRow = [aTableView rowAtPoint:aRow];
if ([aTableView selectedRow] != aRow)
[aTableView selectRowIndexes:[CPIndexSet indexSetWithIndex:aRow] byExtendingSelection:NO];

So, you have an item selected, and you right click on a void row -> crash as above.

@CyrilPeponnet

This comment has been minimized.

Show comment Hide comment
@CyrilPeponnet

CyrilPeponnet Apr 23, 2013

Let me bump this one ! Thanks

Let me bump this one ! Thanks

@CyrilPeponnet

This comment has been minimized.

Show comment Hide comment
@CyrilPeponnet

CyrilPeponnet Apr 25, 2013

@cacaodev, in CPTableView.j

 4523     else if (_allowsMultipleSelection)
 4524     {
 4525         newSelection = [CPIndexSet indexSetWithIndexesInRange:CPMakeRange(MIN(aRow, _selectionAnchorRow), ABS(aRow - _selectionAnchorRow) + 1)];
 4526         shouldExtendSelection = [self mouseDownFlags] & CPShiftKeyMask &&
 4527                                 ((_lastSelectedRow == [_selectedRowIndexes lastIndex] && aRow > _lastSelectedRow) ||
 4528                                 (_lastSelectedRow == [_selectedRowIndexes firstIndex] && aRow < _lastSelectedRow));
 4529     }

when crashing _selectionAnchorRow = -1, so the calculated range is not valid.

Going deeper :


 - (BOOL)startTrackingAt:(CGPoint)aPoint
 4035 {
 4036     // Try to become the first responder, but if we can't, that's okay.
 4037     [[self window] makeFirstResponder:self];
 4038 
 4039     var row = [self rowAtPoint:aPoint];
 4040 
 4041     // If the user clicks outside a row then deselect everything.
 4042     if (row < 0 && _allowsEmptySelection)
 4043         [self selectRowIndexes:[CPIndexSet indexSet] byExtendingSelection:NO];
 4044 
 4045     [self _noteSelectionIsChanging];
 4046 
 4047     if ([self mouseDownFlags] & CPShiftKeyMask)
 4048         _selectionAnchorRow = (ABS([_selectedRowIndexes firstIndex] - row) < ABS([_selectedRowIndexes lastIndex] - row)) ?
 4049             [_selectedRowIndexes firstIndex] : [_selectedRowIndexes lastIndex];
 4050     else
 4051         _selectionAnchorRow = row;

row = [self rowAtPoint:aPoint] when we are below the last item return -1.

Going deeper :

 1799 - (CPInteger)rowAtPoint:(CGPoint)aPoint
 1800 {
 1801  if (_implementedDelegateMethods & CPTableViewDelegate_tableView_heightOfRow_)
 1802  {
 1803  return [_cachedRowHeights indexOfObject:aPoint
 1804  inSortedRange:nil
 1805  options:0
 1806  usingComparator:function(aPoint, rowCache)
 1807  {
 1808  var upperBound = rowCache.heightAboveRow;
 1809 
 1810  if (aPoint.y < upperBound)
 1811  return CPOrderedAscending;
 1812 
 1813  if (aPoint.y > upperBound + rowCache.height + _intercellSpacing.height)
 1814  return CPOrderedDescending;
 1815 
 1816  return CPOrderedSame;
 1817  }];
 1818  }
 1819 
 1820  var y = aPoint.y,
 1821  row = FLOOR(y / (_rowHeight + _intercellSpacing.height));
 1822 
 1823  if (row >= _numberOfRows)
 1824  return CPNotFound;
 1825 
 1826  return row;
 1827 }

return CPNotFound value is "-1" when we row > _numberOfRows. I think I've found the issue.

@cacaodev, in CPTableView.j

 4523     else if (_allowsMultipleSelection)
 4524     {
 4525         newSelection = [CPIndexSet indexSetWithIndexesInRange:CPMakeRange(MIN(aRow, _selectionAnchorRow), ABS(aRow - _selectionAnchorRow) + 1)];
 4526         shouldExtendSelection = [self mouseDownFlags] & CPShiftKeyMask &&
 4527                                 ((_lastSelectedRow == [_selectedRowIndexes lastIndex] && aRow > _lastSelectedRow) ||
 4528                                 (_lastSelectedRow == [_selectedRowIndexes firstIndex] && aRow < _lastSelectedRow));
 4529     }

when crashing _selectionAnchorRow = -1, so the calculated range is not valid.

Going deeper :


 - (BOOL)startTrackingAt:(CGPoint)aPoint
 4035 {
 4036     // Try to become the first responder, but if we can't, that's okay.
 4037     [[self window] makeFirstResponder:self];
 4038 
 4039     var row = [self rowAtPoint:aPoint];
 4040 
 4041     // If the user clicks outside a row then deselect everything.
 4042     if (row < 0 && _allowsEmptySelection)
 4043         [self selectRowIndexes:[CPIndexSet indexSet] byExtendingSelection:NO];
 4044 
 4045     [self _noteSelectionIsChanging];
 4046 
 4047     if ([self mouseDownFlags] & CPShiftKeyMask)
 4048         _selectionAnchorRow = (ABS([_selectedRowIndexes firstIndex] - row) < ABS([_selectedRowIndexes lastIndex] - row)) ?
 4049             [_selectedRowIndexes firstIndex] : [_selectedRowIndexes lastIndex];
 4050     else
 4051         _selectionAnchorRow = row;

row = [self rowAtPoint:aPoint] when we are below the last item return -1.

Going deeper :

 1799 - (CPInteger)rowAtPoint:(CGPoint)aPoint
 1800 {
 1801  if (_implementedDelegateMethods & CPTableViewDelegate_tableView_heightOfRow_)
 1802  {
 1803  return [_cachedRowHeights indexOfObject:aPoint
 1804  inSortedRange:nil
 1805  options:0
 1806  usingComparator:function(aPoint, rowCache)
 1807  {
 1808  var upperBound = rowCache.heightAboveRow;
 1809 
 1810  if (aPoint.y < upperBound)
 1811  return CPOrderedAscending;
 1812 
 1813  if (aPoint.y > upperBound + rowCache.height + _intercellSpacing.height)
 1814  return CPOrderedDescending;
 1815 
 1816  return CPOrderedSame;
 1817  }];
 1818  }
 1819 
 1820  var y = aPoint.y,
 1821  row = FLOOR(y / (_rowHeight + _intercellSpacing.height));
 1822 
 1823  if (row >= _numberOfRows)
 1824  return CPNotFound;
 1825 
 1826  return row;
 1827 }

return CPNotFound value is "-1" when we row > _numberOfRows. I think I've found the issue.

@CyrilPeponnet

This comment has been minimized.

Show comment Hide comment
@CyrilPeponnet

CyrilPeponnet May 31, 2013

Bump for this one, nobody for a fix ?

Bump for this one, nobody for a fix ?

@Dogild

This comment has been minimized.

Show comment Hide comment
@Dogild

Dogild May 31, 2013

Member

I'll try to take a look these next days

Member

Dogild commented May 31, 2013

I'll try to take a look these next days

@CyrilPeponnet

This comment has been minimized.

Show comment Hide comment
@CyrilPeponnet

CyrilPeponnet Dec 20, 2013

@Dogild I still have the issue with the right click... your PR fixes the drag/drop but not the right click:

Exception: CPInvalidArgumentException: Range {-1, 1} is out of bounds.

capture2

capture

@Dogild I still have the issue with the right click... your PR fixes the drag/drop but not the right click:

Exception: CPInvalidArgumentException: Range {-1, 1} is out of bounds.

capture2

capture

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment