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

[Alexey Ivanov]

On Thu, 24 Jul 2025 02:34:44 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
>
> 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