From 5afc1bebec7b5da270a6a648b7677f4de819b7da Mon Sep 17 00:00:00 2001 From: dragonmacher <48328597+dragonmacher@users.noreply.github.com> Date: Wed, 3 Mar 2021 14:34:04 -0500 Subject: [PATCH] GP-747 - Keybinding 'fix' for difference in key codes for Left/Right Alt keys on some OSes Closes #2008 --- .../plugintool/dialog/KeyBindingsTest.java | 40 +++++++++++++++++++ .../docking/action/KeyBindingsManager.java | 39 +++++++++++++++++- 2 files changed, 77 insertions(+), 2 deletions(-) diff --git a/Ghidra/Features/Base/src/test.slow/java/ghidra/framework/plugintool/dialog/KeyBindingsTest.java b/Ghidra/Features/Base/src/test.slow/java/ghidra/framework/plugintool/dialog/KeyBindingsTest.java index 7b90f6c136..5c339b36bb 100644 --- a/Ghidra/Features/Base/src/test.slow/java/ghidra/framework/plugintool/dialog/KeyBindingsTest.java +++ b/Ghidra/Features/Base/src/test.slow/java/ghidra/framework/plugintool/dialog/KeyBindingsTest.java @@ -27,10 +27,12 @@ import javax.swing.table.*; import org.junit.*; +import docking.DockingWindowManager; import docking.KeyEntryTextField; import docking.action.DockingActionIf; import docking.tool.util.DockingToolConstants; import docking.widgets.MultiLineLabel; +import generic.test.TestUtils; import ghidra.app.plugin.core.codebrowser.CodeBrowserPlugin; import ghidra.framework.plugintool.PluginTool; import ghidra.test.AbstractGhidraHeadedIntegrationTest; @@ -233,6 +235,44 @@ public class KeyBindingsTest extends AbstractGhidraHeadedIntegrationTest { assertEquals(KeyStroke.getKeyStroke(KeyEvent.VK_PAGE_UP, 0), getKeyStroke(action1)); } + @Test + public void testSetKeyBinding_AltGraphFix() throws Exception { + + // + // This test is verifying a hack that was put in to fix the difference in 'Alt' key handling + // on Windows (https://bugs.openjdk.java.net/browse/JDK-8194873). + // Create an action and set the keybinding to use the 'Alt' modifier. + // Verify that the action will also get mapped to the 'Alt Graph' modifier. + // + + // verify that no action is mapped to the new binding + int keyCode = KeyEvent.VK_0; + int modifiers = InputEvent.ALT_DOWN_MASK | InputEvent.ALT_GRAPH_DOWN_MASK; + KeyEvent keyEvent = + new KeyEvent(dialog, KeyEvent.KEY_PRESSED, System.currentTimeMillis(), modifiers, + keyCode, KeyEvent.CHAR_UNDEFINED); + KeyStroke keyStroke = KeyStroke.getKeyStrokeForEvent(keyEvent); + DockingWindowManager dwm = DockingWindowManager.getActiveInstance(); + Action action = + (Action) TestUtils.invokeInstanceMethod("getActionForKeyStroke", dwm, keyStroke); + assertNull(action); + + // set the new binding that uses the 'Alt' key + selectRowForAction(action1); + triggerActionKey(keyField, InputEvent.ALT_DOWN_MASK, keyCode); + String keyStrokeString = KeyEntryTextField.parseKeyStroke( + KeyStroke.getKeyStroke(keyCode, InputEvent.ALT_DOWN_MASK)); + assertEquals(keyStrokeString, keyField.getText()); + apply(); + assertEquals(KeyStroke.getKeyStroke(keyCode, InputEvent.ALT_DOWN_MASK), + getKeyStroke(action1)); + + // verify the additional binding for 'Alt Graph' + action = + (Action) TestUtils.invokeInstanceMethod("getActionForKeyStroke", dwm, keyStroke); + assertNotNull(action); + } + @Test public void testClearKeyBinding1() throws Exception { diff --git a/Ghidra/Framework/Docking/src/main/java/docking/action/KeyBindingsManager.java b/Ghidra/Framework/Docking/src/main/java/docking/action/KeyBindingsManager.java index e433c7705a..f2e9140e5c 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/action/KeyBindingsManager.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/action/KeyBindingsManager.java @@ -15,6 +15,7 @@ */ package docking.action; +import java.awt.event.InputEvent; import java.beans.PropertyChangeEvent; import java.beans.PropertyChangeListener; import java.util.*; @@ -76,14 +77,48 @@ public class KeyBindingsManager implements PropertyChangeListener { private void addKeyBinding(ComponentProvider provider, DockingActionIf action, KeyStroke keyStroke) { + if (ReservedKeyBindings.isReservedKeystroke(keyStroke)) { throw new AssertException("Cannot assign action to a reserved keystroke. " + "Action: " + action.getName() + " - Keystroke: " + keyStroke); } - DockingKeyBindingAction existingAction = dockingKeyMap.get(keyStroke); + // map standard keybinding to action + doAddKeyBinding(provider, action, keyStroke); + + fixupAltGraphKeyStrokeMapping(provider, action, keyStroke); + } + + private void fixupAltGraphKeyStrokeMapping(ComponentProvider provider, DockingActionIf action, + KeyStroke keyStroke) { + + // special case + int modifiers = keyStroke.getModifiers(); + if ((modifiers & InputEvent.ALT_DOWN_MASK) == InputEvent.ALT_DOWN_MASK) { + // + // Also register the 'Alt' binding with the 'Alt Graph' mask. This fixes the but + // on Windows (https://bugs.openjdk.java.net/browse/JDK-8194873) + // that have different key codes for the left and right Alt keys. + // + modifiers |= InputEvent.ALT_GRAPH_DOWN_MASK; + KeyStroke updateKeyStroke = + KeyStroke.getKeyStroke(keyStroke.getKeyCode(), modifiers, false); + doAddKeyBinding(provider, action, updateKeyStroke, keyStroke); + } + } + + private void doAddKeyBinding(ComponentProvider provider, DockingActionIf action, + KeyStroke keyStroke) { + doAddKeyBinding(provider, action, keyStroke, keyStroke); + } + + private void doAddKeyBinding(ComponentProvider provider, DockingActionIf action, + KeyStroke mappingKeyStroke, KeyStroke actionKeyStroke) { + + DockingKeyBindingAction existingAction = dockingKeyMap.get(mappingKeyStroke); if (existingAction == null) { - dockingKeyMap.put(keyStroke, new MultipleKeyAction(tool, provider, action, keyStroke)); + dockingKeyMap.put(mappingKeyStroke, + new MultipleKeyAction(tool, provider, action, actionKeyStroke)); return; }