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

[Semyon Sadetsky]

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; 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/20180130/64827805/attachment.html>