On Thu, 24 Jul 2025 02:34:44 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 > > Prasanta Sadhukhan has updated the pull request incrementally with one > additional commit since the last revision: > > typo Changes requested by aivanov (Reviewer). src/java.desktop/share/classes/javax/swing/JPopupMenu.java line 129: > 127: transient Frame frame; > 128: private int desiredLocationX,desiredLocationY; > 129: transient private PropertyChangeListener propListener; Suggestion: private transient PropertyChangeListener propListener; src/java.desktop/share/classes/javax/swing/JPopupMenu.java line 952: > 950: oldInvoker.removePropertyChangeListener(oldPropListener); > 951: } > 952: propListener = new PropertyChangeListener() { I believe one instance of `PropertyChangeListener` is enough. It could be created at the `propListener` field declaration. test/jdk/javax/swing/JPopupMenu/TestPopupInvoker.java line 63: > 61: } > 62: > 63: public static void main(String args[]) throws Exception { Suggestion: public static void main(String[] args) throws Exception { Use Java-style array declaration. test/jdk/javax/swing/JPopupMenu/TestPopupInvoker.java line 66: > 64: try { > 65: Robot robot = new Robot(); > 66: SwingUtilities.invokeAndWait(() -> createUI()); Suggestion: SwingUtilities.invokeAndWait(TestPopupInvoker::createUI); Method references are preferred in this case. test/jdk/javax/swing/JPopupMenu/TestPopupInvoker.java line 69: > 67: robot.waitForIdle(); > 68: robot.delay(1000); > 69: SwingUtilities.invokeAndWait(() -> { Suggestion: robot.delay(1000); SwingUtilities.invokeAndWait(() -> { Introduce sections of code to improve readability. test/jdk/javax/swing/JPopupMenu/TestPopupInvoker.java line 76: > 74: jpm.show(label, 0, 0); > 75: pt = label.getLocationOnScreen(); > 76: size = label.getBounds(); Neither `pt` nor `size` are used. test/jdk/javax/swing/JPopupMenu/TestPopupInvoker.java line 80: > 78: robot.waitForIdle(); > 79: robot.delay(2000); > 80: SwingUtilities.invokeAndWait(() -> { Suggestion: robot.delay(2000); SwingUtilities.invokeAndWait(() -> { Introduce sections of code to improve readability. ------------- PR Review: https://git.openjdk.org/jdk/pull/26407#pullrequestreview-3052590110 PR Review Comment: https://git.openjdk.org/jdk/pull/26407#discussion_r2229138254 PR Review Comment: https://git.openjdk.org/jdk/pull/26407#discussion_r2229143184 PR Review Comment: https://git.openjdk.org/jdk/pull/26407#discussion_r2229191620 PR Review Comment: https://git.openjdk.org/jdk/pull/26407#discussion_r2229193103 PR Review Comment: https://git.openjdk.org/jdk/pull/26407#discussion_r2229152876 PR Review Comment: https://git.openjdk.org/jdk/pull/26407#discussion_r2229185127 PR Review Comment: https://git.openjdk.org/jdk/pull/26407#discussion_r2229184368