RFR: 4938801: The popup does not go when the component is removed.

[Alexey Ivanov]

On Mon, 21 Jul 2025 02:16:27 GMT, Prasanta Sadhukhan <psadhukhan at openjdk.org> wrote:

> Issue is seen that a popup doesn't get closed when the component that invokes it, gets removed from the parent container.
> This is because the JPopupMenu does not listen to its invoker liefecycle thereby behaving as a standalone entity after creation.
> Fix is made to make sure popup listens to its invoker lifecycle by registering its PropertyChangeListener to the invoker and listens to the ["ancestor" property name ], https://github.com/openjdk/jdk/blob/441dbde2c3c915ffd916e39a5b4a91df5620d7f3/src/java.desktop/share/classes/javax/swing/JComponent.java#L4853-L4858 which will become null when removed, wherein we should dispose of the popup

Changes requested by aivanov (Reviewer).

src/java.desktop/share/classes/javax/swing/JPopupMenu.java line 962:

> 960:             ui.uninstallUI(this);
> 961:             if (oldInvoker != null) {
> 962:                 oldInvoker.removePropertyChangeListener(propListener);

This won't remove the install `propListener` because a new instance of the listener is created each time the method is called.

test/jdk/javax/swing/JPopupMenu/TestPopupInvoker.java line 56:

> 54:     static volatile Rectangle size;
> 55: 
> 56:     public static class MyThread implements Runnable {

Do you even need another thread?

The test is driven from the main thread, and you can remove the component from container on the main thread.

Perhaps, a better version of the test would remove the component when the event from the popup menu that it's now displayed is received. If the popup is hidden as the result of component removal, another event should be received. Thus, the test would depend on the events and latches instead of on the delays.

test/jdk/javax/swing/JPopupMenu/TestPopupInvoker.java line 68:

> 66:             System.out.println("Starting 3 second countdown...");
> 67:             try{
> 68:                 Thread.currentThread().sleep(3000);

Suggestion:

                Thread.sleep(3000);

`sleep` is a static method, no instance of `Thread` is needed.

test/jdk/javax/swing/JPopupMenu/TestPopupInvoker.java line 71:

> 69:             } catch (Exception e) {}
> 70:             System.out.println("Removing popup invoker from the container.");
> 71:             box.remove(invo);

This breaks Swing contract that components must be accessed via EDT only.

test/jdk/javax/swing/JPopupMenu/TestPopupInvoker.java line 92:

> 90:                     jpm.add("Three");
> 91:                 }
> 92:                 jpm.show(label, 0, 0);

The popup can be shown programmatically without mouse click: `invokeAndWait(() -> jpm.show())` after the initial `waitForIdle` and delay.

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

PR Review: https://git.openjdk.org/jdk/pull/26407#pullrequestreview-3038970876
PR Review Comment: https://git.openjdk.org/jdk/pull/26407#discussion_r2219765853
PR Review Comment: https://git.openjdk.org/jdk/pull/26407#discussion_r2219783806
PR Review Comment: https://git.openjdk.org/jdk/pull/26407#discussion_r2219767297
PR Review Comment: https://git.openjdk.org/jdk/pull/26407#discussion_r2219772804
PR Review Comment: https://git.openjdk.org/jdk/pull/26407#discussion_r2219786884