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; [email protected]
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

 

 

Reply via email to