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; [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:
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
