On Wed, 7 Oct 2026 14:56:34 GMT, Florian Kirmaier <[email protected]> wrote:
>> modules/javafx.controls/src/main/java/javafx/scene/control/ContextMenu.java
>> line 255:
>>
>>> 253: if (getItems().size() == 0) return;
>>> 254:
>>> 255:
>>> PopupWindowHelper.ownerWindow(this).set(anchor.getScene().getWindow());
>>
>> this runs before `PopupWindow.validateOwnerWindow()`, causing regression
>> when, for example, an already visible menu is shown from a node in some
>> other window
>> since it'll likely mess up other things like internal listeners and native
>> peers.
>
> my suggestions are in the comments bellow.
there seems to be two regression scenarios, resulting from a lack of validation
prior to the owner assignment.
1. making the menu its own owner causes an infinite loop in PopupWindow:889
(master throws an IllegalArgumentException)
2. if you show an already visible menu from another stage it might break the
native peer assignment, see Popupwindow:559
>> modules/javafx.controls/src/main/java/javafx/scene/control/ContextMenu.java
>> line 257:
>>
>>> 255:
>>> PopupWindowHelper.ownerWindow(this).set(anchor.getScene().getWindow());
>>> 256: PopupWindowHelper.ownerNode(this).set(anchor);
>>> 257: PopupWindowHelper.applyStylesheetFromOwner(this,
>>> anchor.getScene().getWindow());
>>
>> an anchor may have a scene without a window, likely causing an NPE.
>
> my suggestions are in the comments bellow.
What comments? I suggest you simply check for null.
>> modules/javafx.controls/src/test/java/test/javafx/scene/control/ContextMenuTest.java
>> line 701:
>>
>>> 699: }
>>> 700:
>>> 701: @Test public void testCssProcessedOnlyOnce() {
>>
>> please keep `@Test` annotation on its own line
>
> This is the pattern in the whole class.
> I can change it only here, for the whole class - or leave it as it is.
I would prefer the new code to follow the modern convention.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/2215#discussion_r4415582439
PR Review Comment: https://git.openjdk.org/jfx/pull/2215#discussion_r4415798741
PR Review Comment: https://git.openjdk.org/jfx/pull/2215#discussion_r4414780932
