8367376: Bad ButtonUI prevents other components from updating when system changes desktop properties #27205

mickleness · 2025-09-10T22:28:54Z

Previously:

If DesktopProperty#updateAllUIs threw an exception, we would never reset the update-pending property to false. This means any subsequent call to updateUI() would not attempt to call updateAllUIs()

With this change:
Subsequent calls to DesktopProperty#updateUI() can still trigger at least one call to updateAllUIs().

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-8367376: Bad ButtonUI prevents other components from updating when system changes desktop properties (Bug - P4)

Reviewers

Phil Race (@prrace - Reviewer) Review applies to 33937331
Alexey Ivanov (@aivanov-jdk - Reviewer)
Prasanta Sadhukhan (@prsadhuk - Reviewer) Review applies to 39d4ad96

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 27205

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

Using diff file

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

Using Webrev

Link to Webrev Comment

Previously: If updateAllUIs threw an exception, we would never reset the update-pending property to false. This means any subsequent call to `updateUI()` would not attempt to call `updateAllUIs()`

bridgekeeper · 2025-09-10T22:30:47Z

👋 Welcome back jwood! 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-09-10T22:32:45Z

@mickleness 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:

8367376: Bad ButtonUI prevents other components from updating when system changes desktop properties

Reviewed-by: aivanov, prr, psadhukhan

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 156 new commits pushed to the master branch:

79bcc7b: 8369257: PPC: compiler/whitebox/RelocateNMethodMultiplePaths.java fails with assertion
927aa3f: 8369178: G1: Use NMethodMarkingScope and ThreadsClaimTokenScope in G1RootProcessor
23fcbb0: 8365153: AArch64: Set JVM flags for Neoverse N3 and V3 cores
... and 153 more: https://git.openjdk.org/jdk/compare/52e777845f0a09b4c285131f1eff02dfbffa0d1f...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.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@prrace, @aivanov-jdk, @prsadhuk) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

openjdk · 2025-09-10T22:33:57Z

@mickleness 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-09-10T22:39:03Z

Webrevs

prrace

Looks reasonable to me.
@prsadhuk please review.

test/jdk/com/sun/java/swing/plaf/Test8367376.java

aivanov-jdk · 2025-09-17T20:12:39Z

test/jdk/com/sun/java/swing/plaf/Test8367376.java

+        p.add(button);
+        getContentPane().add(p);
+    }
+}


A file should end with new line (\n) character.

OK, this is updated

aivanov-jdk · 2025-09-17T20:17:11Z

test/jdk/com/sun/java/swing/plaf/Test8367376.java

+import java.awt.Toolkit;
+import java.util.concurrent.CountDownLatch;
+
+public class Test8367376 extends JFrame {


Give the test a meaningful name? UninstallUIThrowsException?

OK. I used DesktopPropertyResetPendingFlagTest

Co-authored-by: Alexey Ivanov <alexey.ivanov@oracle.com>

This is in response to: openjdk#27205 (comment)

prsadhuk · 2025-09-18T06:51:19Z

src/java.desktop/share/classes/sun/swing/plaf/DesktopProperty.java

+                    } finally {
+                        setUpdatePending(false);
+                    }
                }


I guess same thing we need to do here too

jdk/src/java.desktop/share/classes/javax/swing/plaf/metal/MetalLookAndFeel.java

Lines 2293 to 2296 in c96d09a

public void run() {

updateAllUIs();

setUpdatePending(false);

}

and here too

jdk/src/java.desktop/share/classes/javax/swing/plaf/synth/SynthLookAndFeel.java

Lines 911 to 914 in c96d09a

public void run() {

updateAllUIs();

setUpdatePending(false);

}

OK, I updated Metal and Synth L&F.

The test in this PR does not currently test either of these new changes, though. Normally I'd feel nervous about that, but IMO the changes in this PR are acceptably low-risk.

Does anyone disagree? If so: should I split this into 2-3 separate tickets/PRs? The original steps involved the Windows L&F and changing the OS's accessibility contrast settings; I don't know if those steps affect Metal/Synth L&F's at all.

prsadhuk · 2025-09-18T06:51:26Z

test/jdk/com/sun/java/swing/plaf/DesktopPropertyResetPendingFlagTest.java

