JDK-6385366 : REGRESSION: Descriptive text and accelerator text overlap on JMenuItem.
  • Type: Bug
  • Component: client-libs
  • Sub-Component: javax.swing
  • Affected Version: 5.0,6
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • OS: windows_xp
  • CPU: x86
  • Submitted: 2006-02-14
  • Updated: 2011-02-16
  • Resolved: 2006-04-12
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 6
6 b80Fixed
Related Reports
Relates :  
Relates :  
Relates :  
Relates :  
Relates :  
Relates :  
Relates :  
Description
FULL PRODUCT VERSION :
java version "1.6.0-rc"
Java(TM) 2 Runtime Environment, Standard Edition (build 1.6.0-rc-b71)
Java HotSpot(TM) Client VM (build 1.6.0-rc-b71, mixed mode, sharing)

ADDITIONAL OS VERSION INFORMATION :
Microsoft Windows XP [Version 5.1.2600]

A DESCRIPTION OF THE PROBLEM :
Under certain circumstances the descriptive text and accelerator text overlap on JMenuItems.
This behaviour occurs in the following situation:
An entry earlier in the menu has an accelerator set on it (eg Ctrl+L) and no icon.
A later entry possesses an icon and the descriptive text is shorter than the previous entry.

The resulting width of the JMenu seems to completley ignore the extra size required by the accelerator text and overlaps it with the descriptive text.



REPRODUCIBILITY :
This bug can be reproduced always.

---------- BEGIN SOURCE ----------
import java.awt.*;
import java.awt.event.*;
import javax.swing.*;
public class MenuTest{
	public static void main(String[] args){
		int modifier = Toolkit.getDefaultToolkit().getMenuShortcutKeyMask();
		ImageIcon im = new ImageIcon(Toolkit.getDefaultToolkit().getImage("Document.gif"));
		
		JMenuItem shortitem1 = new JMenuItem("Short",im);
		JMenuItem longitem1 = new JMenuItem("Very long menu entry");
		longitem1.setAccelerator(KeyStroke.getKeyStroke(KeyEvent.VK_L,modifier));
		JMenu testmenu1 = new JMenu("Test1");
		testmenu1.add(shortitem1);
		testmenu1.add(longitem1);
		
		JMenuItem shortitem2 = new JMenuItem("Short",im);
		JMenuItem longitem2 = new JMenuItem("Very long menu entry");
		longitem2.setAccelerator(KeyStroke.getKeyStroke(KeyEvent.VK_L,modifier));
		JMenu testmenu2 = new JMenu("Test2");
		testmenu2.add(longitem2);
		testmenu2.add(shortitem2);
		
		JMenuItem shortitem3 = new JMenuItem("Short");
		JMenuItem longitem3 = new JMenuItem("Very long menu entry");
		longitem3.setAccelerator(KeyStroke.getKeyStroke(KeyEvent.VK_L,modifier));
		JMenu testmenu3 = new JMenu("Test3");
		testmenu3.add(longitem3);
		testmenu3.add(shortitem3);
		
		JMenuItem shortitem4 = new JMenuItem("Short",im);
		JMenuItem longitem4 = new JMenuItem("Very long menu entry",im);
		longitem4.setAccelerator(KeyStroke.getKeyStroke(KeyEvent.VK_L,modifier));
		JMenu testmenu4 = new JMenu("Test4");
		testmenu4.add(longitem4);
		testmenu4.add(shortitem4);
		
		JMenuItem shortitem5 = new JMenuItem("Short",im);
		JMenuItem longitem5 = new JMenuItem("Very long menu entry");
		shortitem5.setAccelerator(KeyStroke.getKeyStroke(KeyEvent.VK_S,modifier));
		longitem5.setAccelerator(KeyStroke.getKeyStroke(KeyEvent.VK_L,modifier));
		JMenu testmenu5 = new JMenu("Test5");
		testmenu5.add(longitem5);
		testmenu5.add(shortitem5);
		
		JMenuBar appmenu = new JMenuBar();
		appmenu.add(testmenu1);
		appmenu.add(testmenu2);
		appmenu.add(testmenu3);
		appmenu.add(testmenu4);
		appmenu.add(testmenu5);
		JFrame f = new JFrame();
		f.setDefaultCloseOperation(JFrame.DISPOSE_ON_CLOSE);
		f.setJMenuBar(appmenu);
		f.setVisible(true);
		f.pack();
		f.setLocationRelativeTo(null);
	}
}
---------- END SOURCE ----------

