RFR: 8373650: Test javax/swing/JMenuItem/6458123/ManualBug6458123.java fails because the check icons are not aligned properly as expected [v2]

[Prasanta Sadhukhan]

On Fri, 2 Jan 2026 19:48:55 GMT, Alexey Ivanov <aivanov at openjdk.org> wrote:

>> Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Alignment fix
>
> src/java.desktop/windows/classes/com/sun/java/swing/plaf/windows/WindowsIconFactory.java line 922:
> 
>> 920:                                                 x + OFFSET,
>> 921:                                                 (icon.getIconHeight() <= 16) ? y + OFFSET : (y + icon.getIconHeight() / 2), state);
>> 922:                                     } else if (icon.getIconWidth() <= 16) {
> 
> I don't understand why painting an icon and check marks / radio bullets in RTL case depends on the width of the icon but there's no such dependency in LTR case.
> 
> Shouldn't the performed menu layout handle these cases?

As I see, MenuItemLayout handler's x position calculation for RTL depends on icon rect width
https://github.com/openjdk/jdk/blob/6eaabed55ca4670d8c317f0a4323ccea4dd0b9ca/src/java.desktop/share/classes/sun/swing/MenuItemLayoutHelper.java#L737-L741
but LTR doesn't depend on rect width
https://github.com/openjdk/jdk/blob/6eaabed55ca4670d8c317f0a4323ccea4dd0b9ca/src/java.desktop/share/classes/sun/swing/MenuItemLayoutHelper.java#L726-L730

> src/java.desktop/windows/classes/com/sun/java/swing/plaf/windows/WindowsIconFactory.java line 923:
> 
>> 921:                                                 (icon.getIconHeight() <= 16) ? y + OFFSET : (y + icon.getIconHeight() / 2), state);
>> 922:                                     } else if (icon.getIconWidth() <= 16) {
>> 923:                                         if ((c instanceof JMenuItem mi) && mi.getText().isEmpty()) {
> 
> The condition `(c instanceof JMenuItem mi)` is always `true` according to the assert at line 881:
> 
> https://github.com/openjdk/jdk/blob/2daf12edd24e641d4d7706d582994c2b3fe95e87/src/java.desktop/windows/classes/com/sun/java/swing/plaf/windows/WindowsIconFactory.java#L881
> 
> where `menuItem` is a field in `VistaMenuItemCheckIcon`, the type of the field is `JMenuItem`.
> 
> This means, you can use `menuItem` and ignore the passed in parameter `c`.

ok

> src/java.desktop/windows/classes/com/sun/java/swing/plaf/windows/WindowsIconFactory.java line 936:
> 
>> 934:                                             skin.paintSkin(g,
>> 935:                                                 (type == JRadioButtonMenuItem.class) ? (x + 3 * OFFSET) : (x + 4 * OFFSET),
>> 936:                                                 (icon.getIconHeight() <= 16) ? y + OFFSET : (y + icon.getIconHeight() / 2), state);
> 
> I wonder why painting the skin for radio button menu requires such a fix up whereas it's not required for `JCheckBoxMenuItem`.

it is taken care via ternary operator, width/spread of check mark is usually more than radio bullet so starting point of checkmark is considered a little ahead compared to radio bullet

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/28889#discussion_r2660479443
PR Review Comment: https://git.openjdk.org/jdk/pull/28889#discussion_r2660478835
PR Review Comment: https://git.openjdk.org/jdk/pull/28889#discussion_r2660480432