Re: [cp-patches] Patch: BasicMenuItemUI fixes

[Lillian Angel]

On Tue, 2005-11-01 at 17:47 +0100, Roman Kennke wrote:
> Hi Lillian,
> 
> Am Dienstag, den 01.11.2005, 10:37 -0500 schrieb Lillian Angel:
> > Played around with the defaults a bit so BasicMenuItemUI initializes
> > checkIcon. I had to remove the checkIcon default from BasicL&F, so the
> > checkIcon is not drawn all the time.
> > 
> > 2005-11-01  Lillian Angel  <address@hidden>
> > 
> >         * javax/swing/plaf/basic/BasicCheckBoxMenuItemUI.java
> >         (BasicCheckBoxMenuItem): Moved code to 
> >     BasicMenuItemUI.installDefaults.
> >         (processMouseEvent): Added TODO comment. Not sure about the
> >         correct implementation yet.
> >         * javax/swing/plaf/basic/BasicLookAndFeel.java
> >         (initComponentDefaults): Removed MenuItem.checkIcon default. 
> >     Only RadioButtonMenuItem's and CheckBoxMenuItem's have 
> >     checkIcons.
> 
> I disagree. My little UIDefaultsInspector tool reveals the following for
> MenuItem.checkIcon:
> 
> address@hidden:~/src/test$ java UIDefaultsInspector MyBasicLookAndFeel
> |grep MenuItem.checkIcon
> Key: CheckBoxMenuItem.checkIcon  Value:
> address@hidden,
> Type: javax.swing.plaf.basic.BasicIconFactory$CheckBoxMenuItemIcon
> Key: MenuItem.checkIcon  Value: javax.swing.plaf.basic.BasicIconFactory
> address@hidden, Type:
> javax.swing.plaf.basic.BasicIconFactory$MenuItemCheckIcon
> Key: RadioButtonMenuItem.checkIcon       Value:
> address@hidden,
> Type: javax.swing.plaf.basic.BasicIconFactory$RadioButtonMenuItemIcon
> 
> As you can see, the MenuItem.checkIcon is ok. This also backs up my
> claim that the checkIcon should really be loaded in BasicMenuItemUI with
> the propertyPrefix. And why else would we need a
> BasicIconFactory.getMenuItemCheckIcon() method?

I agree, and this is the reason I said it would not work. The painting
of the checkIcon happens when checkIcon is not null. If we are always
initializing it to something in BasicMenuItemUI, then the checkIcon will
_always_ be drawn

> 
> I am also quite sure that all the UIDefaults have mauve tests, so a
> removal of a UIDefault would cause a regression...
> 
> /Roman
>