Release Regression From : 5.0u6
The above release value was the last known release where this 
bug was known to work. Since then there has been a regression.

Comments
SUGGESTED FIX Thanks to Peter Zheleznyakov for the good idea: to calculate maximal widths of icon, text and accelerator text in BasicMenuItemUI.getPreferredSize() and return just its sum. The latest (traversed by DefaultMenuLayout) menu item will have correct maximal widths and will be able to find the maximal width of the current menu item group and to return it as its preferred size. Parent popup menu will have the width based on the preferred width of the latest menu item. The new fix uses this idea: http://sa.sfbay.sun.com/projects/swing_data/mustang/6385366/
31-03-2006

SUGGESTED FIX --- /export/mlapshin/mustang/webrev/src/share/classes/javax/swing/plaf/basic/BasicMenuItemUI.java- Wed Mar 22 19:58:32 2006 +++ BasicMenuItemUI.java Wed Mar 22 18:11:54 2006 @@ -69,11 +69,12 @@ private static final boolean TRACE = false; // trace creates and disposes private static final boolean VERBOSE = false; // show reuse hits/misses private static final boolean DEBUG = false; // show bad params, misc. - /* Client Property keys for text and accelerator text widths */ + /* Client Property keys for icon, text and accelerator widths */ + static final String MAX_ICON_WIDTH = "maxIconWidth"; static final String MAX_TEXT_WIDTH = "maxTextWidth"; static final String MAX_ACC_WIDTH = "maxAccWidth"; /* Client Property keys for the icon and text maximal offsets */ static final StringBuffer MAX_ICON_OFFSET = @@ -430,52 +431,65 @@ b.getVerticalTextPosition(), b.getHorizontalTextPosition(), viewRect, iconRect, textRect, acceleratorRect, checkIconRect, arrowIconRect, text == null ? 0 : defaultTextIconGap, defaultTextIconGap ); - // find the union of the icon and text rects - r.setBounds(textRect); - r = SwingUtilities.computeUnion(iconRect.x, - iconRect.y, - iconRect.width, - iconRect.height, - r); - // r = iconRect.union(textRect); - - // To make the accelerator texts appear in a column, find the widest MenuItem text - // and the widest accelerator text. - //Get the parent, which stores the information. JComponent p = getMenuItemParent(menuItem); if(p != null) { - //Get widest text so far from parent, if no one exists null is returned. + r.height = (iconRect.height > textRect.height) + ? iconRect.height + : textRect.height; + + // To make the accelerator texts appear in a column, find the widest text, + // the widest icon and the widest accelerator text. + + // Get maximal widths from the parent client properties Integer maxTextWidth = (Integer) p.getClientProperty(BasicMenuItemUI.MAX_TEXT_WIDTH); Integer maxAccWidth = (Integer) p.getClientProperty(BasicMenuItemUI.MAX_ACC_WIDTH); - int maxTextValue = maxTextWidth!=null ? maxTextWidth.intValue() : 0; int maxAccValue = maxAccWidth!=null ? maxAccWidth.intValue() : 0; - - //Compare the text widths, and adjust the r.width to the widest. - if (r.width < maxTextValue) { - r.width = maxTextValue; + + // Walk through all parent's menu item children + // to find the maximal icon width. + int maxIconValue = getMaxIconWidth(p); + + r.width = maxIconValue; + // If there are icons, add the gap between icon and text + if (maxIconValue > 0) { + r.width += defaultTextIconGap; + } + + //Compare the text widths, and adjust the r.width to the widest. + if (textRect.width < maxTextValue) { + r.width += maxTextValue; } else { - p.putClientProperty(BasicMenuItemUI.MAX_TEXT_WIDTH, new Integer(r.width) ); + p.putClientProperty(BasicMenuItemUI.MAX_TEXT_WIDTH, new Integer(textRect.width)); + r.width += textRect.width; } - //Compare the accelarator widths. + //Compare the accelarator widths. if (acceleratorRect.width > maxAccValue) { maxAccValue = acceleratorRect.width; p.putClientProperty(BasicMenuItemUI.MAX_ACC_WIDTH, new Integer(acceleratorRect.width) ); } //Add on the widest accelerator r.width += maxAccValue; r.width += defaultTextIconGap; - } + } else { + // find the union of the icon and text rects + r.setBounds(textRect); + r = SwingUtilities.computeUnion(iconRect.x, + iconRect.y, + iconRect.width, + iconRect.height, + r); + } if( useCheckAndArrow() ) { // Add in the checkIcon r.width += checkIconRect.width; r.width += defaultTextIconGap; @@ -517,10 +531,45 @@ System.out.println("Current getSize: " + b.getSize() + "\n"); }*/ return r.getSize(); } + /** + * Finds the maximal icon width among the all parent's child menu items + * and stores it in the parent's BasicMenuItemUI.MAX_ICON_WIDTH + * client property. + * At first, attempts to get the stored value from the client property. + * @return Maximal icon width among the all parent's child menu items. + * If there are no icons returns 0. + * @since 1.6 + */ + private int getMaxIconWidth(JComponent parent) { + Integer storedMaxIconWidth = + (Integer) parent.getClientProperty(BasicMenuItemUI.MAX_ICON_WIDTH); + + if (storedMaxIconWidth != null) { + return storedMaxIconWidth; + } + + int maxWidth = 0; + int curWidth = 0; + for (Component comp : parent.getComponents()) { + if (comp instanceof AbstractButton) { + Icon icon = ((AbstractButton) comp).getIcon(); + if (icon != null) { + curWidth = icon.getIconWidth(); + if (curWidth > maxWidth) { + maxWidth = curWidth; + } + } + } + } + + parent.putClientProperty(BasicMenuItemUI.MAX_ICON_WIDTH, maxWidth); + return maxWidth; + } --- /export/mlapshin/mustang/webrev/src/share/classes/javax/swing/plaf/basic/DefaultMenuLayout.java- Wed Mar 22 19:58:32 2006 +++ DefaultMenuLayout.java Wed Mar 22 17:15:16 2006 @@ -29,10 +29,12 @@ } public Dimension preferredLayoutSize(Container target) { if (target instanceof JPopupMenu) { ((JPopupMenu)target).putClientProperty( + BasicMenuItemUI.MAX_ICON_WIDTH, null); + ((JPopupMenu)target).putClientProperty( BasicMenuItemUI.MAX_TEXT_WIDTH, null); ((JPopupMenu)target).putClientProperty( BasicMenuItemUI.MAX_ACC_WIDTH, null); ((JPopupMenu)target).putClientProperty( BasicMenuItemUI.MAX_ICON_OFFSET, null);
23-03-2006

EVALUATION The old algorithm of menu viewable width calculation is: > walk through all menu children: > ... > get MAX_TEXT_OFFSET from parent's client property (client properties are available during of the menu width calculation) > current text offset = MAX_ICON_OFFSET + ... > if current text offset greater than MAX_TEXT_OFFSET, store it as MAX_TEXT_OFFSET > preferredWidth = MAX_TEXT_OFFSET + ... > menu viewable width = preferred width of the longest menu item The traversal order of menu items is determined by BoxLayout. It is equal to the order of item addition to parent menu. If the widest menu item without an icon is traversed earlier than a shorter item with an icon, MAX_TEXT_OFFSET will be smaller (on the icon width) at the moment of preferred width calculation. Consequently, the longest item preferred width will be shorter than it should be. Menu viewable width will be shorter. The longest menu item will not be able to place the accelerator text in the short viewable box. It is the bug. To fix it, we should have the width of the widest icon among neighbor menu items at the moment of menu item preferred size calculation.
23-03-2006

EVALUATION I have found the cause: it lies in the BasicMenuItemUI.getPreferredMenuItemSize() method. It sometimes does not take in account the icon width of neighbour menu items for the widest menu item without icon.
22-03-2006

EVALUATION work in progress...
09-03-2006