Fix accelerator keybindings for the tab menu

Some items in this menu had accelerator keys (shortcuts) defined.
Normally, this automatically takes care of registering such keybindings
when the menu item is added to a menu. However, this requires adding the
item (indirectly) to a menubar, which is again added to a window. Since
the tab menu is just a separate popup menu, this did not work.

It seems an attempt was made to fix this by adding the popup menu to the
EditorHeader JComponent, which indeed made the keybindings work.
However, this is a hack at best, and as soon as the popup menu was
opened, it would be moved to another container and again detached when
the menu was closed, breaking the keyboard shortcuts again (re-adding
the popup menu turned out not to work either, then the menu would
actually be drawn on top of the tab bar).

To properly fix this, keybindings for the menu items are added to the
EditorHeader itself. By looking at the existing accelerator keystroke
property of the actions, there is no need to duplicate the keystrokes
themselves, and the displayed value will always match the actually bound
value. To simplify this, some methods are added to the Keys helper
class, which will likely come in handy in other places as well.

Author: matthijskooijman

app/src/processing/app/EditorHeader.java

@@ -102,6 +102,15 @@ public class Actions {
     public final Action nextTab = new SimpleAction(tr("Next Tab"),
         Keys.ctrlAlt(KeyEvent.VK_RIGHT),
         () -> editor.sketch.handleNextCode());
+
+    Actions() {
+      // Explicitly bind keybindings for the actions with accelerators above
+      // Normally, this happens automatically for any actions bound to menu
+      // items, but only for menus attached to a window, not for popup menus.
+      Keys.bind(EditorHeader.this, newTab);
+      Keys.bind(EditorHeader.this, prevTab);
+      Keys.bind(EditorHeader.this, nextTab);
+    }
   }
   public Actions actions = new Actions();
 
@@ -269,7 +278,6 @@ public void rebuildMenu() {
       menu = new JMenu();
       MenuScroller.setScrollerFor(menu);
       popup = menu.getPopupMenu();
-      add(popup);
       popup.setLightWeightPopupEnabled(true);
     }
     JMenuItem item;

app/src/processing/app/helpers/Keys.java

@@ -42,6 +42,91 @@
  */
 public class Keys {
 
+  /**
+   * Register a keybinding in the given components WHEN_IN_FOCUSED_WINDOW input
+   * map, runing the given action when the action's accelerator key is pressed.
+   *
+   * Note that this is typically automatically handled when the action is
+   * assigned to a JMenuItem, but it can still be needed for other actions, or
+   * actions mapped to JMenuItems in a popup menu that is not added to any
+   * window normally (and thus does not fire when the popup is closed).
+   *
+   * When the action is disabled, the keybinding is unregistered, and when it is
+   * enabled, it is registered again.
+   */
+  public static void bind(final JComponent component, final Action action) {
+    bind(component, action,
+         (KeyStroke) action.getValue(Action.ACCELERATOR_KEY));
+  }
+
+  /**
+   * Register a keybinding, running the given action when the given keystroke is
+   * pressed when the given component is in the focused window.
+   *
+   * This is typically used to bind an additional keystroke to a menu item, in
+   * addition to the primary accelerator key.
+   *
+   * When the action is disabled, the keybinding is unregistered, and when it is
+   * enabled, it is registered again.
+   */
+  public static void bind(final JComponent component, final Action action,
+                          KeyStroke keystroke) {
+    bind(component, action, keystroke, JComponent.WHEN_IN_FOCUSED_WINDOW);
+  }
+
+  /**
+   * Register a keybinding to be handled in given condition, running the given
+   * action when the given keystroke is pressed.
+   *
+   * When the action is disabled, the keybinding is unregistered, and when it is
+   * enabled, it is registered again.
+   *
+   * @param component
+   *          The component to register the keybinding on.
+   * @param action
+   *          The action to run when the keystroke is pressed
+   * @param action
+   *          The keystroke to bind
+   * @param condition
+   *          The condition under which to run the keystroke. Should be one of
+   *          JComponent.WHEN_FOCUSED,
+   *          JComponent.WHEN_ANCESTOR_OF_FOCUSED_COMPONENT or
+   *          JComponent.WHEN_IN_FOCUSED_WINDOW.
+   */
+  public static void bind(final JComponent component, final Action action,
+                          KeyStroke keystroke, int condition) {
+    // The input map maps keystrokes to arbitrary objects (originally strings
+    // that described the option, we just use the Action object itself).
+    if (action.isEnabled())
+      enableBind(component, action, keystroke, condition);
+
+    // The action map maps the arbitrary option to an Action to execute. These
+    // be kept in the component even when the action is disabled.
+    component.getActionMap().put(action, action);
+
+    // Enable and disable the binding when the action is enabled / disabled.
+    action.addPropertyChangeListener((PropertyChangeEvent e) -> {
+      if (e.getPropertyName().equals("enabled")) {
+        if (e.getNewValue().equals(Boolean.TRUE))
+          enableBind(component, action, keystroke, condition);
+        else
+          disableBind(component, action, keystroke, condition);
+      }
+    });
+  }
+
+  private static void enableBind(final JComponent component,
+                                 final Action action, final KeyStroke keystroke,
+                                 int condition) {
+    component.getInputMap(condition).put(keystroke, action);
+  }
+
+  private static void disableBind(final JComponent component,
+                                  final Action action,
+                                  final KeyStroke keystroke, int condition) {
+    component.getInputMap(condition).put(keystroke, action);
+  }
+
   private static final int CTRL = Toolkit.getDefaultToolkit()
       .getMenuShortcutKeyMask();