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