On Thu, 24 Sep 2026 08:13:46 GMT, Christopher Schnick <[email protected]> 
wrote:

> This is simple fix for the NPE. The second test is a realistic case on how 
> this can happen accidentally, as that originally happened to me
> 
> ---------
> - [x] I confirm that I make this contribution in accordance with the [OpenJDK 
> Interim AI Policy](https://openjdk.org/legal/ai).

some comments, simple fix that makes sense.

modules/javafx.controls/src/main/java/javafx/scene/control/ContextMenu.java 
line 249:

> 247:     public void show(Node anchor, Side side, double dx, double dy) {
> 248:         Toolkit.getToolkit().checkFxUserThread();
> 249:         if (anchor == null) return;

Minor, but I would suggest we check that here in this branch:

`anchor == null || anchor.getScene() == null`.

modules/javafx.controls/src/test/java/test/javafx/scene/control/ContextMenuTest.java
 line 374:

> 372: 
> 373:         // Fail on internal exceptions
> 374:         ControlTestUtils.runWithExceptionHandler(() -> subMenu.show());

You could also use: `assertDoesNotThrow(() -> subMenu.show());`

but that will require the following setup already used in other tests:


    @BeforeEach
    public void setup() {
        Thread.currentThread().setUncaughtExceptionHandler((thread, throwable) 
-> {
            if (throwable instanceof RuntimeException) {
                throw (RuntimeException)throwable;
            } else {
                
Thread.currentThread().getThreadGroup().uncaughtException(thread, throwable);
            }
        });
    }

    @AfterEach
    public void cleanup() {
        Thread.currentThread().setUncaughtExceptionHandler(null);
    }


Personally I like that more, because `assertDoesNotThrow` shows the intention 
very clearly.

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

PR Review: https://git.openjdk.org/jfx/pull/2321#pullrequestreview-5302912607
PR Review Comment: https://git.openjdk.org/jfx/pull/2321#discussion_r4092357499
PR Review Comment: https://git.openjdk.org/jfx/pull/2321#discussion_r4092439990

Reply via email to