8348760: RadioButton is not shown if JRadioButtonMenuItem is rendered with ImageIcon in WindowsLookAndFeel #23324

prsadhuk · 2025-01-28T03:40:26Z

When JRadioButtonMenuItem is called with imageIcon, then only imageIcon is shown without radiobutton in WIndowsLookAndFeel as there was no provision of drawing the radiobutton alongside icon.
If icon is not there, the radiobutton is drawn. Added provision of drawing the radiobutton windows Skin even when imageIcon is present.

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8348760: RadioButton is not shown if JRadioButtonMenuItem is rendered with ImageIcon in WindowsLookAndFeel (Bug - P3)

Reviewers

Abhishek Kumar (@kumarabhi006 - Reviewer) 🔄 Re-review required (review applies to ba7ecae3)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/23324/head:pull/23324
$ git checkout pull/23324

Update a local copy of the PR:
$ git checkout pull/23324
$ git pull https://git.openjdk.org/jdk.git pull/23324/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 23324

View PR using the GUI difftool:
$ git pr show -t 23324

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/23324.diff

Using Webrev

Link to Webrev Comment

… with ImageIcon in WindowsLookAndFeel

bridgekeeper · 2025-01-28T03:41:15Z

👋 Welcome back psadhukhan! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2025-01-28T03:41:38Z

@prsadhuk This change is no longer ready for integration - check the PR body for details.

openjdk · 2025-01-28T03:42:13Z

@prsadhuk The following label will be automatically applied to this pull request:

client

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

mlbridge · 2025-01-28T03:47:30Z

Webrevs

src/java.desktop/windows/classes/com/sun/java/swing/plaf/windows/WindowsIconFactory.java

test/jdk/javax/swing/JMenuItem/TestImageIconWithJRadioButtonMenuItem.java

src/java.desktop/windows/classes/com/sun/java/swing/plaf/windows/WindowsIconFactory.java

test/jdk/javax/swing/JMenuItem/TestImageIconWithJRadioButtonMenuItem.java

test/jdk/javax/swing/JMenuItem/duke.gif

kumarabhi006 · 2025-01-30T08:21:13Z

@prsadhuk Unable to see the duke image along with RadioButtonMenuItem. Is it correct behaviour?

prsadhuk · 2025-01-30T08:27:02Z

@prsadhuk Unable to see the duke image along with RadioButtonMenuItem. Is it correct behaviour?

did you download the duke file from the PR?

kumarabhi006 · 2025-01-30T08:32:10Z

@prsadhuk Unable to see the duke image along with RadioButtonMenuItem. Is it correct behaviour?

did you download the duke file from the PR?

Yeah, it's inside "javax\swing\JMenuItem" folder.

src/java.desktop/windows/classes/com/sun/java/swing/plaf/windows/WindowsIconFactory.java

test/jdk/javax/swing/JMenuItem/TestImageIconWithJRadioButtonMenuItem.java

kumarabhi006 · 2025-01-30T08:52:28Z

@prsadhuk After image path correction, duke image icon is visible.

src/java.desktop/windows/classes/com/sun/java/swing/plaf/windows/WindowsIconFactory.java

kumarabhi006

Looks good to me.

aivanov-jdk

I am also for generating an ImageIcon on the fly to avoid adding a binary file.

src/java.desktop/windows/classes/com/sun/java/swing/plaf/windows/WindowsIconFactory.java

aivanov-jdk · 2025-01-30T12:55:45Z

This does not look right at all:

You shouldn't render an unselected radio button in menu.

