GP-3115 - Fixed focus issue with editing data types in the Function Signature Editor dialog

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/FunctionEditorDialog.java

@@ -25,6 +25,7 @@ import javax.swing.*;
 import javax.swing.border.Border;
 import javax.swing.border.CompoundBorder;
 import javax.swing.event.*;
+import javax.swing.plaf.TableUI;
 import javax.swing.table.TableCellEditor;
 
 import org.apache.commons.lang3.StringUtils;
@@ -423,15 +424,7 @@ public class FunctionEditorDialog extends DialogComponentProvider implements Mod
 		paramTableModel = new ParameterTableModel(model);
 		parameterTable = new ParameterTable(paramTableModel);
 		selectionListener = e -> model.setSelectedParameterRow(parameterTable.getSelectedRows());
-		parameterTable.getSelectionModel().addListSelectionListener(selectionListener);
+
-		// set the preferred viewport height smaller that the button panel, otherwise it is huge!
-		parameterTable.setPreferredScrollableViewportSize(new Dimension(600, 100));
-		parameterTable.setDefaultEditor(DataType.class,
-			new ParameterDataTypeCellEditor(this, service));
-		parameterTable.setDefaultRenderer(DataType.class, new ParameterDataTypeCellRenderer());
-		parameterTable.setDefaultEditor(VariableStorage.class, new StorageTableCellEditor(model));
-		parameterTable.setDefaultRenderer(VariableStorage.class, new VariableStorageCellRenderer());
-		parameterTable.setDefaultRenderer(String.class, new VariableStringCellRenderer());
 		JScrollPane tableScroll = new JScrollPane(parameterTable);
 		panel.add(tableScroll, BorderLayout.CENTER);
 		panel.add(buildButtonPanel(), BorderLayout.EAST);
@@ -686,9 +679,19 @@ public class FunctionEditorDialog extends DialogComponentProvider implements Mod
 		private FocusListener focusListener = new FocusAdapter() {
 			@Override
 			public void focusLost(FocusEvent e) {
-				e.getComponent().removeFocusListener(this);
+				Component component = e.getComponent();
-				if (cellEditor != null) {
+				Component opposite = e.getOppositeComponent();
-					cellEditor.stopCellEditing();
+				if (!SwingUtilities.isDescendingFrom(opposite, component)) {
+					component.removeFocusListener(this);
+					if (cellEditor != null) {
+						cellEditor.stopCellEditing();
+					}
+				}
+				else {
+					// One of the editor's internal components has gotten focus.  Listen to that as
+					// well to know when to stop the edit.
+					opposite.removeFocusListener(this);
+					opposite.addFocusListener(this);
 				}
 			}
 		};
@@ -697,6 +700,21 @@ public class FunctionEditorDialog extends DialogComponentProvider implements Mod
 			super(model);
 		}
 
+		@Override
+		public void setUI(TableUI ui) {
+			super.setUI(ui);
+
+			getSelectionModel().addListSelectionListener(selectionListener);
+			// set the preferred viewport height smaller that the button panel, otherwise it is huge!
+			setPreferredScrollableViewportSize(new Dimension(600, 100));
+			setDefaultEditor(DataType.class,
+				new ParameterDataTypeCellEditor(FunctionEditorDialog.this, service));
+			setDefaultRenderer(DataType.class, new ParameterDataTypeCellRenderer());
+			setDefaultEditor(VariableStorage.class, new StorageTableCellEditor(model));
+			setDefaultRenderer(VariableStorage.class, new VariableStorageCellRenderer());
+			setDefaultRenderer(String.class, new VariableStringCellRenderer());
+		}
+
 		@Override
 		public Component prepareEditor(TableCellEditor editor, int row, int column) {
 			Component component = super.prepareEditor(editor, row, column);

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/StorageAddressEditorDialog.java

@@ -29,6 +29,8 @@ import docking.widgets.label.GDLabel;
 import docking.widgets.label.GLabel;
 import docking.widgets.table.GTable;
 import ghidra.app.services.DataTypeManagerService;
+import ghidra.app.util.datatype.DataTypeSelectionEditor;
+import ghidra.app.util.datatype.NavigationDirection;
 import ghidra.program.model.address.Address;
 import ghidra.program.model.data.*;
 import ghidra.program.model.lang.Register;
@@ -129,8 +131,7 @@ public class StorageAddressEditorDialog extends DialogComponentProvider
 	private void setDataType(DataType dt) {
 		currentDataType = dt;
 		size = dt.getLength();
-		boolean unconstrained =
+		boolean unconstrained = (dt instanceof AbstractFloatDataType) || Undefined.isUndefined(dt);
-			(dt instanceof AbstractFloatDataType) || Undefined.isUndefined(dt);
 		model.setRequiredSize(size, unconstrained);
 		if (sizeLabel != null) {
 			sizeLabel.setText(Integer.toString(size));
@@ -138,6 +139,33 @@ public class StorageAddressEditorDialog extends DialogComponentProvider
 		}
 	}
 
+	private void maybeHandleTabNavigation() {
+		DataTypeSelectionEditor internalEditor = dataTypeEditor.getEditor();
+		NavigationDirection navigationDirection = internalEditor.getNavigationDirection();
+		if (navigationDirection == NavigationDirection.BACKWARD) {
+			// Not all buttons are always enabled.  Walk backwards until we find one
+			if (downButton.isEnabled()) {
+				downButton.requestFocusInWindow();
+			}
+			else if (upButton.isEnabled()) {
+				upButton.requestFocusInWindow();
+			}
+			else if (removeButton.isEnabled()) {
+				removeButton.requestFocusInWindow();
+			}
+			else if (addButton.isEnabled()) {
+				addButton.requestFocusInWindow();
+			}
+			else {
+				varnodeTable.requestFocusInWindow();
+			}
+		}
+		else if (navigationDirection == NavigationDirection.FORWARD) {
+			varnodeTable.requestFocusInWindow();
+		}
+		// navigationDirection == null implies that no navigation event happened
+	}
+
 	private Component buildInfoPanel(DataTypeManagerService service) {
 		JPanel panel = new JPanel(new PairLayout(10, 4));
 		panel.setBorder(BorderFactory.createEmptyBorder(20, 20, 20, 20));
@@ -152,11 +180,12 @@ public class StorageAddressEditorDialog extends DialogComponentProvider
 			public void editingStopped(ChangeEvent e) {
 				DataType dt = (DataType) dataTypeEditor.getCellEditorValue();
 				setDataType(dt);
+				maybeHandleTabNavigation();
 			}
 
 			@Override
 			public void editingCanceled(ChangeEvent e) {
-				// ignore
+				maybeHandleTabNavigation();
 			}
 		});