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

[Richard Bair]

Hey Dave,

Thanks for the ping.

It is in the review queue. I think the recent mad work on the Nimbus  
LAF and JSR 295 is slowing things down. I know Nimbus is sure taking  
it's toll on my schedule :-)

So just to let you know what is going on, when the bugfix has been  
applied to a local workspace, we generate a "webrev" which shows the  
revisions between the old and new files (a kind of diff, but with a  
lot more ways to view the changes). This then gets sent to two or  
more reviewers, even for trivial bugs like this one. Sometimes these  
reviews can sit in a queue for a bit, if the reviewers are hurting  
for time.

After both reviewers give it the ok, I can put the bug back. It will  
then take several weeks before the swing workspace is integrated back  
into the main workspace, goes through testing, and pops out in a build.

Thanks
Richard

On Jul 10, 2007, at 5:59 AM, David Gilbert wrote:

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