<Swing Dev> [9] Review request for 8032874: ArrayIndexOutOfBoundsException in JTable while clearing data in JTable

[Alexander Potochkin]

Looks good

Thanks
alexp

On 4/15/2014 3:30 PM, Alexander Scherbatiy wrote:
>
>   The fix looks good for me.
>
>   Thanks,
>   Alexandr.
>
> On 4/15/2014 11:13 AM, dmitry markov wrote:
>>
>> On 14/04/2014 17:06, Alexander Scherbatiy wrote:
>>> On 4/14/2014 4:33 PM, dmitry markov wrote:
>>>> Alexandr,
>>>>
>>>> Please find my answer below.
>>>>
>>>> Thanks,
>>>> Dmitry
>>>> On 14/04/2014 14:12, Alexander Scherbatiy wrote:
>>>>> On 4/14/2014 10:59 AM, dmitry markov wrote:
>>>>>> Hi Alexandr,
>>>>>>
>>>>>> Thank you for the review. According to the specification in 
>>>>>> ListSelectionModel: getAnchorSelectionIndex() and 
>>>>>> getLeadSelectionIndex() return the first index and the second 
>>>>>> index arguments from the most recent call to 
>>>>>> setSelectionInterval(), addSelectionInterval() or 
>>>>>> removeSelectionInterval(). So if -1 is not set explicitly via the 
>>>>>> methods described above, it is correct to return value which is 
>>>>>> not -1 even for empty selection. I guess, such behavior is 
>>>>>> intended for storing the information about the previous selection.
>>>>>      Is it possible that the lead selection index can be greater 
>>>>> than the table row numbers?
>>>> Yes, it is possible. The lead selection index may be greater or 
>>>> equal to the row count, (e.g. for case described in the bug the 
>>>> lead index is 0 for the empty selection).
>>>
>>>        What about if there are only 2 row counts in the table but 
>>> the  lead selection index is 5? Will the 
>>> convertRowIndexToView(viewLeadIndex) method
>>>        works correctly in this case.
>> It will work properly in this case, since the lead index will be 
>> updated by DefaultListSelectionModel.removeIndexInterval(). The 
>> problem takes place only when we remove the last row from a table. In 
>> this case removeIndexInterval() does not update the lead and the 
>> anchor indexes, (i.e. they stay equal to 0). This causes failure in 
>> convertRowIndexToView() method, since we try to use it on empty 
>> selection. Possible way to avoid the problem is not invoke 
>> convertRowIndexToView() for empty selection.
>>
>> I do not think we should change the current implementation of 
>> removeIndexInterval(), since it keeps the values of lead and anchor 
>> indexes in valid and correct range and prevents us from returning 
>> something strange, (e.g. -5, -10) as lead or anchor index of the 
>> previous selection.
>>
>> Thanks,
>> Dmitry
>>>
>>>   Thanks,
>>>   Alexandr.
>>>
>>>>>
>>>>>      I seems that the getAdjustedIndex(index, row) method checks 
>>>>> both these cases.
>>>> That is right. The fix does the same thing as getAdjustedIndex() 
>>>> method. Unfortunately we can not use the method in the fix, since 
>>>> if I get it right getAdjustedIndex() is designed for work with 
>>>> selectionModel, but here we work with modelSelection.
>>>>>
>>>>>      Thanks,
>>>>>      Alexandr.
>>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>> Dmitry
>>>>>> On 11/04/2014 17:07, Alexander Scherbatiy wrote:
>>>>>>> On 4/11/2014 4:36 PM, dmitry markov wrote:
>>>>>>>> Hello,
>>>>>>>>
>>>>>>>> Could you review the fix for jdk9, please?
>>>>>>>>
>>>>>>>>     bug: https://bugs.openjdk.java.net/browse/JDK-8032874
>>>>>>>>     webrev: 
>>>>>>>> http://cr.openjdk.java.net/~dmarkov/8032874/jdk9/webrev.00/
>>>>>>>>
>>>>>>>> Problem description: If JTable has Sorter and Filter and 
>>>>>>>> selected row is removed ArrayIndexOutOfBoundsException will be 
>>>>>>>> thrown.
>>>>>>>> Fix: The method restoreSelection() in SortManager class should 
>>>>>>>> invoke convertRowIndexToView() only when modelSelection is NOT 
>>>>>>>> empty.
>>>>>>>
>>>>>>>     Is it the right behavior that 
>>>>>>> modelSelection.getLeadSelectionIndex() does not return -1 when 
>>>>>>> modelSelection.isSelectionEmpty() is true?
>>>>>>>
>>>>>>>    Thanks,
>>>>>>>    Alexandr.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Dmitry
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>