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

[Semyon Sadetsky]

Hi Pankaj,

ListModel belongs to Swing. Developer may not use ListModel equals() 
only for its application needs and expect that ListModel continues to be 
embedded into Swing framework as usual. This is a generic approach, if 
you want to change it why you change it only for JList?

--Semyon


On 02/06/2018 10:01 AM, Pankaj Bansal wrote:
>
> 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:
>
>     http://cr.openjdk.java.net/~pbansal/6562442/webrev.00/
>     <http://cr.openjdk.java.net/%7Epbansal/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/20180207/04f6ed5c/attachment.html>