From 3b3b7259bc9f6017e71b26e51e1e3d0eee017682 Mon Sep 17 00:00:00 2001 From: dragonmacher <48328597+dragonmacher@users.noreply.github.com> Date: Mon, 25 Nov 2019 17:55:07 -0500 Subject: [PATCH] Tests - fixed flaw in key event processing for non-attached UI components --- .../help/topics/ConsolePlugin/console.html | 1 - .../help/help/topics/Python/interpreter.html | 1 - .../java/docking/actions/KeyBindingUtils.java | 49 +++++++++++++++++-- 3 files changed, 45 insertions(+), 6 deletions(-) diff --git a/Ghidra/Features/Base/src/main/help/help/topics/ConsolePlugin/console.html b/Ghidra/Features/Base/src/main/help/help/topics/ConsolePlugin/console.html index d6150e20b8..7480eb9e09 100644 --- a/Ghidra/Features/Base/src/main/help/help/topics/ConsolePlugin/console.html +++ b/Ghidra/Features/Base/src/main/help/help/topics/ConsolePlugin/console.html @@ -53,6 +53,5 @@

Provided by: ConsolePlugin

-

Related Topics:

diff --git a/Ghidra/Features/Python/src/main/help/help/topics/Python/interpreter.html b/Ghidra/Features/Python/src/main/help/help/topics/Python/interpreter.html index a351b93db0..ddc272d65e 100644 --- a/Ghidra/Features/Python/src/main/help/help/topics/Python/interpreter.html +++ b/Ghidra/Features/Python/src/main/help/help/topics/Python/interpreter.html @@ -165,6 +165,5 @@

Provided by: PythonPlugin

-

Related Topics:

diff --git a/Ghidra/Framework/Docking/src/main/java/docking/actions/KeyBindingUtils.java b/Ghidra/Framework/Docking/src/main/java/docking/actions/KeyBindingUtils.java index 793de40b0b..b7b48bfee9 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/actions/KeyBindingUtils.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/actions/KeyBindingUtils.java @@ -176,8 +176,14 @@ 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. + * one component and give it to another component. + * + *

This method exists to deal with the complicated nature of key event processing and + * how our (not Java's) framework processes key event bindings to trigger actions. If not + * for our special processing of action key bindings, then this method would not be + * necessary. + * + *

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 @@ -190,9 +196,44 @@ public class KeyBindingUtils { KeyEvent newEvent = new KeyEvent(newSource, e.getID(), e.getWhen(), e.getModifiersEx(), e.getKeyCode(), e.getKeyChar(), e.getKeyLocation()); - e.consume(); + + /* + Unusual Code Alert! + + The KeyboardFocusManager is a complicated beast. Here we use knowledge of one such + complication to correctly route key events. If the client of this method passes + a component whose 'isShowing()' returns false, then the manager will not send the + event to that component. Almost all clients will pass fully attached/realized + components to the manager. We, however, will sometimes pass components that are not + attached; for example, when we are using said components with a renderer to perform + our own painting. In the case of non-attached components, we must call the + redispatchEvent() method ourselves. + + Why don't we just always call redispatchEvent()? Well, that + method will not pass the new cloned event we just created back through the full + key event pipeline. This means that tool-level (our Tool API, not Java) + actions will not work, as tool-level actions are handled at the beginning of the + key event pipeline, not by the components themselves. + + Also, we have here guilty knowledge that the aforementioned tool-level key processing + will check to see if the event was consumed. If consumed, then no further processing + will happen; if not consumed, then the framework will continue to process the event + passed into this method. Thus, after we send the new event, we will update the + original event to match the consumed state of our new event. This means that the + component passed to this method must, somewhere in its processing, consume the key + event we dispatch here, if they do not wish for any further processing to take place. + */ KeyboardFocusManager kfm = KeyboardFocusManager.getCurrentKeyboardFocusManager(); - kfm.dispatchEvent(newEvent); + if (newSource.isShowing()) { + kfm.dispatchEvent(newEvent); + } + else { + kfm.redispatchEvent(newSource, newEvent); + } + + if (newEvent.isConsumed()) { + e.consume(); + } } /**