[cp-patches] FYI: ComboBox fixes

[David Gilbert]

This patch (committed) fixes most of the failing Mauve tests for the combo box 
classes:

2005-10-19  David Gilbert  <address@hidden>

        * javax/swing/plaf/basic/BasicArrowButton.java
        (buttonBorder): removed,
        (BasicArrowButton): don't set border, use default border,
        * javax/swing/plaf/basic/BasicComboBoxUI.java
        (configureArrowButton): don't set margin here,
        (getPreferredSize): added comment,
        (getMinimumSize): icon width is the same as the display area height,
        (getDefaultSize): return size of space char plus 1 pixel margin,
        (getDisplaySize): take into account the prototype display value, if
        there is one,
        * javax/swing/plaf/basic/BasicLookAndFeel.java
        (initComponentDefaults): updated 'Button.margin' default,
        * javax/swing/plaf/metal/MetalComboBoxButton.java
        (MetalComboBoxButton(JComboBox, Icon, boolean, CellRendererPane,
        JList)): check for null JComboBox, don't set margin here,
        * javax/swing/plaf/metal/MetalComboBoxUI.java
        (createArrowButton): set margin,
        (getMinimumSize): reimplemented,
        * javax/swing/plaf/metal/MetalLookAndFeel.java
        (initComponentDefaults): changed 'Button.margin' default to from Insets
        to InsetsUIResource.

Regards,

Dave

Index: javax/swing/plaf/basic/BasicArrowButton.java
===================================================================
RCS file: 
/cvsroot/classpath/classpath/javax/swing/plaf/basic/BasicArrowButton.java,v
retrieving revision 1.14
diff -u -r1.14 BasicArrowButton.java
--- javax/swing/plaf/basic/BasicArrowButton.java        19 Sep 2005 16:53:46 
-0000      1.14
+++ javax/swing/plaf/basic/BasicArrowButton.java        19 Oct 2005 14:28:10 
-0000
@@ -98,48 +98,6 @@
    */
   transient Color highlight = Color.WHITE;
 
-  /** The border around the ArrowButton. */
-  private transient Border buttonBorder = new Border()
-    {
-      public Insets getBorderInsets(Component c)
-      {
-       return new Insets(2, 2, 2, 2);
-      }
-
-      public boolean isBorderOpaque()
-      {
-       return true;
-      }
-
-      public void paintBorder(Component c, Graphics g, int x, int y, int w,
-                              int h)
-      {
-        Color saved = g.getColor();
-        AbstractButton b = (AbstractButton) c;
-        if (b.getModel().isPressed())
-          {
-            g.setColor(darkShadow);
-            g.drawRect(x, y, x + w - 1, y + h - 1);
-          }
-        else
-          {
-            g.setColor(highlight);
-            g.drawLine(x + 1, y + 1, x + w - 3, y + 1);
-            g.drawLine(x + 1, y + 1, x + 1, y + h - 2);
-
-            g.setColor(shadow);
-            g.drawLine(x + 1, y + h - 2, x + w - 1, y + h - 2);
-            g.drawLine(x + w - 2, y + 1, x + w - 2, y + h - 2);
-
-            g.setColor(darkShadow);
-            g.drawLine(x, y + h - 1, x + w - 1, y + h - 1);
-            g.drawLine(x + w - 1, y, x + w - 1, y + h - 1);
-
-            g.setColor(saved);
-          }
-      }
-    };
-
   /**
    * Creates a new BasicArrowButton object.
    *
@@ -148,7 +106,6 @@
   public BasicArrowButton(int direction)
   {
     super();
-    setBorder(buttonBorder);
     setDirection(direction);
   }
 
Index: javax/swing/plaf/basic/BasicComboBoxUI.java
===================================================================
RCS file: 
/cvsroot/classpath/classpath/javax/swing/plaf/basic/BasicComboBoxUI.java,v
retrieving revision 1.25
diff -u -r1.25 BasicComboBoxUI.java
--- javax/swing/plaf/basic/BasicComboBoxUI.java 18 Oct 2005 15:27:55 -0000      
1.25
+++ javax/swing/plaf/basic/BasicComboBoxUI.java 19 Oct 2005 14:28:11 -0000
@@ -43,6 +43,7 @@
 import java.awt.Container;
 import java.awt.Dimension;
 import java.awt.Font;
+import java.awt.FontMetrics;
 import java.awt.Graphics;
 import java.awt.Insets;
 import java.awt.LayoutManager;
@@ -556,7 +557,6 @@
   {
     arrowButton.setEnabled(comboBox.isEnabled());
     arrowButton.setFont(comboBox.getFont());
-    arrowButton.setMargin(new Insets(0, 0, 0, 0));
   }
 
   /**
@@ -651,6 +651,9 @@
    */
   public Dimension getPreferredSize(JComponent c)
   {
+    // note:  overriding getMinimumSize() (for example in the MetalComboBoxUI 
+    // class) affects the getPreferredSize() result, so it seems logical that
+    // this method is implemented by delegating to the getMinimumSize() method
     return getMinimumSize(c);
   }
 
