JavaFX CustomMenuItem strange behaviour with TextField

Question

maybe someone can explain the following behaviour - and hopefully, possible workarounds... thanks.

When using a CustomMenuItem containing a TextField, the action of the MenuItem above gets triggered by pressing enter inside the textfield... unless the textfield has an actionlistener set (not added)... I need to use addEventHandler, not setOnAction... :-/

import javafx.application.Application;
import javafx.scene.Scene;
import javafx.scene.control.CustomMenuItem;
import javafx.scene.control.MenuButton;
import javafx.scene.control.MenuItem;
import javafx.scene.control.TextField;
import javafx.stage.Stage;

public class CustomMenuTest extends Application {
    @Override
    public void start(Stage primaryStage) throws Exception {

        MenuButton menu = new MenuButton("Fancy Menu...");

        MenuItem hello = new MenuItem("Hello");
        hello.setOnAction(event -> System.out.println("Hello | " + event.getSource()));

        MenuItem world = new MenuItem("World!");
        world.setOnAction(event -> System.out.println("World! | " + event.getSource()));

        /*
        Set the cursor into the TextField, maybe type something, and hit enter.
        --> Expected: "ADD: <Text you typed> ..."
        --> Actual: "ADD: <Text you typed> ..." AND "World! ..." - so the button above gets triggered as well.
        If I activate listener (II) or (III), everything works fine - even the empty action in (III) does is job, but this is ugly...
        (And I can't use (II), because I need (I).
         */
        TextField textField = new TextField();
        /*   I */ textField.addEventHandler(ActionEvent.ACTION,
                                  event -> System.out.println("ADD: " + textField.getText() + " | " + event.getSource()));
        /*  II */ // textField.setOnAction(event -> System.out.println("SET: " + textField.getText() + " | " + event.getSource()));
        /* III */ // textField.setOnAction(__ -> {/* do nothing */});

        CustomMenuItem custom = new CustomMenuItem(textField, false);

        menu.getItems().addAll(hello, world, custom);

        primaryStage.setScene(new Scene(menu));
        primaryStage.show();
    }
}

I'm using Java 8.

Any ideas?

Answer 1

The (at least part of) reason is that internals of ContextMenuContent (which acts as kind-of skin for the list of menuItems) get confused:

• it registers a keyHandler on ENTER that (in the end) fires the internally tracked current focused item
• the internal tracking doesn't work correctly on clicking into custom content

A hack around is to force the update of internal state (beware: requires access to hidden api!), f.i. in a mouseEntered handler of the textField.

Some code:

    TextField textField = new TextField();
    CustomMenuItem custom = new CustomMenuItem(textField, false);
    // menuItemContainer registers a mouseClicked handler that fires
    // need to consume before it reaches the container
    textField.addEventFilter(MouseEvent.MOUSE_CLICKED, e -> e.consume());
    // hack to update internal state of ContextMenuItem
    textField.addEventHandler(MouseEvent.MOUSE_ENTERED, e -> {
        ContextMenuContent cmContent = null;
        Node node = textField;
        while (node != null) {
            node = node.getParent();
            if (node instanceof ContextMenuContent) {
                cmContent = (ContextMenuContent) node;
                break;
            }
        }
        if (cmContent != null) {
            Parent menuItemContainer = textField.getParent();
            Parent menuBox = menuItemContainer.getParent();
            int index = menuBox.getChildrenUnmodifiable().indexOf(menuItemContainer);
            // hack internal state
            cmContent.requestFocusOnIndex(index);
        }
    });
    /* I */
    textField.addEventHandler(ActionEvent.ACTION, event -> {
        System.out.println("ADD: " + textField.getText() + " | "
                + event.getSource()
        );
        // consume to prevent item to fire twice
        event.consume();

    });

    custom.setOnAction(e -> {
        // someone needs to hide the popup
        // could be done in textField actionHandler as well
        if (custom.getParentPopup() != null) {
            custom.getParentPopup().hide();
        }
        System.out.println("action custom menu " + e.getSource());
    });

The bug is reported.

On further digging: the actual culprit seems to be MenuItemContainer (that is the container for a single item)

• for customMenuItems it registers a mouseEntered handler that requests focus on itself
• for menuItems of other types it registers a listener on its focusedProperty which updates the currentFocusedIndex of ContextMenuContent

A clean fix might be to register the focus listener for all items (except separators)

---

Digging a bit further, turned up another option/bug ;) The reason for the different behaviour of setOnAction vs addHandler(ActionEvent...) is some fishy code in TextFieldBehaviour:

@Override protected void fire(KeyEvent event) {
    TextField textField = getNode();
    EventHandler<ActionEvent> onAction = textField.getOnAction();
    ActionEvent actionEvent = new ActionEvent(textField, null);

    textField.commitValue();
    textField.fireEvent(actionEvent);
    // ---> this condition smells 
    if (onAction == null && !actionEvent.isConsumed()) {
        forwardToParent(event);
    }
}

I think the implementation intention was to forward only if either there's that special onAction handler or no normal handler had consumed it. Checking for consumed on the event always returns false, even if a handler had it consumed - because the event is copied during dispatch ... that is the handler changes the consumed on a copy, not the original.

Forwarding to parent fires the incorrect menuItem (due to the bug in internal book-keeping, see above), so looking for another way to prevent it:

protected void forwardToParent(KeyEvent event) {
    // fix for JDK-8145515
    if (getNode().getProperties().containsKey(DISABLE_FORWARD_TO_PARENT)) {
        return;
    }

    if (getNode().getParent() != null) {
        getNode().getParent().fireEvent(event);
    }
}

and indeed, adding that marker to the properties prevents firing the wrong item - without access to internal classes (though still highly implementation dependent ..):

textField.getProperties().put(
   "TextInputControlBehavior.disableForwardToParent", true);
textField.addEventHandler(ActionEvent.ACTION, event -> {