This is an automated email from the ASF dual-hosted git repository.
hansva pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/hop.git
The following commit(s) were added to refs/heads/main by this push:
new 2ae7cb3159 Fix metadata items not opening in Hop Web, fixes #8151
(#8153)
2ae7cb3159 is described below
commit 2ae7cb3159c10aa5a60a6b684f5e1a4724de9786
Author: Bart Maertens <[email protected]>
AuthorDate: Fri Aug 28 17:12:41 2026 +0200
Fix metadata items not opening in Hop Web, fixes #8151 (#8153)
The key handler is attached to every widget from PropsUi.setLook(), so it
also covers widgets that are created after their shell was set up. Composite
widgets that pass their listeners on to a control inside them are skipped:
they style themselves before creating that control, so it is not there yet,
and it gets the handler through its own setLook() call anyway.
That check only recognised widgets overriding addKeyListener(). Widgets like
MetaSelectionLine and StyledTextComp override the addListener() that
Control.addKeyListener() ends up calling instead, so they were not skipped
and opening a metadata editor threw a NullPointerException on the control
that was not created yet.
Recognise both ways of delegating. Only Hop Web was affected: RAP's
Control.addKeyListener() calls the public addListener(), while SWT on the
desktop routes it through the internal _addListener() that subclasses cannot
override.
Co-authored-by: Claude Opus 5 (1M context) <[email protected]>
---
.../org/apache/hop/ui/hopgui/HopGuiKeyHandler.java | 31 +++++++++++++++-------
.../apache/hop/ui/hopgui/HopGuiKeyHandlerTest.java | 20 ++++++++++++++
2 files changed, 42 insertions(+), 9 deletions(-)
diff --git a/ui/src/main/java/org/apache/hop/ui/hopgui/HopGuiKeyHandler.java
b/ui/src/main/java/org/apache/hop/ui/hopgui/HopGuiKeyHandler.java
index 2d8dd4b7f9..1aeaf512c0 100644
--- a/ui/src/main/java/org/apache/hop/ui/hopgui/HopGuiKeyHandler.java
+++ b/ui/src/main/java/org/apache/hop/ui/hopgui/HopGuiKeyHandler.java
@@ -42,6 +42,7 @@ import org.eclipse.swt.widgets.Combo;
import org.eclipse.swt.widgets.Composite;
import org.eclipse.swt.widgets.Control;
import org.eclipse.swt.widgets.Display;
+import org.eclipse.swt.widgets.Listener;
import org.eclipse.swt.widgets.Shell;
import org.eclipse.swt.widgets.Table;
import org.eclipse.swt.widgets.TableItem;
@@ -182,21 +183,33 @@ public class HopGuiKeyHandler extends KeyAdapter {
* Composite widgets like TextVar and ComboVar pass the key listeners they
get on to the widget
* inside them. They style themselves before creating that widget, so it is
not always there yet,
* and it gets the handler through its own {@code PropsUi.setLook()} call
anyway.
+ *
+ * <p>Both ways of delegating count: overriding {@code addKeyListener()} and
overriding the {@code
+ * addListener()} that {@link Control#addKeyListener(KeyListener)} ends up
calling. Widgets like
+ * MetaSelectionLine and StyledTextComp only do the latter.
*/
private static boolean delegatesKeyListeners(Control control) {
if (!(control instanceof Composite)) {
return false;
}
return DELEGATING_KEY_LISTENERS.computeIfAbsent(
- control.getClass(),
- widgetClass -> {
- try {
- Method method = widgetClass.getMethod("addKeyListener",
KeyListener.class);
- return !Control.class.equals(method.getDeclaringClass());
- } catch (NoSuchMethodException e) {
- return Boolean.FALSE;
- }
- });
+ control.getClass(), HopGuiKeyHandler::isDelegatingClass);
+ }
+
+ private static Boolean isDelegatingClass(Class<?> widgetClass) {
+ return isOverridden(widgetClass, Control.class, "addKeyListener",
KeyListener.class)
+ || isOverridden(widgetClass, Widget.class, "addListener", int.class,
Listener.class);
+ }
+
+ /** Is the method of the given widget class declared below the class that
normally declares it? */
+ private static boolean isOverridden(
+ Class<?> widgetClass, Class<?> declaringClass, String name, Class<?>...
parameterTypes) {
+ try {
+ Method method = widgetClass.getMethod(name, parameterTypes);
+ return !declaringClass.equals(method.getDeclaringClass());
+ } catch (NoSuchMethodException e) {
+ return false;
+ }
}
/** The terminal widget and everything in it handles all keys itself. */
diff --git
a/ui/src/test/java/org/apache/hop/ui/hopgui/HopGuiKeyHandlerTest.java
b/ui/src/test/java/org/apache/hop/ui/hopgui/HopGuiKeyHandlerTest.java
index 7cab8c20df..8b125c2cbb 100644
--- a/ui/src/test/java/org/apache/hop/ui/hopgui/HopGuiKeyHandlerTest.java
+++ b/ui/src/test/java/org/apache/hop/ui/hopgui/HopGuiKeyHandlerTest.java
@@ -33,6 +33,8 @@ import org.apache.hop.core.gui.plugin.GuiRegistry;
import org.apache.hop.core.gui.plugin.key.GuiKeyboardShortcut;
import org.apache.hop.core.gui.plugin.key.GuiOsxKeyboardShortcut;
import org.apache.hop.core.gui.plugin.key.KeyboardShortcut;
+import org.apache.hop.ui.core.widget.MetaSelectionLine;
+import org.apache.hop.ui.core.widget.StyledTextComp;
import org.apache.hop.ui.core.widget.TextVar;
import org.apache.hop.ui.hopgui.file.pipeline.HopGuiPipelineGraph;
import org.apache.hop.ui.hopgui.file.workflow.HopGuiWorkflowGraph;
@@ -213,6 +215,24 @@ class HopGuiKeyHandlerTest {
verify(textVar, never()).addKeyListener(keyHandler);
}
+ @Test
+ void doesNotAttachToWidgetsThatOnlyPassOnTheirListeners() {
+ // MetaSelectionLine and StyledTextComp delegate addListener() instead of
addKeyListener(), so
+ // they were not recognised and opening a metadata editor threw a
NullPointerException.
+ HopGuiKeyHandler keyHandler = HopGuiKeyHandler.getInstance();
+ Shell shell = mock(Shell.class);
+ MetaSelectionLine<?> metaSelectionLine = mock(MetaSelectionLine.class);
+ when(metaSelectionLine.getShell()).thenReturn(shell);
+ StyledTextComp styledTextComp = mock(StyledTextComp.class);
+ when(styledTextComp.getShell()).thenReturn(shell);
+ keyHandler.addHandledShell(null, shell);
+
+ assertFalse(keyHandler.attachTo(metaSelectionLine));
+ verify(metaSelectionLine, never()).addKeyListener(keyHandler);
+ assertFalse(keyHandler.attachTo(styledTextComp));
+ verify(styledTextComp, never()).addKeyListener(keyHandler);
+ }
+
@Test
void doesNotAttachToTheTerminalWidget() {
HopGuiKeyHandler keyHandler = HopGuiKeyHandler.getInstance();
