Hi Florian,
I have several comment about your patch:
1. Run an attached test, enter some text (e.g. "aa") and press enter.
The text disappears, but if you reenter text it stays.
2. The following changes in the
javax.swing.plaf.basic.BasicComboBoxEditor#getItem() looks strange for
me, could you describe them please?
+ if (oldValue instanceof String) {
+ return (E) newValue;
+ } else {...
BTW: The code "Method method = cls.getMethod("valueOf", new
Class[]{String.class});" and other stuff like that were added with the
fix of CR 4239610
(http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4239610). That code
looks strange for me, I believe the check
"newValue.equals(oldValue.toString())" is enough for that bug...
Moreover the "valueof" method cannot be used as a universal solution for
String->AnyClass conversion because of that's impossible to use in case
oldValue is null.
Therefore I think we should not change the WindowsComboBoxUI.java,
ComboBoxEditor.java, BasicComboBoxEditor.java and
MetalComboBoxEditor.java files at all, because there are no available
API for converting string into *any* class. What do you think about
that? Logically it means that editor can edit only strings but not any
class.
3. After item 2 we will see that it's not possible to generify the
set/getSelectedItem methods in the javax.swing.ComboBoxModel interface
because of some code in the javax.swing.JComboBox#actionPerformed
method. The same set/getSelectedItem methods in the JComboBox class
shouldn't be generified as well. That's also correspondent to my old
proposal:
http://mail.openjdk.java.net/pipermail/swing-dev/2009-December/000878.html
4. As the result of item 3 we can generify
javax.swing.JComboBox#dataModel like this:
protected ComboBoxModel<? extends E> dataModel;
I remember our discussion about JList model generification:
http://mail.openjdk.java.net/pipermail/swing-dev/2009-March/000443.html
So we can improve JList generification as well (it's possible to do
without backward incompatibility because of JList generification is a
jdk7 feature)
5. Some regression tests are needed (like this one
http://hg.openjdk.java.net/jdk7/swing/jdk/file/7bcb1864f424/test/javax/swing/JList/6823603/bug6823603.java).
We should write them after generification will be fully discussed and
coordinated.
I attached a raw patch that contains all changes that I described above.
What do you think about this way of generification?
Regards, Pavel
Hi Pavel,
here is a patch to "generify" JComboBox along with: ComboBoxEditor,
ComboBoxModel,
DefaultComboBoxModel and MutableComboBoxModel.
I fixed the BasicComboBoxEditor as I proposed in my last post.
MutableComboBoxModel.removeElement is not generified currently and thus still
takes an Object as argument. This is consistent with the Collection Framework
(eg. java.util.List.remove). Please tell me if you prefer this method to take a
generic parameter as well.
I also created a new issue in the OpenJDK Bugzilla system:
https://bugs.openjdk.java.net/show_bug.cgi?id=100153
Regards,
Florian
package crs.issue100153;
import javax.swing.*;
public class Test {
public static void main(String[] args) {
SwingUtilities.invokeLater(new Runnable() {
public void run() {
JComboBox<Integer> comboBox = new JComboBox<Integer>(new
Integer[]{1, 2, 666});
comboBox.setEditable(true);
JPanel content = new JPanel();
content.add(comboBox);
JFrame frame = new JFrame();
frame.setContentPane(content);
frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
frame.pack();
frame.setVisible(true);
}
});
}
}
--- old/src/share/classes/javax/swing/ComboBoxModel.java 2010-11-23 13:51:19.984375000 +0300
+++ new/src/share/classes/javax/swing/ComboBoxModel.java 2010-11-23 13:51:18.843750000 +0300
@@ -33,9 +33,11 @@
* <code>ListModel</code>. This disjoint behavior allows for the temporary
* storage and retrieval of a selected item in the model.
*
+ * @param <E> the type of the elements of this model
+ *
* @author Arnaud Weber
*/
-public interface ComboBoxModel extends ListModel {
+public interface ComboBoxModel<E> extends ListModel<E> {
/**
* Set the selected item. The implementation of this method should notify
--- old/src/share/classes/javax/swing/DefaultComboBoxModel.java 2010-11-23 13:51:24.718750000 +0300
+++ new/src/share/classes/javax/swing/DefaultComboBoxModel.java 2010-11-23 13:51:23.703125000 +0300
@@ -44,19 +44,21 @@
/**
* The default model for combo boxes.
*
+ * @param <E> the type of the elements of this model
+ *
* @author Arnaud Weber
* @author Tom Santos
*/
-public class DefaultComboBoxModel extends AbstractListModel implements MutableComboBoxModel, Serializable {
- Vector objects;
+public class DefaultComboBoxModel<E> extends AbstractListModel<E> implements MutableComboBoxModel<E>, Serializable {
+ Vector<E> objects;
Object selectedObject;
/**
* Constructs an empty DefaultComboBoxModel object.
*/
public DefaultComboBoxModel() {
- objects = new Vector();
+ objects = new Vector<E>();
}
/**
@@ -65,8 +67,8 @@
*
* @param items an array of Object objects
*/
- public DefaultComboBoxModel(final Object items[]) {
- objects = new Vector();
+ public DefaultComboBoxModel(final E items[]) {
+ objects = new Vector<E>();
objects.ensureCapacity( items.length );
int i,c;
@@ -84,7 +86,7 @@
*
* @param v a Vector object ...
*/
- public DefaultComboBoxModel(Vector<?> v) {
+ public DefaultComboBoxModel(Vector<E> v) {
objects = v;
if ( getSize() > 0 ) {
@@ -117,7 +119,7 @@
}
// implements javax.swing.ListModel
- public Object getElementAt(int index) {
+ public E getElementAt(int index) {
if ( index >= 0 && index < objects.size() )
return objects.elementAt(index);
else
@@ -136,7 +138,7 @@
}
// implements javax.swing.MutableComboBoxModel
- public void addElement(Object anObject) {
+ public void addElement(E anObject) {
objects.addElement(anObject);
fireIntervalAdded(this,objects.size()-1, objects.size()-1);
if ( objects.size() == 1 && selectedObject == null && anObject != null ) {
@@ -145,7 +147,7 @@
}
// implements javax.swing.MutableComboBoxModel
- public void insertElementAt(Object anObject,int index) {
+ public void insertElementAt(E anObject,int index) {
objects.insertElementAt(anObject,index);
fireIntervalAdded(this, index, index);
}
--- old/src/share/classes/javax/swing/JComboBox.java 2010-11-23 13:51:29.953125000 +0300
+++ new/src/share/classes/javax/swing/JComboBox.java 2010-11-23 13:51:28.937500000 +0300
@@ -69,6 +69,8 @@
* @see ComboBoxModel
* @see DefaultComboBoxModel
*
+ * @param <E> the type of the elements of this combo box
+ *
* @beaninfo
* attribute: isContainer false
* description: A combination of a text field and a drop-down list.
@@ -76,7 +78,7 @@
* @author Arnaud Weber
* @author Mark Davidson
*/
-public class JComboBox extends JComponent
+public class JComboBox<E> extends JComponent
implements ItemSelectable,ListDataListener,ActionListener, Accessible {
/**
* @see #getUIClassID
@@ -91,7 +93,7 @@
* @see #getModel
* @see #setModel
*/
- protected ComboBoxModel dataModel;
+ protected ComboBoxModel<? extends E> dataModel;
/**
* This protected field is implementation specific. Do not access directly
* or override. Use the accessor methods instead.
@@ -99,7 +101,7 @@
* @see #getRenderer
* @see #setRenderer
*/
- protected ListCellRenderer renderer;
+ protected ListCellRenderer<? super E> renderer;
/**
* This protected field is implementation specific. Do not access directly
* or override. Use the accessor methods instead.
@@ -156,7 +158,7 @@
*/
protected Object selectedItemReminder = null;
- private Object prototypeDisplayValue;
+ private E prototypeDisplayValue;
// Flag to ensure that infinite loops do not occur with ActionEvents.
private boolean firingActionEvent = false;
@@ -175,7 +177,7 @@
* displayed list of items
* @see DefaultComboBoxModel
*/
- public JComboBox(ComboBoxModel aModel) {
+ public JComboBox(ComboBoxModel<? extends E> aModel) {
super();
setModel(aModel);
init();
@@ -189,9 +191,9 @@
* @param items an array of objects to insert into the combo box
* @see DefaultComboBoxModel
*/
- public JComboBox(final Object items[]) {
+ public JComboBox(final E items[]) {
super();
- setModel(new DefaultComboBoxModel(items));
+ setModel(new DefaultComboBoxModel<E>(items));
init();
}
@@ -203,9 +205,9 @@
* @param items an array of vectors to insert into the combo box
* @see DefaultComboBoxModel
*/
- public JComboBox(Vector<?> items) {
+ public JComboBox(Vector<E> items) {
super();
- setModel(new DefaultComboBoxModel(items));
+ setModel(new DefaultComboBoxModel<E>(items));
init();
}
@@ -219,7 +221,7 @@
*/
public JComboBox() {
super();
- setModel(new DefaultComboBoxModel());
+ setModel(new DefaultComboBoxModel<E>());
init();
}
@@ -263,7 +265,7 @@
public void updateUI() {
setUI((ComboBoxUI)UIManager.getUI(this));
- ListCellRenderer renderer = getRenderer();
+ ListCellRenderer<? super E> renderer = getRenderer();
if (renderer instanceof Component) {
SwingUtilities.updateComponentTreeUI((Component)renderer);
}
@@ -302,8 +304,8 @@
* bound: true
* description: Model that the combo box uses to get data to display.
*/
- public void setModel(ComboBoxModel aModel) {
- ComboBoxModel oldModel = dataModel;
+ public void setModel(ComboBoxModel<? extends E> aModel) {
+ ComboBoxModel<? extends E> oldModel = dataModel;
if (oldModel != null) {
oldModel.removeListDataListener(this);
}
@@ -322,7 +324,7 @@
* @return the <code>ComboBoxModel</code> that provides the displayed
* list of items
*/
- public ComboBoxModel getModel() {
+ public ComboBoxModel<? extends E> getModel() {
return dataModel;
}
@@ -461,8 +463,8 @@
* expert: true
* description: The renderer that paints the item selected in the list.
*/
- public void setRenderer(ListCellRenderer aRenderer) {
- ListCellRenderer oldRenderer = renderer;
+ public void setRenderer(ListCellRenderer<? super E> aRenderer) {
+ ListCellRenderer<? super E> oldRenderer = renderer;
renderer = aRenderer;
firePropertyChange( "renderer", oldRenderer, renderer );
invalidate();
@@ -475,7 +477,7 @@
* @return the <code>ListCellRenderer</code> that displays
* the selected item.
*/
- public ListCellRenderer getRenderer() {
+ public ListCellRenderer<? super E> getRenderer() {
return renderer;
}
@@ -640,7 +642,7 @@
public int getSelectedIndex() {
Object sObject = dataModel.getSelectedItem();
int i,c;
- Object obj;
+ E obj;
for ( i=0,c=dataModel.getSize();i<c;i++ ) {
obj = dataModel.getElementAt(i);
@@ -658,7 +660,7 @@
* @see #setPrototypeDisplayValue
* @since 1.4
*/
- public Object getPrototypeDisplayValue() {
+ public E getPrototypeDisplayValue() {
return prototypeDisplayValue;
}
@@ -683,7 +685,7 @@
* attribute: visualUpdate true
* description: The display prototype value, used to compute display width and height.
*/
- public void setPrototypeDisplayValue(Object prototypeDisplayValue) {
+ public void setPrototypeDisplayValue(E prototypeDisplayValue) {
Object oldValue = this.prototypeDisplayValue;
this.prototypeDisplayValue = prototypeDisplayValue;
firePropertyChange("prototypeDisplayValue", oldValue, prototypeDisplayValue);
@@ -708,12 +710,12 @@
* }
* </pre>
*
- * @param anObject the Object to add to the list
+ * @param item the item to add to the list
* @see MutableComboBoxModel
*/
- public void addItem(Object anObject) {
+ public void addItem(E item) {
checkMutableComboBoxModel();
- ((MutableComboBoxModel)dataModel).addElement(anObject);
+ ((MutableComboBoxModel<E>)dataModel).addElement(item);
}
/**
@@ -721,14 +723,14 @@
* This method works only if the <code>JComboBox</code> uses a
* mutable data model.
*
- * @param anObject the <code>Object</code> to add to the list
+ * @param item the item to add to the list
* @param index an integer specifying the position at which
* to add the item
* @see MutableComboBoxModel
*/
- public void insertItemAt(Object anObject, int index) {
+ public void insertItemAt(E item, int index) {
checkMutableComboBoxModel();
- ((MutableComboBoxModel)dataModel).insertElementAt(anObject,index);
+ ((MutableComboBoxModel<E>)dataModel).insertElementAt(item,index);
}
/**
@@ -736,12 +738,12 @@
* This method works only if the <code>JComboBox</code> uses a
* mutable data model.
*
- * @param anObject the object to remove from the item list
+ * @param item the item to remove from the item list
* @see MutableComboBoxModel
*/
- public void removeItem(Object anObject) {
+ public void removeItem(E item) {
checkMutableComboBoxModel();
- ((MutableComboBoxModel)dataModel).removeElement(anObject);
+ ((MutableComboBoxModel<E>)dataModel).removeElement(item);
}
/**
@@ -756,7 +758,7 @@
*/
public void removeItemAt(int anIndex) {
checkMutableComboBoxModel();
- ((MutableComboBoxModel)dataModel).removeElementAt( anIndex );
+ ((MutableComboBoxModel<E>)dataModel).removeElementAt( anIndex );
}
/**
@@ -764,15 +766,15 @@
*/
public void removeAllItems() {
checkMutableComboBoxModel();
- MutableComboBoxModel model = (MutableComboBoxModel)dataModel;
+ MutableComboBoxModel<E> model = (MutableComboBoxModel<E>)dataModel;
int size = model.getSize();
if ( model instanceof DefaultComboBoxModel ) {
- ((DefaultComboBoxModel)model).removeAllElements();
+ ((DefaultComboBoxModel<E>)model).removeAllElements();
}
else {
for ( int i = 0; i < size; ++i ) {
- Object element = model.getElementAt( 0 );
+ E element = model.getElementAt( 0 );
model.removeElement( element );
}
}
@@ -1188,11 +1190,11 @@
private static class ComboBoxActionPropertyChangeListener
- extends ActionPropertyChangeListener<JComboBox> {
- ComboBoxActionPropertyChangeListener(JComboBox b, Action a) {
+ extends ActionPropertyChangeListener<JComboBox<?>> {
+ ComboBoxActionPropertyChangeListener(JComboBox<?> b, Action a) {
super(b, a);
}
- protected void actionPropertyChanged(JComboBox cb,
+ protected void actionPropertyChanged(JComboBox<?> cb,
Action action,
PropertyChangeEvent e) {
if (AbstractAction.shouldReconfigure(e)) {
@@ -1454,10 +1456,10 @@
*
* @param index an integer indicating the list position, where the first
* item starts at zero
- * @return the <code>Object</code> at that list position; or
+ * @return the item at that list position; or
* <code>null</code> if out of range
*/
- public Object getItemAt(int index) {
+ public E getItemAt(int index) {
return dataModel.getElementAt(index);
}
@@ -1491,11 +1493,11 @@
* @return an int equal to the selected row, where 0 is the
* first item and -1 is none.
*/
- int selectionForKey(char aKey,ComboBoxModel aModel);
+ int selectionForKey(char aKey,ComboBoxModel<?> aModel);
}
class DefaultKeySelectionManager implements KeySelectionManager, Serializable {
- public int selectionForKey(char aKey,ComboBoxModel aModel) {
+ public int selectionForKey(char aKey,ComboBoxModel<?> aModel) {
int i,c;
int currentSelection = -1;
Object selectedItem = aModel.getSelectedItem();
@@ -1616,7 +1618,7 @@
implements AccessibleAction, AccessibleSelection {
- private JList popupList; // combo box popup list
+ private JList<?> popupList; // combo box popup list
private Accessible previousSelectedAccessible = null;
/**
--- old/src/share/classes/javax/swing/MutableComboBoxModel.java 2010-11-23 13:51:35.765625000 +0300
+++ new/src/share/classes/javax/swing/MutableComboBoxModel.java 2010-11-23 13:51:34.609375000 +0300
@@ -30,16 +30,16 @@
* @author Tom Santos
*/
-public interface MutableComboBoxModel extends ComboBoxModel {
+public interface MutableComboBoxModel<E> extends ComboBoxModel<E> {
/**
* Adds an item at the end of the model. The implementation of this method
* should notify all registered <code>ListDataListener</code>s that the
* item has been added.
*
- * @param obj the <code>Object</code> to be added
+ * @param item the item to be added
*/
- public void addElement( Object obj );
+ public void addElement( E item );
/**
* Removes an item from the model. The implementation of this method should
@@ -55,17 +55,17 @@
* should notify all registered <code>ListDataListener</code>s that the
* item has been added.
*
- * @param obj the <code>Object</code> to be added
+ * @param item the item to be added
* @param index location to add the object
*/
- public void insertElementAt( Object obj, int index );
+ public void insertElementAt( E item, int index );
/**
* Removes an item at a specific index. The implementation of this method
* should notify all registered <code>ListDataListener</code>s that the
* item has been removed.
*
- * @param index location of object to be removed
+ * @param index location of the item to be removed
*/
public void removeElementAt( int index );
}
