-
Bug
-
Resolution: Fixed
-
P4
-
7
-
b10
-
generic
-
generic
-
Verified
A DESCRIPTION OF THE FIX :
BasicListUI has a private class Handler which has a method propertyChange(PropertyChangeEvent e). In this method the propertyName is compared with the == operator. This is very bad style and can lead to strange problems (see attached testcases). In addition to that the property "cellRenderer" is checked twice.
The submitter created a patch against build 1.7.0-ea-b06:
-------------------
--- original/BasicListUI.java 2007-01-20 22:24:31.000000000 +0100
+++ BasicListUI.java 2007-01-20 22:28:14.000000000 +0100
@@ -2436,7 +2436,7 @@
/* If the JList.model property changes, remove our listener,
* listDataListener from the old model and add it to the new one.
*/
- if (propertyName == "model") {
+ if ("model".equals(propertyName)) {
ListModel oldModel = (ListModel)e.getOldValue();
ListModel newModel = (ListModel)e.getNewValue();
if (oldModel != null) {
@@ -2452,7 +2452,7 @@
/* If the JList.selectionModel property changes, remove our listener,
* listSelectionListener from the old selectionModel and add it to the new one.
*/
- else if (propertyName == "selectionModel") {
+ else if ("selectionModel".equals(propertyName)) {
ListSelectionModel oldModel = (ListSelectionModel)e.getOldValue();
ListSelectionModel newModel = (ListSelectionModel)e.getNewValue();
if (oldModel != null) {
@@ -2464,48 +2464,44 @@
updateLayoutStateNeeded |= modelChanged;
redrawList();
}
- else if (propertyName == "cellRenderer") {
+ else if ("cellRenderer".equals(propertyName)) {
updateLayoutStateNeeded |= cellRendererChanged;
redrawList();
}
- else if (propertyName == "font") {
+ else if ("font".equals(propertyName)) {
updateLayoutStateNeeded |= fontChanged;
redrawList();
}
- else if (propertyName == "prototypeCellValue") {
+ else if ("prototypeCellValue".equals(propertyName)) {
updateLayoutStateNeeded |= prototypeCellValueChanged;
redrawList();
}
- else if (propertyName == "fixedCellHeight") {
+ else if ("fixedCellHeight".equals(propertyName)) {
updateLayoutStateNeeded |= fixedCellHeightChanged;
redrawList();
}
- else if (propertyName == "fixedCellWidth") {
+ else if ("fixedCellWidth".equals(propertyName)) {
updateLayoutStateNeeded |= fixedCellWidthChanged;
redrawList();
}
- else if (propertyName == "cellRenderer") {
- updateLayoutStateNeeded |= cellRendererChanged;
- redrawList();
- }
- else if (propertyName == "selectionForeground") {
+ else if ("selectionForeground".equals(propertyName)) {
list.repaint();
}
- else if (propertyName == "selectionBackground") {
+ else if ("selectionBackground".equals(propertyName)) {
list.repaint();
}
- else if ("layoutOrientation" == propertyName) {
+ else if ("layoutOrientation".equals(propertyName)) {
updateLayoutStateNeeded |= layoutOrientationChanged;
layoutOrientation = list.getLayoutOrientation();
redrawList();
}
- else if ("visibleRowCount" == propertyName) {
+ else if ("visibleRowCount".equals(propertyName)) {
if (layoutOrientation != JList.VERTICAL) {
updateLayoutStateNeeded |= layoutOrientationChanged;
redrawList();
}
}
- else if ("componentOrientation" == propertyName) {
+ else if ("componentOrientation".equals(propertyName)) {
isLeftToRight = list.getComponentOrientation().isLeftToRight();
updateLayoutStateNeeded |= componentOrientationChanged;
redrawList();
@@ -2513,10 +2509,10 @@
InputMap inputMap = getInputMap(JComponent.WHEN_FOCUSED);
SwingUtilities.replaceUIInputMap(list, JComponent.WHEN_FOCUSED,
inputMap);
- } else if ("List.isFileList" == propertyName) {
+ } else if ("List.isFileList".equals(propertyName)) {
updateIsFileList();
redrawList();
- } else if ("dropLocation" == propertyName) {
+ } else if ("dropLocation".equals(propertyName)) {
JList.DropLocation oldValue = (JList.DropLocation)e.getOldValue();
repaintDropLocation(oldValue);
repaintDropLocation(list.getDropLocation());
-------------------
JUnit TESTCASE :
Here is a testcase that verifies the problem:
-------------------
import java.beans.PropertyChangeEvent;
import java.beans.PropertyChangeListener;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
import javax.swing.JList;
import javax.swing.plaf.basic.BasicListUI;
import junit.framework.TestCase;
/**
*
* @author ###@###.###
*/
public class BasicListUITest extends TestCase {
public void testBasicListUI() {
try {
JList list = new JList();
BasicListUI basicListUI = (BasicListUI) list.getUI();
// get updateLayoutStateNeeded
Field ulsnField = BasicListUI.class.getDeclaredField("updateLayoutStateNeeded");
ulsnField.setAccessible(true);
int ulsnBefore = ulsnField.getInt(basicListUI);
// get handler
Method getHandlerMethod =
BasicListUI.class.getDeclaredMethod("getHandler");
getHandlerMethod.setAccessible(true);
PropertyChangeListener propertyChangeListener =
(PropertyChangeListener) getHandlerMethod.invoke(basicListUI);
propertyChangeListener.propertyChange(
new PropertyChangeEvent(this, "model", null, null));
// check updateLayoutStateNeeded again
int ulsnAfter = ulsnField.getInt(basicListUI);
assertFalse("The field \"updateLayoutStateNeeded\" was unchanged after property changes!",
ulsnBefore == ulsnAfter);
} catch (Exception ex) {
ex.printStackTrace();
}
}
}
-------------------
Here is an example program that shows the problem in a real example without the reflection magic used in the JUnit test. I subclassed JList and suddenly setModel() stops working!
-------------------
import javax.swing.*;
/**
* @author ###@###.###
*/
public class Test extends JFrame {
private JList jList1;
private JScrollPane jScrollPane1;
public Test() {
initComponents();
DefaultListModel listModel = new DefaultListModel();
listModel.addElement("test");
jList1.setModel(listModel);
}
private void initComponents() {
jScrollPane1 = new JScrollPane();
jList1 = new MyJList();
setDefaultCloseOperation(WindowConstants.EXIT_ON_CLOSE);
jScrollPane1.setViewportView(jList1);
GroupLayout layout = new GroupLayout(getContentPane());
getContentPane().setLayout(layout);
layout.setHorizontalGroup(
layout.createParallelGroup(GroupLayout.Alignment.LEADING)
.addGroup(layout.createSequentialGroup()
.addContainerGap()
.addComponent(jScrollPane1, GroupLayout.DEFAULT_SIZE, 376, Short.MAX_VALUE)
.addContainerGap())
);
layout.setVerticalGroup(
layout.createParallelGroup(GroupLayout.Alignment.LEADING)
.addGroup(layout.createSequentialGroup()
.addContainerGap()
.addComponent(jScrollPane1, GroupLayout.DEFAULT_SIZE, 276, Short.MAX_VALUE)
.addContainerGap())
);
pack();
}
public static void main(String args[]) {
java.awt.EventQueue.invokeLater(new Runnable() {
public void run() {
new Test().setVisible(true);
}
});
}
private class MyJList extends JList {
public void setModel(ListModel model) {
if (model == null) {
throw new IllegalArgumentException("model must be non null");
}
ListModel dataModel = getModel();
ListModel oldValue = dataModel;
dataModel = model;
firePropertyChange("model", oldValue, dataModel);
clearSelection();
}
}
}
-------------------
BasicListUI has a private class Handler which has a method propertyChange(PropertyChangeEvent e). In this method the propertyName is compared with the == operator. This is very bad style and can lead to strange problems (see attached testcases). In addition to that the property "cellRenderer" is checked twice.
The submitter created a patch against build 1.7.0-ea-b06:
-------------------
--- original/BasicListUI.java 2007-01-20 22:24:31.000000000 +0100
+++ BasicListUI.java 2007-01-20 22:28:14.000000000 +0100
@@ -2436,7 +2436,7 @@
/* If the JList.model property changes, remove our listener,
* listDataListener from the old model and add it to the new one.
*/
- if (propertyName == "model") {
+ if ("model".equals(propertyName)) {
ListModel oldModel = (ListModel)e.getOldValue();
ListModel newModel = (ListModel)e.getNewValue();
if (oldModel != null) {
@@ -2452,7 +2452,7 @@
/* If the JList.selectionModel property changes, remove our listener,
* listSelectionListener from the old selectionModel and add it to the new one.
*/
- else if (propertyName == "selectionModel") {
+ else if ("selectionModel".equals(propertyName)) {
ListSelectionModel oldModel = (ListSelectionModel)e.getOldValue();
ListSelectionModel newModel = (ListSelectionModel)e.getNewValue();
if (oldModel != null) {
@@ -2464,48 +2464,44 @@
updateLayoutStateNeeded |= modelChanged;
redrawList();
}
- else if (propertyName == "cellRenderer") {
+ else if ("cellRenderer".equals(propertyName)) {
updateLayoutStateNeeded |= cellRendererChanged;
redrawList();
}
- else if (propertyName == "font") {
+ else if ("font".equals(propertyName)) {
updateLayoutStateNeeded |= fontChanged;
redrawList();
}
- else if (propertyName == "prototypeCellValue") {
+ else if ("prototypeCellValue".equals(propertyName)) {
updateLayoutStateNeeded |= prototypeCellValueChanged;
redrawList();
}
- else if (propertyName == "fixedCellHeight") {
+ else if ("fixedCellHeight".equals(propertyName)) {
updateLayoutStateNeeded |= fixedCellHeightChanged;
redrawList();
}
- else if (propertyName == "fixedCellWidth") {
+ else if ("fixedCellWidth".equals(propertyName)) {
updateLayoutStateNeeded |= fixedCellWidthChanged;
redrawList();
}
- else if (propertyName == "cellRenderer") {
- updateLayoutStateNeeded |= cellRendererChanged;
- redrawList();
- }
- else if (propertyName == "selectionForeground") {
+ else if ("selectionForeground".equals(propertyName)) {
list.repaint();
}
- else if (propertyName == "selectionBackground") {
+ else if ("selectionBackground".equals(propertyName)) {
list.repaint();
}
- else if ("layoutOrientation" == propertyName) {
+ else if ("layoutOrientation".equals(propertyName)) {
updateLayoutStateNeeded |= layoutOrientationChanged;
layoutOrientation = list.getLayoutOrientation();
redrawList();
}
- else if ("visibleRowCount" == propertyName) {
+ else if ("visibleRowCount".equals(propertyName)) {
if (layoutOrientation != JList.VERTICAL) {
updateLayoutStateNeeded |= layoutOrientationChanged;
redrawList();
}
}
- else if ("componentOrientation" == propertyName) {
+ else if ("componentOrientation".equals(propertyName)) {
isLeftToRight = list.getComponentOrientation().isLeftToRight();
updateLayoutStateNeeded |= componentOrientationChanged;
redrawList();
@@ -2513,10 +2509,10 @@
InputMap inputMap = getInputMap(JComponent.WHEN_FOCUSED);
SwingUtilities.replaceUIInputMap(list, JComponent.WHEN_FOCUSED,
inputMap);
- } else if ("List.isFileList" == propertyName) {
+ } else if ("List.isFileList".equals(propertyName)) {
updateIsFileList();
redrawList();
- } else if ("dropLocation" == propertyName) {
+ } else if ("dropLocation".equals(propertyName)) {
JList.DropLocation oldValue = (JList.DropLocation)e.getOldValue();
repaintDropLocation(oldValue);
repaintDropLocation(list.getDropLocation());
-------------------
JUnit TESTCASE :
Here is a testcase that verifies the problem:
-------------------
import java.beans.PropertyChangeEvent;
import java.beans.PropertyChangeListener;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
import javax.swing.JList;
import javax.swing.plaf.basic.BasicListUI;
import junit.framework.TestCase;
/**
*
* @author ###@###.###
*/
public class BasicListUITest extends TestCase {
public void testBasicListUI() {
try {
JList list = new JList();
BasicListUI basicListUI = (BasicListUI) list.getUI();
// get updateLayoutStateNeeded
Field ulsnField = BasicListUI.class.getDeclaredField("updateLayoutStateNeeded");
ulsnField.setAccessible(true);
int ulsnBefore = ulsnField.getInt(basicListUI);
// get handler
Method getHandlerMethod =
BasicListUI.class.getDeclaredMethod("getHandler");
getHandlerMethod.setAccessible(true);
PropertyChangeListener propertyChangeListener =
(PropertyChangeListener) getHandlerMethod.invoke(basicListUI);
propertyChangeListener.propertyChange(
new PropertyChangeEvent(this, "model", null, null));
// check updateLayoutStateNeeded again
int ulsnAfter = ulsnField.getInt(basicListUI);
assertFalse("The field \"updateLayoutStateNeeded\" was unchanged after property changes!",
ulsnBefore == ulsnAfter);
} catch (Exception ex) {
ex.printStackTrace();
}
}
}
-------------------
Here is an example program that shows the problem in a real example without the reflection magic used in the JUnit test. I subclassed JList and suddenly setModel() stops working!
-------------------
import javax.swing.*;
/**
* @author ###@###.###
*/
public class Test extends JFrame {
private JList jList1;
private JScrollPane jScrollPane1;
public Test() {
initComponents();
DefaultListModel listModel = new DefaultListModel();
listModel.addElement("test");
jList1.setModel(listModel);
}
private void initComponents() {
jScrollPane1 = new JScrollPane();
jList1 = new MyJList();
setDefaultCloseOperation(WindowConstants.EXIT_ON_CLOSE);
jScrollPane1.setViewportView(jList1);
GroupLayout layout = new GroupLayout(getContentPane());
getContentPane().setLayout(layout);
layout.setHorizontalGroup(
layout.createParallelGroup(GroupLayout.Alignment.LEADING)
.addGroup(layout.createSequentialGroup()
.addContainerGap()
.addComponent(jScrollPane1, GroupLayout.DEFAULT_SIZE, 376, Short.MAX_VALUE)
.addContainerGap())
);
layout.setVerticalGroup(
layout.createParallelGroup(GroupLayout.Alignment.LEADING)
.addGroup(layout.createSequentialGroup()
.addContainerGap()
.addComponent(jScrollPane1, GroupLayout.DEFAULT_SIZE, 276, Short.MAX_VALUE)
.addContainerGap())
);
pack();
}
public static void main(String args[]) {
java.awt.EventQueue.invokeLater(new Runnable() {
public void run() {
new Test().setVisible(true);
}
});
}
private class MyJList extends JList {
public void setModel(ListModel model) {
if (model == null) {
throw new IllegalArgumentException("model must be non null");
}
ListModel dataModel = getModel();
ListModel oldValue = dataModel;
dataModel = model;
firePropertyChange("model", oldValue, dataModel);
clearSelection();
}
}
}
-------------------