ContextMenu

ContextMenu with Accelerators

The Problem

The naive coding approach:

// create and configure menu
ContextMenu c = new ContentMenu();
c.getItems().addAll(itemsWithAccelerators);
// install on a control
control.setContextMenu(c);

Run, press the accelerator and see a menuItem’s onAction handler triggered, just as expected.

// create and configure as above ...
// set eventhandler
node.setOnContextMenuRequested(e -> c.show(node, x, y));

Again, press the accelerator and see .. nothing happen. The technical reason is well explained in JDK-8094409 - accelerators are a Scene-level concept and must be registered with the Scene, once it is known.

Controls or other contextMenu-aware collaborators (like TableColumn, Tab) do so behind the scenes. For all other parts of the the scenegraph, developers need to take over that registration themselves. Note that this fact is undocumented. And there is no obvious place to document it.

The Solution

The fix of JDK-8094409 is a helper class ControlAcceleratorSupport that takes over:

ControlAcceleratorSupport.addAcceleratorToScene(c.getItems(), node);

Unfortunately, it resides in the internal package com.sun.javafx.scene.control (and will remain there for the release of fx9) thus can’t be used in constrained production contexts.

The Solution Revisited

On the other hand: not being included in the first release of fx9 has the chance of re-visiting the decision in favor of a helper class vs providing API on the ContextMenu that takes the anchor node (option 2 in the fixed bug, the helper class was option 3). Should be settable in the constructor or a setter/property, then a working snippet would be:

// create and configure as in the first snippet
c.setAnchor(node)
node.setOnContextMenuRequested(...)

A helper class or not: both require the developer to do something in addition to simply adding the menuItems to the menu, otherwise accelerators simply won’t work! A direct comparison (to get the feel :-)

// either Option 2: on ContextMenu
c.setAnchor(node)
// or Option 3: helper
ControlAcceleratorSupport.addAcceleratorToScene(c.getItems(), node);

To me, option 2 looks much better - sparing me some implementation details - so why choose option 3? Quoting the analysis (excerpting and bulleting the contra arguments)

Option 2: We could add additional API into ContextMenu such that the anchor node is passed in to the constructor

1. Of course, we can’t get rid of the old constructors, so the best we can do is log an error when menu items with accelerators are given to a ContextMenu that does not have the anchor set.

2. If we go down this path we should also consider duplicating the ContextMenu show methods to include versions without the anchor node in the argument list. This clearly leads to duplicated and confusing API…​.

The first isn’t that important, given that it’s the same (minus the possibility to at least log an error) malfunction if application does not/cannot use the undocumented and hidden helper.

For the second, let’s have a look at the methods related to showing:

// from Window
protected void show();
// from PopupWindow
public void show(Window);
public void show(Window, double, double);
public void show(Node, double, double);
// from ContextMenu
public void show(Node, Side, double, double);
// additional api if the ContextMenu would "know" its anchor
public void show(double, double);
public void show(Side, double, double);