<Swing Dev> [PATCH] 6463712: JSpinner forwards events from old model

[David Gilbert]

Hi Richard,

Any update on this?

Dave.

Richard Bair wrote:
> Hey Dave, thanks for the patch.
>
> For those following along, I've been talking with Dave about it 
> offline, sorry for not posting back to the openjdk list. I'm repenting 
> :-). Also, due to post-JavaOne vacations, this is taking a wee bit 
> longer than usual.
>
> I've looked at the patch, it looks good. I *really* appreciate the 
> supplied test as well. The only thing I need to verify is that 
> oldModel is never null (or this new line would produce an NPE). From a 
> cursory look at JSpinner, it appears to never be null, but I need to 
> take another look.
>
> Also, I'm working with Igor on a code review and putback.
>
> Thanks Dave!
>
> Richard
>
> On May 14, 2007, at 6:51 AM, David Gilbert wrote:
>
>> Hi,
>>
>> The attached patch is a proposed fix for bug 6463712.  It is a 
>> one-line fix to the setModel(SpinnerModel) method - the listener that 
>> works on behalf of the JSpinner to forward events from the model 
>> needs to be deregistered from the old model when a new model is 
>> assigned.
>>
>> The patch includes a regression test that can be run using jtreg.  I 
>> also tested using the JSpinner-related tests in Mauve[1].
>>
>> I'm working against the initial OpenJDK source release (b12).
>> Any questions, just ask...
>>
>> Regards,
>>
>> Dave Gilbert
>> http://www.jfree.org/
>>
>> [1]  http://sourceware.org/mauve/
>>
>> diff -ruN j2se/src/share/classes/javax/swing/JSpinner.java 
>> ../mods/openjdk/j2se/src/share/classes/javax/swing/JSpinner.java
>> --- j2se/src/share/classes/javax/swing/JSpinner.java    2007-05-06 
>> 10:11:15.000000000 +0100
>> +++ 
>> ../mods/openjdk/j2se/src/share/classes/javax/swing/JSpinner.java    
>> 2007-05-08 22:19:11.000000000 +0100
>> @@ -286,6 +286,7 @@
>>          SpinnerModel oldModel = this.model;
>>          this.model = model;
>>          if (modelListener != null) {
>> +        oldModel.removeChangeListener(modelListener);
>>          this.model.addChangeListener(modelListener);
>>          }
>>          firePropertyChange("model", oldModel, model);
>> diff -ruN j2se/test/javax/swing/JSpinner/JSpinnerBug6463712.java 
>> ../mods/openjdk/j2se/test/javax/swing/JSpinner/JSpinnerBug6463712.java
>> --- j2se/test/javax/swing/JSpinner/JSpinnerBug6463712.java    
>> 1970-01-01 01:00:00.000000000 +0100
>> +++ 
>> ../mods/openjdk/j2se/test/javax/swing/JSpinner/JSpinnerBug6463712.java    
>> 2007-05-14 12:19:22.000000000 +0100
>> @@ -0,0 +1,55 @@
>> +/*
>> + * Copyright 2007, Object Refinery Limited.  All Rights Reserved.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, write to the Free Software
>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
>> + * 02110-1301, USA
>> + */
>> +
>> +import javax.swing.JSpinner;
>> +import javax.swing.SpinnerDateModel;
>> +import javax.swing.SpinnerNumberModel;
>> +import javax.swing.event.ChangeEvent;
>> +import javax.swing.event.ChangeListener;
>> +
>> +/*
>> + * @test
>> + * @bug 6463712
>> + * @summary Events forwarded from previous model
>> + */
>> +public class JSpinnerBug6463712 implements ChangeListener {
>> +
>> +  public JSpinnerBug6463712()
>> +  {
>> +    SpinnerNumberModel m1 = new SpinnerNumberModel();
>> +    JSpinner s = new JSpinner(m1);
>> +    s.addChangeListener(this);
>> +    SpinnerDateModel m2 = new SpinnerDateModel();
>> +    s.setModel(m2);
>> +
>> +    // m1 is no longer linked to the JSpinner (it has been replaced 
>> by m2), so
>> +    // the following should not trigger a call to our stateChanged() 
>> method...
>> +    m1.setValue(new Integer(1));
>> +  }
>> +
>> +  public void stateChanged(ChangeEvent e)
>> +  {
>> +    throw new RuntimeException("Should not receive this event.");
>> +  }
>> +
>> +  public static void main(String[] args)
>> +  {
>> +    JSpinnerBug6463712 bug = new JSpinnerBug6463712();
>> +  }
>> +}
>
>
>