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







Reply via email to