@@ -665,9 +668,8 @@
   public Dimension getMinimumSize(JComponent c)
   {
     Dimension d = getDisplaySize();
-    Dimension arrowDim = arrowButton.getPreferredSize();
-    Dimension result = new Dimension(d.width + arrowDim.width, 
-            Math.max(d.height, arrowDim.height));
+    int arrowButtonWidth = d.height;
+    Dimension result = new Dimension(d.width + arrowButtonWidth, d.height);
     return result;
   }
 
@@ -828,15 +830,24 @@
   }
 
   /**
-   * Returns default size for the combo box that doesn't contain any elements
-   * in it
+   * Returns the default size for the display area of a combo box that does 
+   * not contain any elements.  This method returns the width and height of
+   * a single space in the current font, plus a margin of 1 pixel. 
    *
-   * @return Default size of the combo box with no elements in it.
+   * @return The default display size.
+   * 
+   * @see #getDisplaySize()
    */
   protected Dimension getDefaultSize()
   {
-    // FIXME: Not implemented properly.
-    return new Dimension(100, 5);
+    // There is nothing in the spec to say how this method should be
+    // implemented...so I've done some guessing, written some Mauve tests,
+    // and written something that gives dimensions that are close to the 
+    // reference implementation.
+    FontMetrics fm = comboBox.getFontMetrics(comboBox.getFont());
+    int w = fm.charWidth(' ') + 2;
+    int h = fm.getHeight() + 2;
+    return new Dimension(w, h);
   }
 
   /**
@@ -847,40 +858,52 @@
    */
   protected Dimension getDisplaySize()
   {
-    ComboBoxModel model = comboBox.getModel();
-    int numItems = model.getSize();
-
-    // if combo box doesn't have any items then simply
-    // return its default size
-    if (numItems == 0)
+    Object prototype = comboBox.getPrototypeDisplayValue();
+    if (prototype != null)
       {
-       displaySize = getDefaultSize();
-       return displaySize;
+        // calculate result based on prototype
+        ListCellRenderer renderer = comboBox.getRenderer();
+        Component comp = renderer.getListCellRendererComponent(listBox, 
+                prototype, -1, false, false);
+        Dimension compSize = comp.getPreferredSize();
+        compSize.width += 2;  // add 1 pixel margin around area
+        compSize.height += 2;
+        return compSize;
       }
+    else
+      {
+        ComboBoxModel model = comboBox.getModel();
+        int numItems = model.getSize();
+
+        // if combo box doesn't have any items then simply
+        // return its default size
+        if (numItems == 0)
+          {
+            displaySize = getDefaultSize();
+            return displaySize;
+          }
 
-    Dimension size = new Dimension(0, 0);
+        Dimension size = new Dimension(0, 0);
 
-    // ComboBox's display size should be equal to the 
-    // size of the largest item in the combo box. 
-    ListCellRenderer renderer = comboBox.getRenderer();
-
-    // FIXME: use the JComboBox.getPrototypeDisplayValue() if there is
-    // one
-    for (int i = 0; i < numItems; i++)
-      {
-        Object item = model.getElementAt(i);
-        String s = item.toString();
-        Component comp = renderer.getListCellRendererComponent(listBox, item,
-            -1, false, false);
+        // ComboBox's display size should be equal to the 
+        // size of the largest item in the combo box. 
+        ListCellRenderer renderer = comboBox.getRenderer();
 
-        Dimension compSize = comp.getPreferredSize();
-        if (compSize.width > size.width)
-          size.width = compSize.width;
-        if (compSize.height > size.height)
-          size.height = compSize.height;
+        for (int i = 0; i < numItems; i++)
+          {
+            Object item = model.getElementAt(i);
+            Component comp = renderer.getListCellRendererComponent(listBox, 
+                    item, -1, false, false);
+
+            Dimension compSize = comp.getPreferredSize();
+            if (compSize.width + 2 > size.width)
+              size.width = compSize.width + 2;
+            if (compSize.height + 2 > size.height)
+              size.height = compSize.height + 2;
+          }
+        displaySize = size;
+        return displaySize;
       }
-    displaySize = size;
-    return displaySize;
   }
 
   /**
Index: javax/swing/plaf/basic/BasicLookAndFeel.java
===================================================================
RCS file: 
/cvsroot/classpath/classpath/javax/swing/plaf/basic/BasicLookAndFeel.java,v
retrieving revision 1.63
diff -u -r1.63 BasicLookAndFeel.java
--- javax/swing/plaf/basic/BasicLookAndFeel.java        18 Oct 2005 17:48:03 
-0000      1.63
+++ javax/swing/plaf/basic/BasicLookAndFeel.java        19 Oct 2005 14:28:13 
-0000
@@ -273,7 +273,7 @@
       "Button.foreground", new ColorUIResource(Color.BLACK),
       "Button.highlight", new ColorUIResource(Color.WHITE),
       "Button.light", new ColorUIResource(Color.LIGHT_GRAY),
-      "Button.margin", new InsetsUIResource(2, 2, 2, 2),
+      "Button.margin", new InsetsUIResource(2, 14, 2, 14),
       "Button.shadow", new ColorUIResource(Color.GRAY),
       "Button.textIconGap", new Integer(4),
       "Button.textShiftOffset", new Integer(0),
Index: javax/swing/plaf/metal/MetalComboBoxButton.java
===================================================================
RCS file: 
/cvsroot/classpath/classpath/javax/swing/plaf/metal/MetalComboBoxButton.java,v
retrieving revision 1.5
diff -u -r1.5 MetalComboBoxButton.java
--- javax/swing/plaf/metal/MetalComboBoxButton.java     17 Oct 2005 10:02:52 
-0000      1.5
+++ javax/swing/plaf/metal/MetalComboBoxButton.java     19 Oct 2005 14:28:14 
-0000
@@ -100,12 +100,13 @@
       CellRendererPane pane, JList list)
   {
     super();
+    if (cb == null)
+      throw new NullPointerException("Null 'cb' argument");
     comboBox = cb;
     comboIcon = i;
     iconOnly = onlyIcon;
     listBox = list;
     rendererPane = pane;
-    setMargin(new Insets(0, 0, 0, 0));
   }
   
   /**
Index: javax/swing/plaf/metal/MetalComboBoxUI.java
===================================================================
RCS file: 
/cvsroot/classpath/classpath/javax/swing/plaf/metal/MetalComboBoxUI.java,v
retrieving revision 1.6
diff -u -r1.6 MetalComboBoxUI.java
--- javax/swing/plaf/metal/MetalComboBoxUI.java 19 Oct 2005 13:46:02 -0000      
1.6
+++ javax/swing/plaf/metal/MetalComboBoxUI.java 19 Oct 2005 14:28:14 -0000
@@ -41,6 +41,7 @@
 import java.awt.Container;
 import java.awt.Dimension;
 import java.awt.Graphics;
+import java.awt.Insets;
 import java.awt.LayoutManager;
 import java.awt.Rectangle;
 import java.awt.event.MouseEvent;
@@ -49,6 +50,7 @@
 
 import javax.swing.CellRendererPane;
 import javax.swing.ComboBoxEditor;
+import javax.swing.Icon;
 import javax.swing.JButton;
 import javax.swing.JComboBox;
 import javax.swing.JComponent;
@@ -210,8 +212,10 @@
    */
   protected JButton createArrowButton()
   {
-    return new MetalComboBoxButton(comboBox, new MetalComboBoxIcon(), 
+    JButton button = new MetalComboBoxButton(comboBox, new 
MetalComboBoxIcon(), 
             new CellRendererPane(), listBox);  
+    button.setMargin(new Insets(0, 1, 1, 3));
+    return button;
   }
   
   /**
@@ -291,10 +295,15 @@
    */
   public Dimension getMinimumSize(JComponent c)
   {
-    // FIXME: this needs work
-    Dimension result = super.getMinimumSize(c);
-    result.height = result.height + 9;
-    return result;   
+    MetalComboBoxButton b = (MetalComboBoxButton) arrowButton;
+    Icon icon = b.getComboIcon();
+    Insets insets = b.getInsets();
+    Dimension d = getDisplaySize();
+    int insetsH = insets.top + insets.bottom;
+    int insetsW = insets.left + insets.right;
+    int iconWidth = icon.getIconWidth() + 6;
+    return new Dimension(d.width + insetsW + iconWidth, 
+            d.height + insetsH);
   }
   
 }
Index: javax/swing/plaf/metal/MetalLookAndFeel.java
===================================================================
RCS file: 
/cvsroot/classpath/classpath/javax/swing/plaf/metal/MetalLookAndFeel.java,v
retrieving revision 1.66
diff -u -r1.66 MetalLookAndFeel.java
--- javax/swing/plaf/metal/MetalLookAndFeel.java        17 Oct 2005 10:02:52 
-0000      1.66
+++ javax/swing/plaf/metal/MetalLookAndFeel.java        19 Oct 2005 14:28:16 
-0000
@@ -781,7 +781,7 @@
       "Button.foreground", getControlTextColor(),
       "Button.highlight", getControlHighlight(),
       "Button.light", getControlHighlight(),
-      "Button.margin", new Insets(2, 14, 2, 14),
+      "Button.margin", new InsetsUIResource(2, 14, 2, 14),
       "Button.select", getControlShadow(),
       "Button.shadow", getControlShadow(),