(Yes, I modified your test case to display the menu items in a popup menu as they're supposed to be.)

The position of the image also seems off. On Windows 10, the selected state has a background which you can see on the screenshot. Without the fix, the duke is centered in this selected background; but with the fix, it's off.

If we add support for painting both an ImageIcon and the bullet / check-mark, the layout of the menu item has to change.

prsadhuk · 2025-01-30T13:18:25Z

You shouldn't render an unselected radio button in menu.

Please check updated PR and share your testcase

aivanov-jdk · 2025-01-30T13:40:32Z

You shouldn't render an unselected radio button in menu.

Please check updated PR…

I saw your update. I wrote this comment before you removed rendering the unselected radio button.

…share your testcase

Here's the latest version of modified TestImageIconWithJRadioButtonMenuItem.java.

The diff between yours and mine.

aivanov-jdk · 2025-01-30T14:57:33Z

Based on the discussion above in relation to JDK-6432667 where the current behaviour/rendering was implemented, this quirk may be a feature of Windows L&F rather than a bug.

Windows 10 has a highlight around the selected icon. Yet there's no such highlight in Windows 11, therefore it's hard to distinguish whether a menu item has a selected state of not.

prrace · 2025-05-07T18:20:42Z

I refined the example program and submitted a support case to Microsoft.
I'll attach to the JBS issue that version of the test case (rbm.cpp) and then
one (rbma.cpp) showing where we've got to.

First there are several elements to understand about menu items
https://learn.microsoft.com/en-us/windows/win32/api/winuser/ns-winuser-menuiteminfoa

(1) The menu item can have a string as the "item" (dwTypeData)
(2) The menu item can have a bitmap as the "item" (hbmpItem)

Historically you could only have ONE of these because the field used to specify them is the same. Now by using the new MIIM* values you can have both. And - I don't quite understand this - using the old MFT_STRING didn't seem to work at all for me I had to use MIIM_STRING.

As well as those you can specify the images to use for the check state

hbmpChecked - if the item is selected
hbmpUnchecked - if it is not selected.

Some questions and some answers with Microsoft

Q. Why is the highlighting background gone in Windows 11 ?

A. That background was part of the “Reveal” lighting effect used by the old Fluent-Design menus in Windows 10. As of WinUI 2.6 (the style set shipped with Windows 11) the UI team removed that effect across context-menus and other pop-ups to achieve a flatter look that meets new contrast guidelines. So it’s an intentional theme change, not a GDI regression.

Q. Why does the item image draw over the default radio button. How can this be anything except a bug ?

A. The Win32 menu engine draws the default bullet/checkmark at X = 0 unless you reserve a check-mark column by supplying hbmpChecked (and optionally hbmpUnchecked).
When an hbmpItem is also present, both bitmaps occupy the same origin, so the icon ends up covering the bullet. This rule has been in place since the Windows 9x/NT days—older SDK samples illustrate the same overlap if you omit the custom check bitmaps. Because the behaviour is longstanding and documented indirectly (see the remarks under SetMenuItemBitmaps and the SM_CXMENUCHECK metric), the shell team classifies it as by design rather than a bug.

Q. Why isn't space automatically left for the default rendering of the checkmark ? Another bug ?

A. For historical reasons the menu layout engine only allocates the check-mark column when it has a concrete size for that column, i.e. when hbmpChecked is non-NULL. If you ask for the default glyph while also providing an item icon, Windows assumes you are handling layout yourself. The docs don’t call this out explicitly (agreed, that would be helpful), but the behaviour is consistent and the same on every supported version of Windows, so it is not tracked as a GDI defect.

So .. in summary, two things that have been a bug so long they are considered a feature are at the root of this and the visual "cue" we've relied on for in Windows 10 was just an accident of some theming that is now gone.

So GDI really does make it hard to have all 3 of these. And maybe we should just not support it ?

But if (as GDI programmers) we want an item string and bitmap, and expect feedback that an item is selected, we seem to be forced into needing to specify bitmaps for the checked and un-checked state. But then you lose the default rendering of the bullet that Windows provides !

There's some hoops we can jump through to get close

Use a transparent bitmap for hbmpUnchecked
Use a GDI API - DrawFrameControl - to render a bullet image into a bitmap we use for hbmpChecked. Prasanta previously noted that AWT uses this API already)

rbma.cpp illustrates this.

This still isn't perfect - the bullet is too large.

I'll leave it to others to figure out how we make use of this in the Swing rendering case.
Are there any unanswered questions ? This is a long PR so perhaps ..

prsadhuk · 2025-05-26T09:36:03Z

I checked rbma.cpp output where I think we can only support the 4th item in the list (which is this)

This is because we have JRadioButtonMenuItem(String text, Icon icon, boolean selected)
API which only supports one icon image (so checked/unchecked icon image in addition to item icon as below image is not possible in my opinion in Java swing)

The current state of PR supports this 4th item.
Only thing different is when there is no imageicon and menuitem is selected, then the text seems to be in same vertical line as the other items (in native rbma output)

