<Swing Dev> [11] Review Request: JDK-4334792: Lead Index in DefaultListSelectionModel will never return to -1

Pankaj Bansal pankaj.b.bansal at oracle.com
Fri Feb 16 17:23:35 UTC 2018


Hi Semyon,

 

Thank you for your time.

>>My point is the same as in 6481195 your fix always changes lead/anchor=0 to -1 even when elements exist.

I don't think there is anything wrong as this is same behavior, if the anchor/lead was 1/1 and we delete the 2nd element, the anchor/lead becomes 0/0. So if the anchor/lead is 0/0 and 1st element is deleted, anchor/lead should become -1/-1. I can't see any reason for special treatment for this case.

>> why you didn't delete those explicit -1s? 

I missed this one, thanks for pointing this out. I see that anchor/lead is set explicitly at few places, at some places to 0/0 and to -1/-1 at other places. May we need not make changes to clearSelection. I can agree with this.

 

Please let me know what do you think. I will make changes as you suggest.

 

Regards,

Pankaj Bansal

From: Semyon Sadetsky 
Sent: Friday, February 16, 2018 9:55 PM
To: Pankaj Bansal; swing-dev at openjdk.java.net
Subject: Re: <Swing Dev> [11] Review Request: JDK-4334792: Lead Index in DefaultListSelectionModel will never return to -1

 

My point is the same as in 6481195 your fix always changes lead/anchor=0 to -1 even when elements exist.

I'm not so sure about clearSelection(). In JDK usages of this method we set lead/anchor to -1 explicitly when this really needed. Although I don't see enough reason to make this implicit now, anyway, why you didn't delete those explicit -1s? 

--Semyon

 

On 02/16/2018 08:08 AM, Pankaj Bansal wrote:

Hi Semyon,

 

1.      This fix has an additional change involved when JList.clearSelection is called. So this is not duplicate bug.

2.      I think you only mentioned that the 6481195 does not talk about lead/achor and we should not change these values in that bug and create a new bug for those changes. I used this bug instead of creating a new bug.

 

I can't close this bug as duplicate as this involve additional changes. I can close 6481195 as "fixed in someother bug" as it is subset of the present issue.

 

Regards,

Pankaj Bansal

 

 

From: Semyon Sadetsky 
Sent: Friday, February 16, 2018 8:59 PM
To: Pankaj Bansal; HYPERLINK "mailto:swing-dev at openjdk.java.net"swing-dev at openjdk.java.net
Subject: Re: <Swing Dev> [11] Review Request: JDK-4334792: Lead Index in DefaultListSelectionModel will never return to -1

 

You've already sent this change in 6481195 review. Please continue in 6481195  tread. The current bug  should be closed as duplicate.

 

On 02/15/2018 09:36 PM, Pankaj Bansal wrote:

Hi All,

 

Please review the fix for JDK 11.

 

Bug:

https://bugs.openjdk.java.net/browse/JDK-4334792

 

webrev:

HYPERLINK "http://cr.openjdk.java.net/%7Epbansal/4334792/webrev.00/"http://cr.openjdk.java.net/~pbansal/4334792/webrev.00/

 

 

Issue:

Lead Index in DefaultListSelectionModel will never return to -1 in some cases as described.

                                                                                                                              

Fix:

1) JList.setSelectionIndex(-1)

The doc says JList.setSelectedIndex() should select a single cell, but it doesn't mean it should clear selection if the argument is -1. 

This will also not be fixed as this is not an issue and behavior is expected.

 

2) JList.clearSelection() 

The proposed fix addresses this issue.

 

3) ListModel.removeAllElements()/ListModel.clear

This issue has been addressed already except the case when the lead/anchor is 0/0 and then JList.removeAllElements/ListModel.clear is called, then the anchor/lead is not set properly. This case is addressed in the proposed fix. This will also fix the https://bugs.openjdk.java.net/browse/JDK-6481195

 

4) ListSelectionModel.setLeadSelectionIndex()

This is not an issue as lead can be set to -1 by setLeadSelectionIndex, if the anchor is already set properly. This is expected behavior and will not be changed.

 

Regards,

Pankaj Bansal

 

 

 

 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20180216/138f0497/attachment.html>


More information about the swing-dev mailing list