GT-3282 - Python - fixed intermittent docking action key binding failure

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/compositeeditor/CompositeEditorPanel.java

@@ -21,7 +21,8 @@ import java.awt.datatransfer.Transferable;
 import java.awt.dnd.*;
 import java.awt.event.*;
 import java.math.BigInteger;
-import java.util.*;
+import java.util.Arrays;
+import java.util.EventObject;
 
 import javax.swing.*;
 import javax.swing.border.Border;
@@ -31,7 +32,6 @@ import javax.swing.table.*;
 import javax.swing.text.JTextComponent;
 
 import docking.DockingWindowManager;
-import docking.action.DockingActionIf;
 import docking.actions.KeyBindingUtils;
 import docking.dnd.*;
 import docking.help.Help;
@@ -1506,33 +1506,6 @@ public abstract class CompositeEditorPanel extends JPanel
 			SwingUtilities.invokeLater(() -> component.requestFocus());
 			return component;
 		}
-
-		@Override
-		public boolean isKeyConsumed(KeyStroke keyStroke) {
-			if (isEditing()) {
-				// don't let actions through when editing our table
-				return true;
-			}
-
-			// TODO this should no longer be needed
-			return !hasLocalActionForKeyStroke(keyStroke);
-		}
-
-		private boolean hasLocalActionForKeyStroke(KeyStroke keyStroke) {
-			Plugin plugin = provider.getPlugin();
-			PluginTool tool = plugin.getTool();
-			Set<DockingActionIf> actions = tool.getDockingActionsByOwnerName(plugin.getName());
-			for (DockingActionIf action : actions) {
-				if (!(action instanceof CompositeEditorTableAction)) {
-					continue;
-				}
-				KeyStroke keyBinding = action.getKeyBinding();
-				if (keyStroke.equals(keyBinding)) {
-					return true;
-				}
-			}
-			return false;
-		}
 	}
 
 }

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/interpreter/InterpreterPanel.java

@@ -24,6 +24,7 @@ import javax.swing.*;
 import javax.swing.text.*;
 
 import docking.DockingUtils;
+import docking.actions.KeyBindingUtils;
 import generic.util.WindowUtilities;
 import ghidra.app.plugin.core.console.CodeCompletion;
 import ghidra.framework.options.OptionsChangeListener;