but current PR the text is shifted to left (when there is no imageicon ie 3rd item in list) and occupies icon space, as below

This is because in most cases (and in SwingSet2) the radiomenu items are drawn without imageicon and there is no extra space between radio bullet and menu text

so if we cater to text shifting to same vertical line, there will be difference in behavior w.r.t existing one and user will notice a shift in text to accomodate non-existing image icon)

but it is fixable,
so if we are in consensus about supporting the 4th item in rbma.cpp output in Java swing with text in same vertical offset, then let mw know..Anything else in addition needs discussion whether we can support...

prrace · 2025-05-27T23:10:11Z

I checked rbma.cpp output where I think we can only support the 4th item in the list (which is this)

This is because we have JRadioButtonMenuItem(String text, Icon icon, boolean selected) API which only supports one icon image (so checked/unchecked icon image in addition to item icon as below image is not possible in my opinion in Java swing)

Not sure I know what you mean by 4th item, but clearly we aren't going to add an API to supply checked/unchecked images.
But we should support rendering like you show above.

app supplies icon image and text, WE supply the radio button.

The current state of PR supports this 4th item.

What is "this 4th item" ? You mean the case of image + text from app, and WE display a radio button checked ?

Only thing different is when there is no imageicon and menuitem is selected, then the text seems to be in same vertical line as the other items (in native rbma output)

but current PR the text is shifted to left (when there is no imageicon ie 3rd item in list) and occupies icon space, as below !

If you mean windows always lines up the start of the text, that seems to be true.
So we should do the same.

image

This is because in most cases (and in SwingSet2) the radiomenu items are drawn without imageicon and there is no extra space between radio bullet and menu text

it seems to me that if there's an image in ANY menu item, Windows reserves the space in all items, hence getting the text aligned.
It just does the funky thing of moving where it displays the item image if there's no checked image.

so if we cater to text shifting to same vertical line, there will be difference in behavior w.r.t existing one and user will notice a shift in text to accomodate non-existing image icon)

You mean user will notice text is now aligned ? I think that's fine.

but it is fixable, so if we are in consensus about supporting the 4th item in rbma.cpp output in Java swing with text in same vertical offset, then let mw know..Anything else in addition needs discussion whether we can support...

I hope (!) we are on the same page. Let me try and say it so we are ..

WE will always draw a check (tick) or radio button to show the selected item
WE will always reserve space for that tick
WE will never over-write the check/tick with the icon image.
Icon images will never move (unlike the funky windows case which is the root of all of this) because we will always reserve space for it.
If ANY item has an icon image, ALL items reserve the space, even if not used, this is needed for the next item :
Text strings will all align. Always

prsadhuk · 2025-05-28T03:38:09Z

I hope (!) we are on the same page. Let me try and say it so we are ..
* WE will always draw a check (tick)  or radio button to show the selected item

Yes, current PR does this..

* WE will always reserve space for that tick

Yes, current PR does this.

* WE will never over-write the check/tick with the icon image.

Yes, current PR does this.

* Icon images will never move  (unlike the funky windows case which is the root of all of this)  because we will always reserve space for it.

Yes, icon image has its own space in current PR

* If ANY item has an icon image, ALL items reserve the space, even if not used, this is needed for the next item :

* Text strings will all align. Always

Current PR doesnt have this..Text string is not aligned for item that doesn't have iconimage with item that does have iconimage. I will fix this.

…48760

prsadhuk · 2025-05-28T05:47:06Z

Text alignment fixed .. Current PR now looks like this

In SwingSet2, now there will be gap between bullet and text to accomodate non-existing imageicon

prrace · 2025-06-18T21:21:35Z

In SwingSet2, now there will be gap between bullet and text to accomodate non-existing imageicon

That gap should only be there if at least one menu item has an image icon. It shoudn't be there if none have an icon.

prsadhuk · 2025-06-19T06:06:23Z

In SwingSet2, now there will be gap between bullet and text to accomodate non-existing imageicon

That gap should only be there if at least one menu item has an image icon. It shoudn't be there if none have an icon.

That will require to find out if current menuItem is part of ButtonGroup and then reiterate each menuitems of the ButtonGroup and see if anyone has an imageicon..It does not seem possible to find out from current menuItem if it part of ButtonGroup as that info is not passed to each MenuItemUI (ButtonGroup info is only known to application/user)

