Hi Jay,
Thanks for the review.
<<May be we are not handling the case where validateLeadIndex() fails and we
don't set selection interval and it is resulting in JCK test fail. If you can
share what is <<behavior of JCK test failure after your change it would be
helpful.
No, the JCK test expects the selection to be actually set to the given interval
irrespective of the number of items in the List. Following test case is failing
public Status JList2067() {
JList c = new JList(); // Create JList object
c.setSelectionInterval(10, 0);
if (c.getSelectedIndices().length != 11) {
ref.println("Wrong selection");
return Status.failed("FAILED");
}
<<Also specification of setSelectionInterval() or addSelectionInterval()
mentions that "{@code anchor} doesn't have to be less than or equal to {@code
lead}". So while <<validating arguments for setSelectionInterval() or
addSelectionInterval() I think we should verify the value of anchor first and
then check the value of (anchor + lead) <<instead of just checking whether lead
< size.
Yes, I think we have to check the condition for both anchor and lead both as
they both can be greater than size of the List. I will make that change if we
decide to go this route depending upon the other reviewers comments. But I feel
not making the change in setSelectionInterval and addSelectionInterval is the
preferred way as we will anyway have the checks in the getSelectedValuesList as
wrong selection can be directly set using DefaultListSelectionModel.
Regards
Pankaj Bansal
Regards
Pankaj Bansal
From: Jayathirth D V
Sent: Friday, December 1, 2017 4:17 PM
To: Pankaj Bansal; swing-dev@openjdk.java.net; Sergey Bylokhov; Semyon Sadetsky
Subject: RE: <Swing Dev> [10] Review Request: JDK-7108280 :
JList.getSelectedValuesList fails if JList.setSelectionInterval larger than list
Hello Pankaj,
Please find my observation:
As you have mentioned I also feel that adding check in setSelectionInterval()
or addSelectionInterval() would be a good approach. Since I am not aware of
swing component code I will leave this decision to others.
Regarding http://cr.openjdk.java.net/~pbansal/7108280/webrev.002/ :
May be we are not handling the case where validateLeadIndex() fails and we
don't set selection interval and it is resulting in JCK test fail. If you can
share what is behavior of JCK test failure after your change it would be
helpful.
Also specification of setSelectionInterval() or
addSelectionInterval() mentions that "{@code anchor} doesn't have to be less
than or equal to {@code lead}". So while validating arguments for
setSelectionInterval() or addSelectionInterval() I think we should verify the
value of anchor first and then check the value of (anchor + lead) instead of
just checking whether lead < size.
Thanks,
Jay
From: Pankaj Bansal
Sent: Friday, December 01, 2017 3:02 PM
To: HYPERLINK "mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net;
Sergey Bylokhov; Semyon Sadetsky
Subject: <Swing Dev> [10] Review Request: JDK-7108280 :
JList.getSelectedValuesList fails if JList.setSelectionInterval larger than list
Hi All,
Please review the fix.
Bug:
https://bugs.openjdk.java.net/browse/JDK-7108280
Webrev:
http://cr.openjdk.java.net/~pbansal/7108280/webrev.00/
Issue:
JList.getSelectedValuesList crashes if the JList.setSelectionInterval or
JList.addSelectionInterval had been called earlier with interval having lead
greater than the size of List
Fix:
Made changes in JList.getSelectedValuesList to check the if the max selection
index is greater than the actual size of the List. If yes, the max is changed
to last element index of List.
Note:
It makes sense to change the behavior of JList.setSelectionInterval or
JList.addSelectionInterval to not allow to set the selection with interval
having indices not present in the list. But it will change the behavior of this
API and will result in failure of 2 JCK tests.
Also, we will still have to put checks inside the JList.getSelectedValuesList
as the selection can be changed by setting selection interval on
DefualtListSelectionModel and there is no way to check if the supplied interval
range actually exist in the List inside DefualtListSelectionModel.
If changing the JList.setSelectionInterval or JList.addSelectionInterval is
possible, the potential fix can be following webrev.
http://cr.openjdk.java.net/~pbansal/7108280/webrev.002/
Regards,
Pankaj Bansal
