8370465: Right to Left Orientation Issues with MenuItem Component #27968

prsadhuk · 2025-10-24T04:21:30Z

Icon rendering offset is wrong for RTL orientation which is why the icon was not rendered properly..Also, LEADING horizontal text position was not accounted for..

Before fix

With fix

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-8370465: Right to Left Orientation Issues with MenuItem Component (Bug - P3)

Reviewers

Alexander Zuev (@azuev-java - Reviewer)
Harshitha Onkar (@honkar-jdk - Reviewer) Review applies to ca4e1904

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 27968

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

Using diff file

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

Using Webrev

Link to Webrev Comment

bridgekeeper · 2025-10-24T04:23:24Z

👋 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-10-24T04:23:44Z

@prsadhuk This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8370465: Right to Left Orientation Issues with MenuItem Component

Reviewed-by: kizune, honkar

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 121 new commits pushed to the master branch:

4f9f086: 8224852: JVM crash on watched field access from native code
6347f10: 8370521: GenShen: Various code cleanup related to promotion
3c1010b: 8370646: TestLargeUTF8Length.java needs lots of memory
... and 118 more: https://git.openjdk.org/jdk/compare/ffcb1585ed6c2a2bff28be6854d44a672aa31a0b...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

openjdk · 2025-10-24T04:24:18Z

@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.

prsadhuk · 2025-10-24T04:28:11Z

/touch

openjdk · 2025-10-24T04:28:32Z

@prsadhuk The pull request is being re-evaluated and the inactivity timeout has been reset.

mlbridge · 2025-10-24T04:32:34Z

Webrevs

TejeshR13 · 2025-10-28T07:29:13Z

With fix, starting position of text and icon seems to be having an offset compared to metal. Icon is starting a bit right side (2nd line) and text is a bit left side.

prsadhuk · 2025-10-28T07:36:36Z

That is expected as for WIndows L&F we are having dedicated offset for icon and checkmark/bullet so that they dont overlap as earlier before JDK-8348760 the icon and radiobutton bullet/checkmark was overlapping so checkmark was not visible so now each is drawn on its own dedicated position