Also, assuming if we do find a way to get if this menuItem is part of a ButtonGroup, then iterating everytime to find out if any menu item has imageicon will affect performance.
Probably it's better to reserve space for imageicon even if not present..

aivanov-jdk · 2025-06-19T09:45:14Z

In SwingSet2, now there will be gap between bullet and text to accomodate non-existing imageicon

That gap should only be there if at least one menu item has an image icon. It shoudn't be there if none have an icon.

That will require to find out if current menuItem is part of ButtonGroup and then reiterate each menuitems of the ButtonGroup and see if anyone has an imageicon..It does not seem possible to find out from current menuItem if it part of ButtonGroup as that info is not passed to each MenuItemUI (ButtonGroup info is only known to application/user)

Why? This depends on two factors only:

Are there any instances of JCheckBoxMenuItem or JRadioButtonMenuItem, and
Does any of them contain an icon?

If both are true, you add two columns for the mark and for the icon; in all other cases, the layout remains as it is now with one column where icons and marks are rendered.

The above is done in Metal L&F (run the test case in the default L&F), why can't it be done in Windows L&F?

Also, assuming if we do find a way to get if this menuItem is part of a ButtonGroup, then iterating everytime to find out if any menu item has imageicon will affect performance. Probably it's better to reserve space for imageicon even if not present..

No, I don't think it's a good approach. Look at how it's done in Metal, ideally re-use the layout code.

… 1 menuitem has imageicon

prsadhuk · 2025-06-24T05:39:13Z

In SwingSet2, now there will be gap between bullet and text to accomodate non-existing imageicon

That gap should only be there if at least one menu item has an image icon. It shoudn't be there if none have an icon.

@prrace Fixed now in latest iteration

SwingSet2 there's no gap for RBMI since there are no imageicons

aivanov-jdk · 2025-06-25T16:17:55Z

The current layout doesn't match the layout of menus in Windows.

The check mark or the bullet has to be painted where it was always painted. The menu in Windows 10 looks broken now:

This applies to the radio bullet JRadioButtonMenuItem3 which has no icon.

In Windows 11, I expect the check mark and the bullet render at the same location as it was before the fix, but it's not the case.

Below is a portion of the menu rendered with the current fix:

The part on the image in an orange rectangle is a portion of the same test case running on JDK that has no fix.

The width of the popup menu with the two columns—for check marks or bullets and icons—is the same, how is it possible? If a second column for the icon is added, the width of the popup menu has to increase.
The text of JCheckBoxMenuItem 3 is not aligned to other menu items text.
This applies to any other item added to the menu: the text of all items must align.
The location of the check mark and the bullet is wrong, you moved the check mark / bullet to the left but now it doesn't match the menu layout of a native Win32 application.
If I add another regular menu item with an icon, the icon is rendered below the menu item text.

aivanov-jdk · 2025-06-25T16:32:30Z

We have to support two cases:

One column is reserved as it is currently done;
Two columns are reserved if there's JCheckBoxMenuItem and/or JRadioButtonMenuItem that has an icon.

To reiterate Phil's suggestion,

One-column layout (nothing changes):
- No menu item has icons, check marks or bullets;
- There are JCheckBoxMenuItem and/or JRadioButtonMenuItem but they have no icons, and no other menu items have icons either;
- There are JCheckBoxMenuItem and/or JRadioButtonMenuItem but none of them has an icon, yet there are other menu items with icons: the check marks / bullets and the icons share the same reserved column.
Two-column layout (new layout):
- There's a least one JCheckBoxMenuItem and/or JRadioButtonMenuItem that has an icon.

In the two-column layout, the first column (which is always present) is reserved for check marks and bullets of JCheckBoxMenuItem and JRadioButtonMenuItem correspondingly, and the second column is reserved for rendering any icons that menu items have.

…48760

prsadhuk · 2025-06-26T04:34:15Z

4. If I add another regular menu item with an icon, the icon is rendered below the menu item text.

I couldn't reproduce this

aivanov-jdk · 2025-06-26T10:40:38Z

This is how it looks for me:

I haven't pulled your recent changes that you pushed 7 hours ago.

aivanov-jdk · 2025-06-26T11:25:22Z