@@ -121,10 +122,12 @@ public class InterpreterPanel extends JPanel implements OptionsChangeListener {
 
 	private void build() {
 		outputTextPane = new JTextPane();
+		outputTextPane.setName("Interpreter Output Display");
 		outputScrollPane = new JScrollPane(outputTextPane);
 		outputScrollPane.setBorder(BorderFactory.createEmptyBorder());
 		promptTextPane = new JTextPane();
 		inputTextPane = new JTextPane();
+		inputTextPane.setName("Interpreter Input Field");
 
 		history = new HistoryManagerImpl();
 
@@ -167,7 +170,7 @@ public class InterpreterPanel extends JPanel implements OptionsChangeListener {
 					return text;
 				}
 				catch (BadLocationException e) {
-					Msg.error(this, "internal document positioning error", e);
+					Msg.error(this, "Interpreter document positioning error", e);
 				}
 				return "";
 			}
@@ -198,9 +201,8 @@ public class InterpreterPanel extends JPanel implements OptionsChangeListener {
 
 		outputTextPane.addKeyListener(new KeyListener() {
 			private void handleEvent(KeyEvent e) {
-				KeyboardFocusManager kfm = KeyboardFocusManager.getCurrentKeyboardFocusManager();
 
-				// If it's a copy, do nothing.  The copy will have worked.
+				// Ignore the copy event, as the output text pane knows how to copy its text
 				KeyStroke copyKeyStroke =
 					KeyStroke.getKeyStroke(KeyEvent.VK_C, DockingUtils.CONTROL_KEY_MODIFIER_MASK);
 				if (copyKeyStroke.equals(KeyStroke.getKeyStrokeForEvent(e))) {
@@ -208,7 +210,7 @@ public class InterpreterPanel extends JPanel implements OptionsChangeListener {
 				}
 
 				// Send everything else down to the inputTextPane.
-				kfm.redispatchEvent(inputTextPane, e);
+				KeyBindingUtils.retargetEvent(inputTextPane, e);
 			}
 
 			@Override

Ghidra/Framework/Docking/src/main/java/docking/AutoLookupKeyStrokeConsumer.java

@@ -1,51 +0,0 @@
-/* ###
- * IP: GHIDRA
- * REVIEWED: YES
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- * 
- *      http://www.apache.org/licenses/LICENSE-2.0
- * 
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package docking;
-
-import java.awt.event.InputEvent;
-import java.awt.event.KeyEvent;
-
-import javax.swing.KeyStroke;
-
-/**
- * An implementation of {@link KeyStrokeConsumer} that checks to see if a given {@link KeyStroke}
- * is valid for performing lookup on trees and tables.
- */
-public class AutoLookupKeyStrokeConsumer implements KeyStrokeConsumer {
-
-	@Override
-	public boolean isKeyConsumed(KeyStroke keyStroke) {
-		int modifier = keyStroke.getModifiers();
-		if ((modifier & InputEvent.CTRL_DOWN_MASK) == InputEvent.CTRL_DOWN_MASK) {
-			return false;
-		}
-
-		if ((modifier & InputEvent.META_DOWN_MASK) == InputEvent.META_DOWN_MASK) {
-			return false;
-		}
-
-		int code = keyStroke.getKeyCode();
-		if (code >= KeyEvent.VK_COMMA && code < KeyEvent.VK_DELETE) {
-			if (modifier == 0 ||
-				(modifier & InputEvent.SHIFT_DOWN_MASK) == InputEvent.SHIFT_DOWN_MASK) {
-				return true;
-			}
-		}
-		return false;
-	}
-
-}

Ghidra/Framework/Docking/src/main/java/docking/KeyBindingOverrideKeyEventDispatcher.java

@@ -15,8 +15,7 @@
  */
 package docking;
 
-import static docking.KeyBindingPrecedence.ActionMapLevel;
+import static docking.KeyBindingPrecedence.*;
-import static docking.KeyBindingPrecedence.DefaultLevel;
 
 import java.awt.*;
 import java.awt.event.KeyEvent;
@@ -37,7 +36,7 @@ import ghidra.util.exception.AssertException;
  * {@link #install()} must be called in order to install this <tt>Singleton</tt> into Java's 
  * key event processing system.
  */
-class KeyBindingOverrideKeyEventDispatcher implements KeyEventDispatcher {
+public class KeyBindingOverrideKeyEventDispatcher implements KeyEventDispatcher {
 
 	private static KeyBindingOverrideKeyEventDispatcher instance = null;
 
@@ -66,7 +65,7 @@ class KeyBindingOverrideKeyEventDispatcher implements KeyEventDispatcher {
 	 * Installs this key event dispatcher into Java's key event processing system.  Calling this
 	 * method more than once has no effect.
 	 */
-	public static void install() {
+	static void install() {
 		if (instance == null) {
 			instance = new KeyBindingOverrideKeyEventDispatcher();
 			KeyboardFocusManager kfm = KeyboardFocusManager.getCurrentKeyboardFocusManager();
@@ -221,15 +220,6 @@ class KeyBindingOverrideKeyEventDispatcher implements KeyEventDispatcher {
 		if (activeWindow instanceof DockingDialog) {
 			return false; // we don't want to process our key bindings when in DockingDialogs
 		}
-
-		Component focusOwner = focusProvider.getFocusOwner();
-		if (focusOwner instanceof KeyStrokeConsumer) {
-			KeyStrokeConsumer keyStrokeConsumer = (KeyStrokeConsumer) focusOwner;
-			if (keyStrokeConsumer.isKeyConsumed(keyStroke)) {
-				return false;
-			}
-		}
-
 		return true; // default case; allow it through
 	}
 

Ghidra/Framework/Docking/src/main/java/docking/KeyStrokeConsumer.java

@@ -1,34 +0,0 @@
-/* ###
- * IP: GHIDRA
- * REVIEWED: YES
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- * 
- *      http://www.apache.org/licenses/LICENSE-2.0
- * 
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package docking;
-
-import javax.swing.KeyStroke;
-
-/**
- * KeyActionConsumer identifies a Component which may want to limit the
- * use of actions associated with a KeyStroke when that Component has focus.
- */
-public interface KeyStrokeConsumer {
-
-	/**
-	 * Returns true when the specified key stroke will be consumed
-	 * and should not invoke an action.
-	 * @param keyStroke key stroke
-	 */
-	boolean isKeyConsumed(KeyStroke keyStroke);
-	
-}

Ghidra/Framework/Docking/src/main/java/docking/action/ShowFocusInfoAction.java

@@ -105,7 +105,14 @@ public class ShowFocusInfoAction extends DockingAction {
 				System.identityHashCode(printComponent);
 		}
 
-		return printComponent.getClass().getName() + ": " + System.identityHashCode(printComponent);
+		String name = "";
+		String componentName = printComponent.getName();
+		if (componentName != null) {
+			name = " - '" + componentName + "' ";
+		}
+
+		return printComponent.getClass().getName() + name + ": " +
+			System.identityHashCode(printComponent);
 	}
 
 }

Ghidra/Framework/Docking/src/main/java/docking/actions/KeyBindingUtils.java

@@ -15,7 +15,7 @@
  */
 package docking.actions;
 
-import static org.apache.commons.lang3.StringUtils.indexOfIgnoreCase;
+import static org.apache.commons.lang3.StringUtils.*;
 
 import java.awt.Component;
 import java.awt.KeyboardFocusManager;
@@ -173,6 +173,28 @@ public class KeyBindingUtils {
 		}
 	}
 
+	/**
+	 * Changes the given key event to the new source component and then dispatches that event.
+	 * This method is intended for clients that wish to effectively take a key event given to 
+	 * one component and give it to another component.  This is seldom-used code; if you don't
+	 * know when to use this code, then don't.
+	 * 
+	 * @param newSource the new target of the event
+	 * @param e the existing event
+	 */
+	public static void retargetEvent(Component newSource, KeyEvent e) {
+
+		if (e.getSource() == newSource) {
+			return; // yes '=='; prevent recursion 
+		}
+
+		KeyEvent newEvent = new KeyEvent(newSource, e.getID(), e.getWhen(), e.getModifiersEx(),
+			e.getKeyCode(), e.getKeyChar(), e.getKeyLocation());
+		e.consume();
+		KeyboardFocusManager kfm = KeyboardFocusManager.getCurrentKeyboardFocusManager();
+		kfm.dispatchEvent(newEvent);
+	}
+
 	/**
 	 * A convenience method to register the given action with the given
 	 * component. This is not usually done, as the action system is usually
@@ -678,7 +700,7 @@ public class KeyBindingUtils {
 	 *    ctrl Z
 	 * </pre>  
 	 * 
-	 * @param keyStroke
+	 * @param keyStroke the key stroke
 	 * @return the new key stroke (as returned by  {@link KeyStroke#getKeyStroke(String)}
 	 */
 	public static KeyStroke parseKeyStroke(String keyStroke) {

Ghidra/Framework/Docking/src/main/java/docking/widgets/table/GTable.java

@@ -70,7 +70,7 @@ import resources.ResourceManager;
  *
  * @see GTableFilterPanel
  */
-public class GTable extends JTable implements KeyStrokeConsumer {
+public class GTable extends JTable {
 
 	private static final KeyStroke COPY_KEY_STROKE =
 		KeyStroke.getKeyStroke(KeyEvent.VK_C, CONTROL_KEY_MODIFIER_MASK);
@@ -89,8 +89,6 @@ public class GTable extends JTable implements KeyStrokeConsumer {
 	private long lastLookupTime;
 	private String lookupString;
 	private int lookupColumn = -1;
-	private AutoLookupKeyStrokeConsumer autoLookupKeyStrokeConsumer =
-		new AutoLookupKeyStrokeConsumer();
 
 	/** A list of default renderers created by this table */
 	protected List<TableCellRenderer> defaultGTableRendererList = new ArrayList<>();
@@ -397,6 +395,10 @@ public class GTable extends JTable implements KeyStrokeConsumer {
 			autoLookupListener = new KeyAdapter() {
 				@Override
 				public void keyPressed(KeyEvent e) {
+					if (!allowActions) {
+						return;
+					}
+
 					if (getRowCount() == 0) {
 						return;
 					}
@@ -421,21 +423,6 @@ public class GTable extends JTable implements KeyStrokeConsumer {
 					}
 					lastLookupTime = when;
 				}
-
-				private boolean isIgnorableKeyEvent(KeyEvent event) {
-					// ignore modified keys
-					if (event.isAltDown() || event.isAltGraphDown() || event.isControlDown() ||
-						event.isMetaDown()) {
-						return true;
-					}
-
-					if (event.isActionKey() || event.getKeyChar() == KeyEvent.CHAR_UNDEFINED ||
-						Character.isISOControl(event.getKeyChar())) {
-						return true;
-					}
-
-					return false;
-				}
 			};
 		}
 
@@ -447,6 +434,30 @@ public class GTable extends JTable implements KeyStrokeConsumer {
 		}
 	}
 
+	private boolean isIgnorableKeyEvent(KeyEvent event) {
+
+		// ignore modified keys, except for SHIFT
+		if (!isUnmodifiedOrShift(event.getModifiersEx())) {
+			return true;
+		}
+
+		if (event.isActionKey() || event.getKeyChar() == KeyEvent.CHAR_UNDEFINED ||
+			Character.isISOControl(event.getKeyChar())) {
+			return true;
+		}
+
+		return false;
+	}
+
+	private boolean isUnmodifiedOrShift(int modifiers) {
+		if (modifiers == 0) {
+			return true;
+		}
+
+		int shift = InputEvent.SHIFT_DOWN_MASK;
+		return (modifiers | shift) != shift;
+	}
+
 	/**
 	 * Enables the keyboard actions to pass through this table and up the component hierarchy.
 	 * Specifically, passing true to this method allows unmodified keystrokes to work
@@ -463,24 +474,6 @@ public class GTable extends JTable implements KeyStrokeConsumer {
 		allowActions = b;
 	}
 
-	/**
-	 * This method is implemented to signal interest in any typed text that may help the user
-	 * change the row in the table.  For example, if the user types 'a', then the table will move
-	 * to the first symbol that begins with the letter 'a'.  This method also wants to handle
-	 * text when the 'shift' key is down.  This method will return false if the control key is
-	 * pressed.
-	 *
-	 * @see docking.KeyStrokeConsumer#isKeyConsumed(javax.swing.KeyStroke)
-	 */
-	@Override
-	public boolean isKeyConsumed(KeyStroke keyStroke) {
-		if (allowActions) {
-			return false;
-		}
-
-		return autoLookupKeyStrokeConsumer.isKeyConsumed(keyStroke);
-	}
-
 	/**
 	 * Enables or disables auto-edit.  When enabled, the user can start typing to trigger an
 	 * edit of an editable table cell.

Ghidra/Framework/Graph/src/main/java/ghidra/graph/viewer/GraphComponent.java

@@ -26,6 +26,7 @@ import com.google.common.base.Function;
 
 import docking.DockingUtils;
 import docking.DockingWindowManager;
+import docking.actions.KeyBindingUtils;
 import docking.help.HelpService;
 import docking.widgets.EmptyBorderButton;
 import docking.widgets.PopupWindow;
@@ -1028,15 +1029,8 @@ public class GraphComponent<V extends VisualVertex, E extends VisualEdge<V>, G e
 				return;
 			}
 
-			KeyboardFocusManager kfm = KeyboardFocusManager.getCurrentKeyboardFocusManager();
+			KeyBindingUtils.retargetEvent(focusedVertex.getComponent(), e);
-			KeyEvent clonedKeyEvent = cloneKeyEvent(e, focusedVertex.getComponent());
-			kfm.redispatchEvent(focusedVertex.getComponent(), clonedKeyEvent);
-
 			viewer.repaint();
-
-			if (clonedKeyEvent.isConsumed()) {
-				e.consume();
-			}
 		}
 
 		@Override
@@ -1046,15 +1040,8 @@ public class GraphComponent<V extends VisualVertex, E extends VisualEdge<V>, G e
 				return;
 			}
 
-			KeyboardFocusManager kfm = KeyboardFocusManager.getCurrentKeyboardFocusManager();
+			KeyBindingUtils.retargetEvent(focusedVertex.getComponent(), e);
-			KeyEvent clonedKeyEvent = cloneKeyEvent(e, focusedVertex.getComponent());
-			kfm.redispatchEvent(focusedVertex.getComponent(), clonedKeyEvent);
-
 			viewer.repaint();
-
-			if (clonedKeyEvent.isConsumed()) {
-				e.consume();
-			}
 		}
 
 		@Override
@@ -1064,21 +1051,8 @@ public class GraphComponent<V extends VisualVertex, E extends VisualEdge<V>, G e
 				return;
 			}
 
-			KeyboardFocusManager kfm = KeyboardFocusManager.getCurrentKeyboardFocusManager();
+			KeyBindingUtils.retargetEvent(focusedVertex.getComponent(), e);
-			KeyEvent clonedKeyEvent = cloneKeyEvent(e, focusedVertex.getComponent());
-			kfm.redispatchEvent(focusedVertex.getComponent(), clonedKeyEvent);
-
 			viewer.repaint();
-
-			if (clonedKeyEvent.isConsumed()) {
-				e.consume();
-			}
-		}
-
-		private KeyEvent cloneKeyEvent(KeyEvent keyEvent, Component newSource) {
-			return new KeyEvent(newSource, keyEvent.getID(), keyEvent.getWhen(),
-				keyEvent.getModifiersEx(), keyEvent.getKeyCode(), keyEvent.getKeyChar(),
-				keyEvent.getKeyLocation());
 		}
 	}