<Swing Dev> [11] Review Request: JDK-6481195 ListSelectionListener indicates events on model.addElement after model.clear()
Semyon Sadetsky
semyon.sadetsky at oracle.com
Wed Jan 31 16:18:46 UTC 2018
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>
More information about the swing-dev
mailing list