The layout for Windows 11 is still wrong and suffers from the same problems that I raised yesterday. The entire popup menu looks the same way that I posted about an hour ago.

The layout for Windows 10 is correct now.

aivanov-jdk · 2025-06-26T12:21:21Z

Your latest change has fixed the position of the icon for a regular JMenuItem.

Text strings will all align. Always

Current PR doesnt have this..Text string is not aligned for item that doesn't have iconimage with item that does have iconimage. I will fix this.

This still remain unresolved: the text of all menu items should be aligned.

prsadhuk · 2025-06-26T12:32:42Z

Current PR doesnt have this..Text string is not aligned for item that doesn't have iconimage with item that does have iconimage. I will fix this.

This still remain unresolved: the text of all menu items should be aligned.

It is fixed for RBMIs as they are part of ButtonGroup (where selecting one will deselect other) so we can find out if any of the RBMIs in that group has imageicon by obtaining ButtonGroup object.
CBMIs are not part of group and can be selected/deselected individually so it doesn't seem possible to know from one CBMI whether another CBMI used by application have imageicon or not (similar for regular MIs) so in their case text are aligned individually depending on presence of imageicon for that CBMI (or regular MI)

aivanov-jdk · 2025-06-26T13:55:55Z

Current PR doesnt have this..Text string is not aligned for item that doesn't have iconimage with item that does have iconimage. I will fix this.

This still remain unresolved: the text of all menu items should be aligned.

It is fixed for RBMIs as they are part of ButtonGroup (where selecting one will deselect other) so we can find out if any of the RBMIs in that group has imageicon by obtaining ButtonGroup object.

Why is it important?

CBMIs are not part of group and can be selected/deselected individually so it doesn't seem possible to know from one CBMI whether another CBMI used by application have imageicon or not (similar for regular MIs) so in their case text are aligned individually depending on presence of imageicon for that CBMI (or regular MI)

Why is it important?

The rule to switching to two-column layout is as simple as that:

There's a least one JCheckBoxMenuItem and/or JRadioButtonMenuItem that has an icon.

If there's at least one JCheckBoxMenuItem and/or JRadioButtonMenuItem in a popup menu, the entire menu should be laid out in two columns reserved columns:

The first column is for check mark or bullet;
The second column is for the icon associated with a menu item;
The text goes in the third column;
Accelerators are rendered in the fourth column.

This is absolutely possible, both Metal & Nimbus L&F use this menu layout.

If none of JCheckBoxMenuItem and JRadioButtonMenuItem has an icon, both check marks / bullets and icons share the same first column.

aivanov-jdk · 2025-06-26T14:18:23Z

As I mentioned accelerators column in my previous comment, I realised that I hadn't tested accelerators so far.

Indeed, accelerators rendering is broken now:

This proves that the layout isn't right. You're trying to fit both check mark / bullet and an icon into the same allocated space by moving text to the right without reserving additional width for the icon. At the same time, you move the check mark / bullet to the left. Such approach doesn't respect the default insets added on the left and right of a menu item.

The popup menu has to become wider if both check marks / bullets and icons are rendered at the same time.

aivanov-jdk · 2025-06-26T11:31:11Z

src/java.desktop/share/classes/com/sun/java/swing/SwingUtilities3.java

@@ -143,6 +155,128 @@ public static RepaintManager getDelegateRepaintManager(Component
        return delegate;
    }

