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

[Richard Bair]

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();
> +  }
> +}