On Mon, 21 Jul 2025 02:16:27 GMT, Prasanta Sadhukhan <psadhuk...@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