azuev-java · 2025-10-29T00:07:06Z

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

                if (icon != null) {
-                    icon.paintIcon(c, g, x + VistaMenuItemCheckIconFactory.getIconWidth(),
-                                   y + OFFSET);
+                    if (!c.getComponentOrientation().equals(ComponentOrientation.RIGHT_TO_LEFT)) {


Why not just if(c.getComponentOrientation().isLeftToRight()) ? Then you will not need the extra import and the documentation for ComponentOrientation clearly says that direct comparison should be avoided.

Yes, I have modified to use the existing LTR check already used in the file

azuev-java · 2025-10-29T00:16:45Z

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

            Rectangle rect = lr.getTextRect();
-
-            rect.x += lh.getAfterCheckIconGap();
+            if (menuItem.getHorizontalTextPosition() != SwingConstants.LEADING) {


Not sure i understand why we only checking for "LEADING" text position. What if it is specified specifically as "LEFT" or "RIGHT"? What would result look like in the different component orientations with this fix?

LEADING causes the text to appear before icon so need to account for it..

Before fix

After fix

Others are working as I can see "horizontalAlignment = LEFT..." and "horizontalAlignment=RIGHT" which uses LEFT and RIGHT horizontal text positioning

Actually I have modified the PR to rectify layouting ensuring radiobutton bullet/checkmark are drawn at dedicated position and doesn't interfere with icon position for RTL too..Previous PR iteration was not taking into account of this for RTL so if the menuitem was selected in RTL, it was not shown..

I just tested the latest version of the fix with the slightly modified test case (i added two extra menu items with
menuItem.setHorizontalTextPosition(SwingConstants.LEFT);
and
menuItem.setHorizontalTextPosition(SwingConstants.RIGHT);
and it does not look exactly correct. Here's the screenshot:

Fixed..it now looks ok to me...Please check

Yes, new version works correctly. I would suggest to add these menu items to the regression test but that is optional.

As @azuev-java suggested, it might be a good idea to add the extra cases to the test.

Added the menu items
@azuev-java Please take a look

honkar-jdk · 2025-10-29T22:26:39Z

There seems to be less gap between text and icon in RTL Windows L&F case. The icon border looks different when the icon is to the left and right side of text, is that expected?

prsadhuk · 2025-10-30T02:13:12Z

There seems to be less gap between text and icon in RTL Windows L&F case. The icon border looks different when the icon is to the left and right side of text, is that expected?

should be ok now in latest iteration..

honkar-jdk

Updated fix LGTM apart from minor test suggestions.

honkar-jdk · 2025-10-30T22:52:23Z

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

 /*
 * @test id=windows
- * @bug 4211052
+ * @bug 4211052 8370465


Instead of multiple separate jtreg tags for each LaF we can combine it using @run tags as below:

/* ... ... * @run main/manual RightLeftOrientation windows * @run main/manual RightLeftOrientation motif * @run main/manual RightLeftOrientation metal */

We decided to use it that way in [8346753](#25907 (comment)) sometime back

honkar-jdk · 2025-10-30T22:55:42Z

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

            Rectangle rect = lr.getTextRect();
-
-            rect.x += lh.getAfterCheckIconGap();
+            if (menuItem.getHorizontalTextPosition() != SwingConstants.LEADING) {


As @azuev-java suggested, it might be a good idea to add the extra cases to the test.

prsadhuk · 2025-10-31T03:32:33Z

/integrate

openjdk · 2025-10-31T03:33:46Z

Going to push as commit fc5df4a.
Since your change was applied there have been 121 commits pushed to the master branch:

4f9f086: 8224852: JVM crash on watched field access from native code
6347f10: 8370521: GenShen: Various code cleanup related to promotion
3c1010b: 8370646: TestLargeUTF8Length.java needs lots of memory
... and 118 more: https://git.openjdk.org/jdk/compare/ffcb1585ed6c2a2bff28be6854d44a672aa31a0b...master

Your commit was automatically rebased without conflicts.

openjdk · 2025-10-31T03:33:53Z

@prsadhuk Pushed as commit fc5df4a.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

8370465: Right to Left Orientation Issues with MenuItem Component.

a664467

openjdk bot added the client [email protected] label Oct 24, 2025

openjdk bot added the rfr Pull request is ready for review label Oct 24, 2025

azuev-java reviewed Oct 29, 2025

View reviewed changes

Use existing check

e891abd

prsadhuk changed the title ~~8370465: Right to Left Orientation Issues with MenuItem Component.~~ 8370465: Right to Left Orientation Issues with MenuItem Component Oct 29, 2025

prsadhuk added 3 commits October 29, 2025 04:12

formatting

b78ea8b

unused import removed

e8446a6

Correct layouting

201f9ac

SwingConstants LEFT and RIGHT fix

ca4e190

azuev-java approved these changes Oct 30, 2025

View reviewed changes

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

honkar-jdk approved these changes Oct 30, 2025

View reviewed changes

Add LEFT, RIGHT textPosition to test

ccc2368

openjdk bot removed the ready Pull request is ready to be integrated label Oct 31, 2025

azuev-java approved these changes Oct 31, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Oct 31, 2025

openjdk bot added the integrated Pull request has been integrated label Oct 31, 2025

openjdk bot closed this Oct 31, 2025

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Oct 31, 2025

prsadhuk deleted the JDK-8370465 branch October 31, 2025 03:34

8370465: Right to Left Orientation Issues with MenuItem Component #27968

8370465: Right to Left Orientation Issues with MenuItem Component #27968

Conversation

prsadhuk commented Oct 24, 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 Oct 24, 2025

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

prsadhuk commented Oct 24, 2025

Uh oh!

openjdk bot commented Oct 24, 2025

Uh oh!

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

Uh oh!

Webrevs

Uh oh!

TejeshR13 commented Oct 28, 2025

Uh oh!

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

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

honkar-jdk commented Oct 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

prsadhuk commented Oct 30, 2025

Uh oh!

honkar-jdk left a comment

Choose a reason for hiding this comment

Uh oh!

honkar-jdk Oct 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

prsadhuk commented Oct 31, 2025

Uh oh!

openjdk bot commented Oct 31, 2025

Uh oh!

openjdk bot commented Oct 31, 2025

Uh oh!

Reviewers

prsadhuk commented Oct 24, 2025 •

edited by openjdk bot

Loading

openjdk bot commented Oct 24, 2025 •

edited

Loading

openjdk bot commented Oct 24, 2025 •

edited

Loading

mlbridge bot commented Oct 24, 2025 •

edited

Loading

prsadhuk commented Oct 28, 2025 •

edited

Loading

prsadhuk Oct 29, 2025 •

edited

Loading

prsadhuk Oct 29, 2025 •

edited

Loading

prsadhuk Oct 31, 2025 •

edited

Loading

honkar-jdk commented Oct 29, 2025 •

edited

Loading

honkar-jdk Oct 30, 2025 •

edited

Loading