On Wed, 15 Jul 2026 13:10:13 GMT, Florian Kirmaier <[email protected]> wrote:
> When ContextMenu.show(node, side, x, y) is executed, the CSS is evaluated > multiple times. > Of these 3 times, 2 process a non-final CSS state. > This leads to temporary, unnecessary changes. > > This is fixed by properly defining the stylesheets and the > ownerWindow/ownerNode properties of the context menu > before applyCss is called. The previous solution was incomplete and resulted > in unnecessary CSS calls. > > Slightly related to my other PR: https://github.com/openjdk/jfx/pull/2213 > But it is fixed in a way that works independently of it. > > --------- > - [x] I confirm that I make this contribution in accordance with the [OpenJDK > Interim AI Policy](https://openjdk.org/legal/ai). Changes requested by angorya (Reviewer). 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. 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 NOE. modules/javafx.controls/src/test/java/test/javafx/scene/control/ContextMenuTest.java line 69: > 67: import test.com.sun.javafx.scene.control.infrastructure.MouseEventFirer; > 68: import test.com.sun.javafx.scene.control.infrastructure.StageLoader; > 69: import test.javafx.scene.CssStyleHelperTest; this produces two errors in Eclipse: Description Resource Type Path Location CssStyleHelperTest cannot be resolved ContextMenuTest.java Java Problem /controls/src/test/java/test/javafx/scene/control line 733 The type test.javafx.scene.CssStyleHelperTest is not accessible ContextMenuTest.java Java Problem /controls/src/test/java/test/javafx/scene/control line 71 what bothers me here is that the `toDataURL()` method referred to should not be in a file called `CssStyleHelperTest` but maybe in `Utils`, and also that the are creating an inter-module dependency for tests. Is there a way to create a utility package accessible to all the modules where we can park all the little utility methods? Or do we have to duplicate this and similar methods across the modules? we also have two `CssStyleHelperTest` classes in graphics/src/test and tests/system 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 modules/javafx.controls/src/test/java/test/javafx/scene/control/ContextMenuTest.java line 704: > 702: String css = """ > 703: .button { -fx-skin: > "test.javafx.scene.control.ContextMenuTest$ButtonSkin1"; } > 704: .anchor .button { -fx-skin: > "st.javafx.scene.control.ContextMenuTest$ButtonSkin2"; } **st.** javafx.scene.control.ContextMenuTest$ButtonSkin2 is this a typo? modules/javafx.controls/src/test/java/test/javafx/scene/control/ContextMenuTest.java line 809: > 807: public ButtonSkin1(Button button) { super(button); } > 808: } > 809: public static class ButtonSkin2 extends ButtonSkin { please add a newline here ------------- PR Review: https://git.openjdk.org/jfx/pull/2215#pullrequestreview-5433185141 PR Review Comment: https://git.openjdk.org/jfx/pull/2215#discussion_r4200505320 PR Review Comment: https://git.openjdk.org/jfx/pull/2215#discussion_r4200530515 PR Review Comment: https://git.openjdk.org/jfx/pull/2215#discussion_r4200442702 PR Review Comment: https://git.openjdk.org/jfx/pull/2215#discussion_r4199247269 PR Review Comment: https://git.openjdk.org/jfx/pull/2215#discussion_r4200548787 PR Review Comment: https://git.openjdk.org/jfx/pull/2215#discussion_r4199249822
