<Swing Dev> [11] Review Request: JDK-6481195 ListSelectionListener indicates events on model.addElement after model.clear()

[Semyon Sadetsky]

Hi Pankaj,

This is directly specified in getLeadSelectionIndex()/ 
getAnchorSelectionIndex().

Anchor and lead indexes are used in lot of places. Which tests did you 
run after the fix?

--Semyon


On 01/31/2018 12:38 AM, Pankaj Bansal wrote:
>
> Hi Semyon,
>
> << When only the first list element deleted the anchor and lead 
> selection indexes become -1 after your fix while should be 0.
>
> Is it mentioned somewhere that it should be 0 ? I mean if lead index 
> was 2 and only 3^rd element is deleted, lead becomes 1. Same should be 
> true when lead is 0 and first element is deleted. Is it rule that lead 
> or anchor can’t go beyond 0 considering their default value is -1? I 
> think there is a similar bug filled as why should not lead or anchor 
> become -1.
>
> https://bugs.openjdk.java.net/browse/JDK-4334792
>
> Regards,
>
> Pankaj Bansal
>
> *From:*Semyon Sadetsky
> *Sent:* Wednesday, January 31, 2018 12:17 PM
> *To:* Pankaj Bansal; Jayathirth D V; swing-dev at openjdk.java.net; 
> Sergey Bylokhov; Prasanta Sadhukhan
> *Subject:* Re: <Swing Dev> [11] Review Request: JDK-6481195 
> ListSelectionListener indicates events on model.addElement after 
> model.clear()
>
> Hi Pankaj,
>
> When only the first list element deleted the anchor and lead selection 
> indexes become -1 after your fix while should be 0.
>
> --Semyon
>
> On 01/30/2018 01:13 AM, Pankaj Bansal wrote:
>
>     Hi Jay,
>
>     I have made the small correction you suggested.
>
>     webrev: http://cr.openjdk.java.net/~pbansal/6481195/webrev.02/
>     <http://cr.openjdk.java.net/%7Epbansal/6481195/webrev.02/>
>
>     Regards,
>
>     Pankaj Bansal
>
>     *From:* Jayathirth D V
>     *Sent:* Tuesday, January 30, 2018 12:13 PM
>     *To:* Pankaj Bansal; swing-dev at openjdk.java.net
>     <mailto:swing-dev at openjdk.java.net>; Sergey Bylokhov; Semyon
>     Sadetsky; Prasanta Sadhukhan
>     *Subject:* RE: <Swing Dev> [11] Review Request: JDK-6481195
>     ListSelectionListener indicates events on model.addElement after
>     model.clear()
>
>     Hi Pankaj,
>
>     Changes are fine.
>
>     As mentioned in previous suggestion Java code convention for
>     different statements like if () and for ()
>     loops(http://www.oracle.com/technetwork/java/javase/documentation/codeconventions-142311.html
>     ) we have to maintain single space between if/for and braces. In
>     test case this needs to be updated. There is no need for another
>     webrev please update it before you push the change.
>
>     Thanks,
>
>     Jay
>
>     *From:* Pankaj Bansal
>     *Sent:* Monday, January 29, 2018 3:39 PM
>     *To:* Jayathirth D V; swing-dev at openjdk.java.net
>     <mailto:swing-dev at openjdk.java.net>; Sergey Bylokhov; Semyon
>     Sadetsky; Prasanta Sadhukhan
>     *Subject:* RE: <Swing Dev> [11] Review Request: JDK-6481195
>     ListSelectionListener indicates events on model.addElement after
>     model.clear()
>
>     Hi Jay,
>
>     Thanks for the review.
>
>     I have made the suggested changes.
>
>     webrev: http://cr.openjdk.java.net/~pbansal/6481195/webrev.01/
>     <http://cr.openjdk.java.net/%7Epbansal/6481195/webrev.01/>
>
>     Regards,
>
>     Pankaj Bansal
>
>     *From:* Jayathirth D V
>     *Sent:* Thursday, January 25, 2018 6:28 PM
>     *To:* Pankaj Bansal; swing-dev at openjdk.java.net
>     <mailto:swing-dev at openjdk.java.net>; Sergey Bylokhov; Semyon
>     Sadetsky; Prasanta Sadhukhan
>     *Subject:* RE: <Swing Dev> [11] Review Request: JDK-6481195
>     ListSelectionListener indicates events on model.addElement after
>     model.clear()
>
>     Hi Pankaj,
>
>     Please find my inputs:
>
>     Test case is working properly before and after changes.
>
>     But I think we should make changes in test to actually detect all
>     failure cases properly instead of throwing RuntimeException when
>     we hit first failure:
>
>     if (numberOfEvents > 2) {
>
>     throw new RuntimeException("Wrong number of Events. " +
>
>     "Expected: 2, Actual: " + numberOfEvents);
>
>     }
>
>     if (list.getLeadSelectionIndex() != -1) {
>
>     throw new RuntimeException("Wrong Lead Index. " +
>
>     "Expected: -1, Actual: " + list
>
>       .getLeadSelectionIndex());
>
>     }
>
>     if (list.getAnchorSelectionIndex() != -1) {
>
>     throw new RuntimeException("Wrong Anchor Index. " +
>
>     "Expected: -1, Actual: " + list
>
>                     .getAnchorSelectionIndex());
>
>     }
>
>     Before your code change we just hit first check throw exception
>     which will not cover all the required conditions(proper number of
>     events, proper lead selection index & proper anchor selection index).
>
>     We can update a boolean variable in each of 3 cases and then check
>     them and throw relevant RuntimeException.
>
>     Also in test case there are some indentation problems in for loop,
>     if condition and expressions please update them also.
>
>     Thanks,
>
>     Jay
>
>     *From:* Pankaj Bansal
>     *Sent:* Monday, January 22, 2018 6:43 PM
>     *To:* swing-dev at openjdk.java.net
>     <mailto:swing-dev at openjdk.java.net>; Sergey Bylokhov; Semyon
>     Sadetsky; Prasanta Sadhukhan
>     *Subject:* <Swing Dev> [11] Review Request: JDK-6481195
>     ListSelectionListener indicates events on model.addElement after
>     model.clear()
>
>     Hi All,
>
>     Please review the fix for JDK 11.
>
>     Bug:
>
>     https://bugs.openjdk.java.net/browse/JDK-6481195
>
>     Webrev:
>
>     http://cr.openjdk.java.net/~pbansal/6481195/webrev.00/
>     <http://cr.openjdk.java.net/%7Epbansal/6481195/webrev.00/>
>
>     Issue:
>
>     Invalid ListSelectionEvents are being fired, when the data is
>     added after the clear() function has been called on
>     DefaultListModel. This only happens when the index 0 was selected
>     before calling clear.
>
>     When the index 0 is selected on a JList and clear() function is
>     called, anchor and lead are not being updated properly in
>     DefaultSelectionModel as they are not being reset to -1 inside the
>     removeIndexInterval() function. Because of this, when new elements
>     are added in model, a ListSelectionEvent is fired from
>     insertIndexInterval() function.
>
>     Fix:
>
>     The code inside removeIndexInterval function handles the case when
>     0 index is selected in special way. Removed that piece of code.
>
>     Note:
>
>     This also fixes https://bugs.openjdk.java.net/browse/JDK-4334792
>     partially when removeAllElements or clear() is called on
>     DefaultListModel.
>
>     Regards
>
>     Pankaj Bansal
>

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