+    public static void applyInsets(Rectangle rect, Insets insets) {


Isn't a similar helper function already exists…

aivanov-jdk · 2025-06-26T11:33:15Z

src/java.desktop/share/classes/com/sun/java/swing/SwingUtilities3.java

+            rect.width -= (insets.right + rect.x);
+            rect.height -= (insets.bottom + rect.y);


Suggested change

rect.width -= (insets.right + rect.x);

rect.height -= (insets.bottom + rect.y);

rect.width -= (insets.right + insets.left);

rect.height -= (insets.bottom + insets.top);

The formula doesn't look right. Why do you subtract rect.x and rect.y instead of insets.left and insets.top?

This would work correctly if both rect.x and rect.y are zero, but it would give wrong results in other cases.

It was the same calculation used in BasicMenuItemUI which is now moved to SwingUtilities3 so that it can be called from Basic and WindowsMenuItemUI to avoid code duplication....you can compare the contents before the fix.....in this specific case, rect.x is inclusive of insets.left..

Similarly other SwingUtilities3 changes are also as it was in BasicMenuItemUI, it was just moved verbatim from BasicMenuItemUI to it so that it can be called from both Basic and WindowsMenuItemUI class with no changes..nothing more and nothing less...probably it can be optimized but I wanted to keep the call and the content in each method in SwingUtilities3 same as it was in BasicMenuItemUI

aivanov-jdk · 2025-06-26T12:24:10Z

src/java.desktop/share/classes/com/sun/java/swing/SwingUtilities3.java

+    private static Color disabledForeground;
+    private static Color acceleratorSelectionForeground;
+    private static Color acceleratorForeground;


Why are these static? I'm pretty sure the colors can be menu item specific, although more commonly they would be L&F specific.

aivanov-jdk · 2025-06-26T14:22:37Z

src/java.desktop/share/classes/javax/swing/plaf/basic/BasicMenuItemUI.java

+        SwingUtilities3.setDisabledForeground(disabledForeground);
+        SwingUtilities3.setAcceleratorSelectionForeground(
+                        acceleratorSelectionForeground);
+        SwingUtilities3.setAcceleratorForeground(acceleratorForeground);
+        SwingUtilities3.paintAccText(g, lh, lr);


This is a really weird way… Pass the colors explicitly as parameters to the SwingUtilities3.paintAccText method.

aivanov-jdk · 2025-06-26T14:30:30Z

src/java.desktop/windows/classes/com/sun/java/swing/plaf/windows/WindowsMenuItemUI.java

+    private static Color disabledForeground;
+    private static Color acceleratorSelectionForeground;
+    private static Color acceleratorForeground;


Why are these needed?

BasicMenuItemUI has fields for all three colors: disabledForeground, acceleratorSelectionForeground, acceleratorForeground. Why do you need to create three new static fields instead?

aivanov-jdk · 2025-06-26T15:07:12Z

src/java.desktop/windows/classes/com/sun/java/swing/plaf/windows/WindowsMenuUI.java

+    /**
+     * Paint MenuItem.
+     */
+    protected void paintMenuItem(Graphics g, JComponent c,


Suggested change

/**

* Paint MenuItem.

*/

protected void paintMenuItem(Graphics g, JComponent c,

@Override

protected void paintMenuItem(Graphics g, JComponent c,

aivanov-jdk · 2025-06-26T15:09:30Z

...va.desktop/windows/classes/com/sun/java/swing/plaf/windows/WindowsRadioButtonMenuItemUI.java

+    /**
+     * Paint MenuItem.
+     */
+    protected void paintMenuItem(Graphics g, JComponent c,


Suggested change

/**

* Paint MenuItem.

*/

protected void paintMenuItem(Graphics g, JComponent c,

@Override

protected void paintMenuItem(Graphics g, JComponent c,

aivanov-jdk · 2025-06-26T15:13:03Z

src/java.desktop/windows/classes/com/sun/java/swing/plaf/windows/WindowsMenuItemUI.java

+        String acceleratorDelimiter =
+                UIManager.getString("MenuItem.acceleratorDelimiter");
+        if (acceleratorDelimiter == null) { acceleratorDelimiter = "+"; }
+        Font acceleratorFont = UIManager.getFont("MenuItem.acceleratorFont");
+        if (acceleratorFont == null) {
+            acceleratorFont = UIManager.getFont("MenuItem.font");
+        }
+        MenuItemLayoutHelper lh = new MenuItemLayoutHelper(mi, checkIcon,
+                arrowIcon, viewRect, defaultTextIconGap, acceleratorDelimiter,
+                mi.getComponentOrientation().isLeftToRight(), mi.getFont(),
+                acceleratorFont, MenuItemLayoutHelper.useCheckAndArrow(menuItem),
+                prefix);
+        MenuItemLayoutHelper.LayoutResult lr = lh.layoutMenuItem();


Can acceleratorDelimiter, acceleratorFont be fetched from any MenuItemUI classes?

Do I understand correctly that the original menu layout that was already performed for a class gets thrown away and is being re-evaluated here?

aivanov-jdk · 2025-06-26T15:23:16Z

src/java.desktop/windows/classes/com/sun/java/swing/plaf/windows/WindowsIconFactory.java

+                            if (!isWindows11OrLater) {
+                                skin.paintSkin(g, x, y,
+                                        getIconWidth(), getIconHeight(), backgroundState);
+                                if (icon == null) {
+                                    skin = xp.getSkin(c, part);
+                                    skin.paintSkin(g, x + OFFSET, y + OFFSET, state);
+                                }
+                            } else {
+                                skin.paintSkin(g, x - 2 * OFFSET, y,
+                                        getIconWidth(), getIconHeight(), backgroundState);
+                                skinWidth = getIconWidth();


To achieve a better layout, the icon size, specifically its width, should increase whenever we detect a situation where both check mark / bullet and associated icon need to be painted together so that VistaMenuItemCheckIconFactory returns an instance of VistaMenuItemCheckIcon or Windows11MenuItemCheckIcon which reserves the space for both check mark / bullet and icon as well as a gap between them. This way, the exiting menu layout code would position the elements correctly.

aivanov-jdk · 2025-06-26T15:24:48Z

src/java.desktop/share/classes/com/sun/java/swing/SwingUtilities3.java

+        }
+    }
+
+    public static void paintCheckIcon(Graphics g, MenuItemLayoutHelper lh,


I suggest creating a new sun.swing.MenuItemRenderHelper class similar to sun.swing.MenuItemLayoutHelper to keep both layout and renderer closer together.

aivanov-jdk · 2025-06-30T14:37:46Z

src/java.desktop/windows/classes/com/sun/java/swing/plaf/windows/WindowsIconFactory.java

+                            if (!isWindows11OrLater) {
+                                skin.paintSkin(g, x, y,
+                                        getIconWidth(), getIconHeight(), backgroundState);
+                                if (icon == null) {
+                                    skin = xp.getSkin(c, part);
+                                    skin.paintSkin(g, x + OFFSET, y + OFFSET, state);
+                                }
+                            } else {
+                                skin.paintSkin(g, x, y,
+                                        getIconWidth(), getIconHeight(), backgroundState);
+                                skinWidth = getIconWidth();
                                skin = xp.getSkin(c, part);


What I expect to see here is that the skin is always painted at (x + OFFSET, y + OFFSET)

skin.paintSkin(g, x + OFFSET, y + OFFSET, state);

in both Windows 10 and 11.

The check mark and bullet have to remain on their own designated place.

aivanov-jdk · 2025-06-30T14:42:16Z

src/java.desktop/windows/classes/com/sun/java/swing/plaf/windows/WindowsIconFactory.java

+                    if (!isWindows11OrLater) {
+                        icon.paintIcon(c, g, x + OFFSET, y + OFFSET);
+                    } else {
+                        icon.paintIcon(c, g, x + icon.getIconWidth(),
+                                y + OFFSET);
+                    }


If an icon is present, it gets painted instead of check mark or bullet in Windows 10.

In Windows 11 the icon gets painted on

on the same location in regular case when the current popup menu contains no check or radio menu items with an icon;

on its own column with has proper large margins when there's at least one check or radio menu item with an icon; the size of the entire popup has to increase to allocate space for the icon column.

8348760: RadioButton is not shown if JRadioButtonMenuItem is rendered…

06c1d44

… with ImageIcon in WindowsLookAndFeel

openjdk bot added the client [email protected] label Jan 28, 2025

jecheck

bc5580f

openjdk bot added the rfr Pull request is ready for review label Jan 28, 2025

DamonGuy reviewed Jan 28, 2025

View reviewed changes

aivanov-jdk reviewed Jan 28, 2025

View reviewed changes

test/jdk/javax/swing/JMenuItem/TestImageIconWithJRadioButtonMenuItem.java Outdated Show resolved Hide resolved

test/jdk/javax/swing/JMenuItem/duke.gif Outdated Show resolved Hide resolved

formatting

befbdf5

kumarabhi006 reviewed Jan 30, 2025

View reviewed changes

src/java.desktop/windows/classes/com/sun/java/swing/plaf/windows/WindowsIconFactory.java Outdated Show resolved Hide resolved

test/jdk/javax/swing/JMenuItem/TestImageIconWithJRadioButtonMenuItem.java Outdated Show resolved Hide resolved

image path correction

2bb0869

kumarabhi006 reviewed Jan 30, 2025

View reviewed changes

src/java.desktop/windows/classes/com/sun/java/swing/plaf/windows/WindowsIconFactory.java Show resolved Hide resolved

optimise

9aeda88

kumarabhi006 approved these changes Jan 30, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Jan 30, 2025

aivanov-jdk suggested changes Jan 30, 2025

View reviewed changes

prsadhuk added 2 commits January 30, 2025 12:44

RadioButton not drawn. ImageIcon on the fly

4a5a779

Remove duke

6a8faec

openjdk bot removed the ready Pull request is ready to be integrated label Jan 30, 2025

Draw checkmark skin at same location with/without icon

9482b26

prsadhuk added 2 commits May 28, 2025 04:43

Merge branch 'master' of https://git.openjdk.java.net/jdk into JDK-83…

ba7ecae

…48760

Text alignment fix

bfc108e

openjdk bot removed the ready Pull request is ready to be integrated label May 28, 2025

aivanov-jdk mentioned this pull request Jun 19, 2025

8335986: Test javax/swing/JCheckBox/4449413/bug4449413.java fails on Windows 11 x64 because RBMenuItem's and CBMenuItem's checkmark on the left side are not visible #25864

Closed

3 tasks

Fix issue for presence of gap between buller and text only if atleast…

898996a

… 1 menuitem has imageicon

prsadhuk added 2 commits June 26, 2025 03:58

Windows 10 offset correction without icon

f019f63

Merge branch 'master' of https://git.openjdk.java.net/jdk into JDK-83…

ea1016f

…48760

MenuItem with icon fix

2348548

aivanov-jdk suggested changes Jun 26, 2025

View reviewed changes

Alignment fix

24fb819

aivanov-jdk reviewed Jun 30, 2025

View reviewed changes

squish fix

fcaeb2d

		rect.width -= (insets.right + rect.x);
		rect.height -= (insets.bottom + rect.y);

8348760: RadioButton is not shown if JRadioButtonMenuItem is rendered with ImageIcon in WindowsLookAndFeel #23324

Are you sure you want to change the base?

8348760: RadioButton is not shown if JRadioButtonMenuItem is rendered with ImageIcon in WindowsLookAndFeel #23324

Conversation

prsadhuk commented Jan 28, 2025 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Reviewing

Uh oh!

bridgekeeper bot commented Jan 28, 2025

Uh oh!

openjdk bot commented Jan 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openjdk bot commented Jan 28, 2025

Uh oh!

mlbridge bot commented Jan 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kumarabhi006 commented Jan 30, 2025

Uh oh!

prsadhuk commented Jan 30, 2025

Uh oh!

kumarabhi006 commented Jan 30, 2025

Uh oh!

Uh oh!

Uh oh!

kumarabhi006 commented Jan 30, 2025

Uh oh!

Uh oh!

kumarabhi006 left a comment

Choose a reason for hiding this comment

Uh oh!

aivanov-jdk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aivanov-jdk commented Jan 30, 2025

Uh oh!

prsadhuk commented Jan 30, 2025

Uh oh!

aivanov-jdk commented Jan 30, 2025

Uh oh!

aivanov-jdk commented Jan 30, 2025

Uh oh!

prrace commented May 7, 2025

Uh oh!

prsadhuk commented May 26, 2025

Uh oh!

prrace commented May 27, 2025

Uh oh!

prsadhuk commented May 28, 2025

Uh oh!

prsadhuk commented May 28, 2025

Uh oh!

prrace commented Jun 18, 2025

Uh oh!

prsadhuk commented Jun 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aivanov-jdk commented Jun 19, 2025

Uh oh!

prsadhuk commented Jun 24, 2025

Uh oh!

aivanov-jdk commented Jun 25, 2025

Uh oh!

aivanov-jdk commented Jun 25, 2025

Uh oh!

prsadhuk commented Jan 28, 2025 •

edited by openjdk bot

Loading

openjdk bot commented Jan 28, 2025 •

edited

Loading

mlbridge bot commented Jan 28, 2025 •

edited

Loading

prsadhuk commented Jun 19, 2025 •

edited

Loading

aivanov-jdk commented Jun 26, 2025 •

edited

Loading