On Wed, 7 Oct 2026 14:31:33 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).
>
> Florian Kirmaier has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   8388313: PR feedback1

These are the checks usually done before the owner properties are set in 
PopupWindow:


    public void show(Node ownerNode, double anchorX, double anchorY) {
        Toolkit.getToolkit().checkFxUserThread();
        if (ownerNode == null) {
            throw new NullPointerException("The owner node must not be null");
        }

        final Scene ownerNodeScene = ownerNode.getScene();
        if ((ownerNodeScene == null)
                || (ownerNodeScene.getWindow() == null)) {
            throw new IllegalArgumentException(
                    "The owner node needs to be associated with a window");
        }

        final Window newOwnerWindow = ownerNodeScene.getWindow();
        validateOwnerWindow(newOwnerWindow);


To call `PopupWindow.validateOwnerWindow`, I could make it public with the 
accessor pattern.

Should I just add these checks to the ContextMenu.show method,
or leave it as is?

We could also make a reusable method in PopupWindow, which can be used more 
freely to check and prepare the Window for an anchor node. Maybe even public, 
because it's a valid use case to use the size of a Popup to decide its position.



Some words on how often the bug appears and with which symptoms.
The motivation for the fix was never performance and always correctness.
I'm seeing it quite often now, with different symptoms and in different projects
Possible symptoms are:
 - Double evaluation of skins
 - Wrong size/position computation of ContextMenu
 - Error messages for invalid CSS variables, without any visual effect because 
it's corrected slightly later.

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

PR Comment: https://git.openjdk.org/jfx/pull/2215#issuecomment-6040639487

Reply via email to