<Swing Dev> [11] Review Request: JDK-6562442: JList.setModel() may silently fail if ListModel.equals() returns true

[Pankaj Bansal]

Hi Semyon,

 

<<I'm not sure that the splitting one property change event into two events would be correct.

I used the first change event to remove the ListDataListeners if any is added to  current list model. I think this step can be skipped and we can directly fire property change for to set new ListModel with null as old value.

<<Anyway, I don't think that any issues take place in the scenario written in the bug description. 

The problem is that UI delegate is using different data model but is being informed of elements being added on different data model. The sample test in bug description has wrong number of elements in JList UI.

<<If for some reason the developer decided to overrides the equals() method to make two list models always equal to each other then he cannot expect that JList class will continue to treat those two models as different.

The equals is overridden to check actual values inside the ListModel, not always return true. I think a developer can override the equals for different reasons and still expect the setModel to work properly as setModel does not through give any feedback whether the new model was set or not. The model should be checked for equality using reference not by actual values stored in model. This is fairly common problem as there are about 3-4 duplicate bugs filed for this.

 

Regards,

Pankaj Bansal

 

From: Semyon Sadetsky 
Sent: Tuesday, February 6, 2018 9:51 PM
To: Pankaj Bansal; swing-dev at openjdk.java.net
Subject: Re: <Swing Dev> [11] Review Request: JDK-6562442: JList.setModel() may silently fail if ListModel.equals() returns true

 

Hi Pankaj,

I'm not sure that the splitting one property change event into two events would be correct.

Anyway, I don't think that any issues take place in the scenario written in the bug description. If for some reason the developer decided to overrides the equals() method to make two list models always equal to each other then he cannot expect that JList class will continue to treat those two models as different.

--Semyon

On 02/06/2018 02:22 AM, Pankaj Bansal wrote:

Hi All,

 

Please review the fix for JDK 11.

 

Bug:

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

 

webrev:

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

 

 

Issue:

JList may not set ListModel properly on calling JList.setModel in some cases.

 

The ListModel should be checked for equality by reference not by actual values, while calling JList.setModel. JList.setModel informs the UI delegate about the change by firing a propertyChange. Now firePropertyChange checks for equality between the models by calling equals. This may return a false true, if a subclass of ListModel has overridden the equals method to test the ListModels for actual values for some other purpose. This may leave UI delegate in inconsistent state.

 

Fix:

Made changes in JList.setModel to check for equality by reference and then call firePropertyChange. In first call to firePropertyChange, newValue is sent as null, so that ListDataListeners are removed from the old ListModel. Then new ListModel is set.

In test, for automation, the check is added on number of ListDataListeners in ListModel as JList.setModel should remove the listeners from old model and add to new model.

 

 

Regards,

Pankaj Bansal

 

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