+    private static void setLookAndFeel() {
+        try {
+            String lf = UIManager.getSystemLookAndFeelClassName();
+            UIManager.setLookAndFeel(lf);


Guess we should iterate for all L&F s, not just test the default..

I went the opposite direction for now: I removed any reference to the L&F. This test is structured so it emulates what the Windows L&F was doing, but it's platform/L&F-independent (for the sake of full automation).

I tested on Mac and Windows and confirmed the test still fails against master, and it passes in this PR.

If you do not want to tinker with the current test, maybe you can have another test testing the L&F s, handling exception gracefully in updateUI

Since the changes are in look-and-feels, you should iterate over installed look-and-feels. Otherwise you test Aqua L&F on macOS and Metal on Windows only, which means an exception may still cause problems in other L&Fs.

Creating several tests if needed is fine, yet I don't think it's needed.

I agree with Alexey. I think we should definitely iterate across all L&Fs and it can be done without creating more tests.

I updated the test to iterate over all available L&F's

aivanov-jdk

Other L&F have the same problem, so it would be good to resolve it and update the test to iterate over all installed L&Fs.

This was probably left over from an earlier draft; this test uses the TestDesktopProperty class so it doesn't rely on/test any particular L&F.

This is in response to: openjdk#27205 (comment)

This is in response to: https://git.openjdk.org/jdk/pull/27205#discussion_r2382574446

prsadhuk · 2025-10-07T07:22:08Z

test/jdk/com/sun/java/swing/plaf/DesktopPropertyResetPendingFlagTest.java

@@ -100,16 +89,25 @@ protected void dispatchEvent(AWTEvent event) {
        };
        Toolkit.getDefaultToolkit().getSystemEventQueue().push(newEventQueue);

-        SwingUtilities.invokeLater(
-                DesktopPropertyResetPendingFlagTest::setLookAndFeel);
+        for (UIManager.LookAndFeelInfo info :


UIManager import is missing

Thanks; this is updated.

you can also dispose of the frame in try-finally block to be safe and consistent with headful regression tests
and you need to add @key headful to the jtreg tag

OK, this is updated

This is in response to: https://git.openjdk.org/jdk/pull/27205#discussion_r2409647336

This is in response to the first point here: openjdk#27205 (comment)

This is in response to the second point here: openjdk#27205 (comment)

test/jdk/com/sun/java/swing/plaf/DesktopPropertyResetPendingFlagTest.java

…agTest.java Co-authored-by: Alexey Ivanov <alexey.ivanov@oracle.com>

mickleness · 2025-10-08T16:52:35Z

/integrate

openjdk · 2025-10-08T16:53:49Z

@mickleness
Your change (at version aa565ef) is now ready to be sponsored by a Committer.

aivanov-jdk · 2025-10-09T16:00:19Z

/sponsor

openjdk · 2025-10-09T16:02:03Z

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

9891cd6: 8369313: java/net/httpclient/TimeoutBasic.java should accept that HttpTimeoutException should be somewhere down the cause chain
48d21b5: 8369434: java/net/httpclient/AltServiceUsageTest.java fails intermittently
13f460c: 8368677: acvp test should throw SkippedException when no ACVP-Server available
... and 177 more: https://git.openjdk.org/jdk/compare/52e777845f0a09b4c285131f1eff02dfbffa0d1f...master

Your commit was automatically rebased without conflicts.

openjdk · 2025-10-09T16:02:13Z

@aivanov-jdk @mickleness Pushed as commit 7c75cb3.

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

8367376: call setUpdatePending(false) in a finally block

abec6a4

Previously: If updateAllUIs threw an exception, we would never reset the update-pending property to false. This means any subsequent call to `updateUI()` would not attempt to call `updateAllUIs()`

openjdk bot added client client-libs-dev@openjdk.org rfr Pull request is ready for review labels Sep 10, 2025

mickleness changed the title ~~8367376: call setUpdatePending(false) in a finally block~~ 8367376: Bad ButtonUI prevents other components from updating when system changes desktop properties Sep 11, 2025

8367376: trivial typos in comment

3393733

prrace approved these changes Sep 15, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Sep 15, 2025

aivanov-jdk suggested changes Sep 17, 2025

View reviewed changes

mickleness and others added 2 commits September 17, 2025 16:32

Update test/jdk/com/sun/java/swing/plaf/Test8367376.java

b7d33b6

Co-authored-by: Alexey Ivanov <alexey.ivanov@oracle.com>

Update test/jdk/com/sun/java/swing/plaf/Test8367376.java

fef2241

Co-authored-by: Alexey Ivanov <alexey.ivanov@oracle.com>

openjdk bot removed the ready Pull request is ready to be integrated label Sep 17, 2025

mickleness added 3 commits September 17, 2025 16:36

8367376: changing field names

299936b

This is in response to: openjdk#27205 (comment)

8367376: add new line to end of file

ac221d0

This is in response to: openjdk#27205 (comment)

8367376: rename test file

108e2d5

This is in response to: openjdk#27205 (comment)

prsadhuk reviewed Sep 18, 2025

View reviewed changes

aivanov-jdk approved these changes Sep 19, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Sep 19, 2025

aivanov-jdk suggested changes Sep 19, 2025

View reviewed changes

openjdk bot removed the ready Pull request is ready to be integrated label Sep 19, 2025

mickleness added 5 commits September 22, 2025 12:47

8367376: remove setLookAndFeel

d85360f

This was probably left over from an earlier draft; this test uses the TestDesktopProperty class so it doesn't rely on/test any particular L&F.

8367376: use same try/finally pattern for MetalL&F

38227ae

This is in response to: openjdk#27205 (comment)

8367376: use same try/finally pattern for SynthL&F

8f05606

This is in response to: openjdk#27205 (comment)

Merge branch 'master' into JDK-8367376

3636202

8367376: test all installed L&F's

1b669f8

This is in response to: https://git.openjdk.org/jdk/pull/27205#discussion_r2382574446

prsadhuk reviewed Oct 7, 2025

View reviewed changes

mickleness added 3 commits October 7, 2025 13:24

8367376: adding missing UIManager import

d93815c

This is in response to: https://git.openjdk.org/jdk/pull/27205#discussion_r2409647336

8367376: adding Frame.dispose() in try/finally

6cf5066

This is in response to the first point here: openjdk#27205 (comment)

8367376: adding @key headful

39d4ad9

This is in response to the second point here: openjdk#27205 (comment)

prsadhuk approved these changes Oct 8, 2025

View reviewed changes

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

aivanov-jdk approved these changes Oct 8, 2025

View reviewed changes

test/jdk/com/sun/java/swing/plaf/DesktopPropertyResetPendingFlagTest.java Show resolved Hide resolved

Update test/jdk/com/sun/java/swing/plaf/DesktopPropertyResetPendingFl…

aa565ef

…agTest.java Co-authored-by: Alexey Ivanov <alexey.ivanov@oracle.com>

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

aivanov-jdk approved these changes Oct 8, 2025

View reviewed changes

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

openjdk bot added the sponsor Pull request is ready to be sponsored label Oct 8, 2025

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

openjdk bot closed this Oct 9, 2025

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

8367376: Bad ButtonUI prevents other components from updating when system changes desktop properties #27205

8367376: Bad ButtonUI prevents other components from updating when system changes desktop properties #27205

Uh oh!

Conversation

mickleness commented Sep 10, 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 Sep 10, 2025

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Webrevs

Uh oh!

prrace left a comment

Choose a reason for hiding this comment

Uh oh!

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!

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!

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!

aivanov-jdk left a comment

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!

Uh oh!

mickleness commented Oct 8, 2025

Uh oh!

openjdk bot commented Oct 8, 2025

Uh oh!

aivanov-jdk commented Oct 9, 2025

Uh oh!

openjdk bot commented Oct 9, 2025

Uh oh!

openjdk bot commented Oct 9, 2025

Uh oh!

Reviewers

Assignees

Labels

Milestone

mickleness commented Sep 10, 2025 •

edited by openjdk bot

Loading

openjdk bot commented Sep 10, 2025 •

edited

Loading

openjdk bot commented Sep 10, 2025 •

edited

Loading

mlbridge bot commented Sep 10, 2025 •

edited

Loading