EVALUATION

This fix introduced a regression.
6496144 REGRESSION: JCheckBox doesn't show on JToolBar under Windows L&F
                                     

EVALUATION

Taking a new approach:

First, remove the WindowsButtonListener and references to it. This is already handled internally by the BasicToolbarUI. Modify BasicToolBarUI to set the borders of toolbar button using the new getRolloverBorder(AbstractButton) and getNonRolloverBorder(AbstractButton) methods.  Then modify WindowsToolBarUI to override getRolloverBorder(AbstractButton) to return a border that comes out of XPStyle.getBorder(), using the appropriate button type.

To make the margins still work on toolbar buttons (as they do in basic) I have modified the XPEmptyBorder to ignore margins on the button if it's in a toolbar and use the hardcoded values, unless the margin is an instance of UIResource.

After this is approved I will file a CCC request for the changes to the Basic and the overridden methods in WindowsToolbarButtonUI.
                                     

EVALUATION

See also duplicate bug 6379094.
                                     

EVALUATION

Merged the bug 6234686 into this one since they have the same cause and are, arguably, the same bug (one refers to JButtons and the other to JToggleButtons).  The fix for this one will fix the other.
                                     

EVALUATION

Notes from java.net user loubs001 on the Swing forum:

Has anyone noticed that toolbar buttons in XP L&F are sized incorrectly? In fact, they're very wrong. Toolbar buttons, say with a 16x16 icon, should be square shaped, which they have been in previous versions of the L&F. But now they are rectangular, far too wide. This has been present since the early builds of Mustang. I guess I should have filed a bug report... I hope this gets fixed soon because not only does the toolbar look strange, but now the buttons are so wide that half of them dont fit in the window anymore.

Now when I report a bug I like to dig in to the Swing sources and find out whats causing it and suggest a fix. But if you're already aware of this problem you can probably skip the rest of this post, otherwise you can read on to hear my take on the subject.

Clearly what's happening is that the toolbar buttons are being sized to what they would be if they were normal buttons, which are rectangular. When buttons get an "acestor" event, XPStyle updates their border. It gives them an XPEmptyBorder with Insets obtained from the UXTheme API using SIZINGMARGINS. This seems like a good idea for truly theme-independant sizing.

For toolbar buttons this creates Insets as [4, 4, 4, 4]. I've checked and these are the correct values. XPEmptyBorder then adds 2 to each of those. Now, a toolbar button with an 6 pixel thick empty border looks great, just right. However these Insets never actually get used since XPEmptyBorder checks the button to see if it's getMargins() is non-null, and if so, uses those instead of the values from Windows. Well, those Margins are always non-null because BasicLookAndFeel sets them to [2, 14, 2, 14]. Obviously those margins were only meant for normal buttons and not toolbar buttons but XPEmptyBorder will use them regardless. Hence, really wide toolbar buttons.

So one solution, the obvious solution, would be to modify XPEmptyBorder so it ignores the Margins for toolbar buttons if they were set by BasicLookAndFeel, eg:

if (c instanceof AbstractButton) {
Insets m = ((AbstractButton)c).getMargin();
if(!(c.getParent() instanceof JToolBar &&
m instanceof InsetsUIResource)) {
margins = m;
}
}

The check for InsetsUIResource would allow the user to override the margins if they wanted, so it shouldnt break anything. So this seems like a good solution, and I was considering getting involved as a contributor and submitting this fix (though I'd prefer it if you guys did it).

But then I thought about it some more, and thought you might want want normal buttons to use native margins aswell, which are also being ignored in favour of Basic's. But unfortunately Windows gives these as (8, 9, 8, 9) which are obviously useless. SIZINGMARGINS are really meant for background stretching and have nothing to do with padding. CONTENTMARGINS on the other hand are given as (3, 3, 3, 3) which might be helpful if combined with margins for padding. But the CONTENTMARGINS for toolbar buttons for some reason are (0, 0, 0, 0) which is no good (they should be 4, 4, 4, 4) so they would need their own margins set separately and... *sigh* you can see how this starts to get complicated =P

I think it would be best to just implement my original simple fix for toolbar buttons and let normal buttons to continue using Basic's margins. They seem to work fine the way they are so why change them.

Anyway, whatever you decide to do, the important thing is that the toolbar buttons need to go back to being square. I hope this gets fixed soon. I'm concerned that if i just file a bug report, it will get put on the back of the queue and wont make it in time for Mustang.

Thanks guys.
                                     

EVALUATION

This bug is a regression from Tiger where JToggleButtons have borders/margins which are too big when they are placed inside of JToolBars. The source of the problem is that the margin is set when the button is created, at which time it's not inside of a toolbar. Once it is moved inside of a toolbar the WindowsButtonListener detects the change and resets the insets, however it's still using the old metrics so the final value hasn't changed. It needs to use the SizingMargins from the toolbar button section of the theme rather than the standard button.

To fix the issue I suggest creating a new value class called XPMarginValue which can retrieve the correct SizingMargins from the theme. We should also create a new UIDefault called ToolBarToggleButton.margin which will hold the correct margin value for the current theme (this will be an instance of XPMarginValue).  The WindowsButtonListener will be modified to update the margin correctly when a toggle button is moved in or out of a toolbar.  This will have a side benefit of making sure the value returned is from the current theme. The tiger code did not update the margin if you switched XP themes while running the java app.

I'm still not satisfied with this idea because there are some fishy explict values in the code. In particular the XPMarginValue needs to subtract -3 from each margin side to account for the extra +2 added in the XPStyle.XPEmptyBorder class. This is the only way to get metrics that match the existing Tiger metrics.  More research needs to be done before this is finalized.