Merge remote-tracking branch

'origin/GP-3647-dragonmacher-structure-editor-fixes' into patch
(Closes #5566)

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

@@ -1254,13 +1254,20 @@ public abstract class CompEditorModel extends CompositeEditorModel {
 	}
 
 	@Override
-	public void setComponentName(int rowIndex, String name)
+	public boolean setComponentName(int rowIndex, String name)
-			throws InvalidInputException, InvalidNameException, DuplicateNameException {
+			throws InvalidNameException {
+
+		String oldName = getComponent(rowIndex).getFieldName();
+		if (Objects.equals(oldName, name)) {
+			return false;
+		}
+
 		if (nameExistsElsewhere(name, rowIndex)) {
 			throw new InvalidNameException("Name \"" + name + "\" already exists.");
 		}
 		try {
 			getComponent(rowIndex).setFieldName(name); // setFieldName handles trimming
+			return true;
 		}
 		catch (DuplicateNameException exc) {
 			throw new InvalidNameException(exc.getMessage());
@@ -1268,13 +1275,22 @@ public abstract class CompEditorModel extends CompositeEditorModel {
 	}
 
 	@Override
-	public void setComponentComment(int rowIndex, String comment) throws InvalidInputException {
+	public boolean setComponentComment(int rowIndex, String comment) {
-		if (comment.equals("")) {
+
-			comment = null;
+		String oldComment = getComponent(rowIndex).getComment();
+		String newComment = comment;
+		if (newComment.equals("")) {
+			newComment = null;
 		}
-		getComponent(rowIndex).setComment(comment);
+
+		if (Objects.equals(oldComment, newComment)) {
+			return false;
+		}
+
+		getComponent(rowIndex).setComment(newComment);
 		fireTableCellUpdated(rowIndex, getCommentColumn());
 		componentDataChanged();
+		return true;
 	}
 
 	/**

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

@@ -271,18 +271,15 @@ public abstract class CompositeEditorModel extends CompositeViewerModel implemen
 		try {
 			applyingFieldEdit = true;
 			if (columnIndex == getDataTypeColumn()) {
-				setComponentDataType(rowIndex, value);
+				return setComponentDataType(rowIndex, value);
 			}
 			else if (columnIndex == getNameColumn()) {
-				setComponentName(rowIndex, ((String) value).trim());
+				return setComponentName(rowIndex, ((String) value).trim());
 			}
 			else if (columnIndex == getCommentColumn()) {
-				setComponentComment(rowIndex, (String) value);
+				return setComponentComment(rowIndex, (String) value);
 			}
-			else {
+			return false;
-				return false;
-			}
-			return true;
 		}
 		catch (UsrException e) {
 			setStatus(e.getMessage());
@@ -368,7 +365,7 @@ public abstract class CompositeEditorModel extends CompositeViewerModel implemen
 	}
 
 	@Override
-	public void setComponentDataType(int rowIndex, Object dataTypeObject) throws UsrException {
+	public boolean setComponentDataType(int rowIndex, Object dataTypeObject) throws UsrException {
 		DataType previousDt = null;
 		int previousLength = 0;
 		String dtName = "";
@@ -392,7 +389,7 @@ public abstract class CompositeEditorModel extends CompositeViewerModel implemen
 		else if (dataTypeObject instanceof String) {
 			String dtString = (String) dataTypeObject;
 			if (dtString.equals(dtName)) {
-				return;
+				return false;
 			}
 			DataTypeManager originalDTM = getOriginalDataTypeManager();
 			newDt = DataTypeHelper.parseDataType(rowIndex, dtString, this, originalDTM,
@@ -400,7 +397,7 @@ public abstract class CompositeEditorModel extends CompositeViewerModel implemen
 			newLength = newDt.getLength();
 		}
 		if (newDt == null) {
-			return; // Was nothing and is nothing.
+			return false; // Was nothing and is nothing.
 		}
 
 		if (DataTypeComponent.usesZeroLengthComponent(newDt)) {
@@ -417,7 +414,7 @@ public abstract class CompositeEditorModel extends CompositeViewerModel implemen
 			DataTypeInstance sizedDataType = DataTypeHelper.getSizedDataType(provider, newDt,
 				suggestedLength, getMaxReplaceLength(rowIndex));
 			if (sizedDataType == null) {
-				return;
+				return false;
 			}
 			newDt = resolveDataType(sizedDataType.getDataType(), viewDTM,
 				DataTypeConflictHandler.DEFAULT_HANDLER);
@@ -427,7 +424,7 @@ public abstract class CompositeEditorModel extends CompositeViewerModel implemen
 			}
 		}
 		if ((previousDt != null) && newDt.isEquivalent(previousDt) && newLength == previousLength) {
-			return;
+			return false;
 		}
 
 		int maxLength = getMaxReplaceLength(rowIndex);
@@ -437,6 +434,7 @@ public abstract class CompositeEditorModel extends CompositeViewerModel implemen
 		}
 		setComponentDataTypeInstance(rowIndex, newDt, newLength);
 		notifyCompositeChanged();
+		return true;
 	}
 
 	/**
@@ -703,15 +701,15 @@ public abstract class CompositeEditorModel extends CompositeViewerModel implemen
 //==================================================================================================
 
 	@Override
-	public boolean beginEditingField(int rowIndex, int columnIndex) {
+	public boolean beginEditingField(int modelRow, int modelColumn) {
 		if (isEditingField()) {
 			return false;
 		}
 		try {
 			stillBeginningEdit = true; // We want to know we are still beginning an edit when we fix the selection.
 			editingField = true;
-			setLocation(rowIndex, columnIndex);
+			setLocation(modelRow, modelColumn);
-			setSelection(new int[] { rowIndex });
+			setSelection(new int[] { modelRow });
 			notifyEditingChanged();
 		}
 		finally {
@@ -735,17 +733,6 @@ public abstract class CompositeEditorModel extends CompositeViewerModel implemen
 		return !settingValueAt && editingField;
 	}
 
-	@Override
-	public int getFirstEditableColumn(int rowIndex) {
-		int numFields = this.getColumnCount();
-		for (int i = 0; i < numFields; i++) {
-			if (this.isCellEditable(rowIndex, i)) {
-				return i;
-			}
-		}
-		return -1;
-	}
-
 	private void notifyEditingChanged() {
 		for (CompositeEditorModelListener listener : listeners) {
 			listener.compositeEditStateChanged(
@@ -946,7 +933,7 @@ public abstract class CompositeEditorModel extends CompositeViewerModel implemen
 	}
 
 	@Override
-	public boolean isEditFieldAllowed(int rowIndex, int columnIndex) {
+	public boolean isEditFieldAllowed() {
 		return !isEditingField();
 	}
 

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

@@ -150,17 +150,17 @@ public abstract class CompositeEditorPanel extends JPanel
 		table.setDefaultRenderer(DataTypeInstance.class, dtiCellRenderer);
 	}
 
-	private boolean launchBitFieldEditor(int modelColumn, int editingRow) {
+	private boolean launchBitFieldEditor(int modelRow, int modelColumn) {
 		if (model.viewComposite instanceof Structure &&
 			!model.viewComposite.isPackingEnabled() &&
-			model.getDataTypeColumn() == modelColumn && editingRow < model.getNumComponents()) {
+			model.getDataTypeColumn() == modelColumn && modelRow < model.getNumComponents()) {
 			// check if we are attempting to edit a bitfield
-			DataTypeComponent dtComponent = model.getComponent(editingRow);
+			DataTypeComponent dtComponent = model.getComponent(modelRow);
 			if (dtComponent.isBitFieldComponent()) {
 				table.getCellEditor().cancelCellEditing();
 
 				BitFieldEditorDialog dlg = new BitFieldEditorDialog(model.viewComposite,
-					provider.dtmService, editingRow, model.showHexNumbers, ordinal -> {
+					provider.dtmService, modelRow, model.showHexNumbers, ordinal -> {
 						model.notifyCompositeChanged();
 					});
 				Component c = provider.getComponent();
@@ -187,9 +187,11 @@ public abstract class CompositeEditorPanel extends JPanel
 						return;
 					}
 
-					int modelColumn = table.convertColumnIndexToModel(table.getEditingColumn());
+					int modelRow = table.convertRowIndexToModel(editingRow);
-					if (!launchBitFieldEditor(modelColumn, editingRow)) {
+					int editingColumn = table.getEditingColumn();
-						model.beginEditingField(editingRow, modelColumn);
+					int modelColumn = table.convertColumnIndexToModel(editingColumn);
+					if (!launchBitFieldEditor(modelRow, modelColumn)) {
+						model.beginEditingField(modelRow, modelColumn);
 					}
 				});
 			}
@@ -287,9 +289,6 @@ public abstract class CompositeEditorPanel extends JPanel
 					editBelowField();
 					break;
 			}
-			if (table.isEditing()) {
-				table.getEditorComponent().requestFocus();
-			}
 		}
 		finally {
 			editorAdjusting = false;
@@ -313,6 +312,7 @@ public abstract class CompositeEditorPanel extends JPanel
 		// Handle the editing for this field.
 		int viewColumn = table.convertColumnIndexToView(modelColumn);
 		scrollToCell(row, viewColumn);
+		table.setColumnSelectionInterval(viewColumn, viewColumn);
 		startCellEditing(row, viewColumn);
 		return table.isEditing();
 	}
@@ -331,20 +331,20 @@ public abstract class CompositeEditorPanel extends JPanel
 		int index = row;
 		int fieldNum = table.convertColumnIndexToView(modelColumn);
 
-		int numFields = model.getColumnCount();
+		int numFields = table.getColumnCount();
 		int numComps = model.getRowCount();
 		do {
 			// Determine the new location for the cursor.
 			if (index < numComps) { // on component row
 				if (++fieldNum < (numFields)) { // not on last field
-					if (model.isCellEditable(index, table.convertColumnIndexToModel(fieldNum))) {
+					if (table.isCellEditable(index, fieldNum)) {
 						foundEditable = true;
 					}
 				}
 				else if ((++index < numComps) // on last field for other than last component
 					|| (index == numComps)) { // Last field and row but unlocked
 					fieldNum = 0; // Set it to first field.
-					if (model.isCellEditable(index, table.convertColumnIndexToModel(fieldNum))) {
+					if (table.isCellEditable(index, fieldNum)) {
 						foundEditable = true;
 					}
 				}
@@ -365,6 +365,7 @@ public abstract class CompositeEditorPanel extends JPanel
 			model.setRow(row);
 			model.setColumn(modelColumn);
 		}
+
 		return foundEditable;
 	}
 
@@ -464,6 +465,7 @@ public abstract class CompositeEditorPanel extends JPanel
 		if (locateNextEditField(currentRow)) {
 			return beginEditField(model.getRow(), model.getColumn());
 		}
+
 		return false;
 	}
 
@@ -559,7 +561,7 @@ public abstract class CompositeEditorPanel extends JPanel
 	}
 
 	private void createTable() {
-		table = new GTable(model);
+		table = new CompositeEditorTable(model);
 
 		TableColumnModel columnModel = table.getColumnModel();
 		if (columnModel instanceof GTableColumnModel) {
@@ -570,16 +572,9 @@ public abstract class CompositeEditorPanel extends JPanel
 			}
 		}
 
-		table.putClientProperty("JTable.autoStartsEdit", Boolean.FALSE);
+		table.setAutoEditEnabled(false); // do not edit when typing
-		table.addMouseListener(new CompositeTableMouseListener());
 
-		CompositeEditorTableAction action = provider.actionMgr.getNamedAction(
+		table.addMouseListener(new CompositeTableMouseListener());
-			CompositeEditorTableAction.EDIT_ACTION_PREFIX + EditFieldAction.ACTION_NAME);
-		Action swingAction = KeyBindingUtils.adaptDockingActionToNonContextAction(action);
-		InputMap map = table.getInputMap();
-		map.put(action.getKeyBinding(), "StartEditing");
-		ActionMap amap = table.getActionMap();
-		amap.put("StartEditing", swingAction);
 
 		table.getSelectionModel().addListSelectionListener(e -> {
 			if (e.getValueIsAdjusting()) {
@@ -599,7 +594,9 @@ public abstract class CompositeEditorPanel extends JPanel
 			TableColumnModel cm = table.getColumnModel();
 			int[] selected = cm.getSelectedColumns();
 			if (selected.length == 1) {
-				model.setColumn(selected[0]);
+				int viewIndex = selected[0];
+				int modelIndex = table.convertColumnIndexToModel(viewIndex);
+				model.setColumn(modelIndex);
 			}
 			else {
 				model.setColumn(-1);
@@ -892,7 +889,7 @@ public abstract class CompositeEditorPanel extends JPanel
 	}
 
 	/**
-	 * Add the object to the droppable component. The DragSrcAdapter calls this method from its 
+	 * Add the object to the droppable component. The DragSrcAdapter calls this method from its
 	 * drop() method.
 	 *
 	 * @param obj Transferable object that is to be dropped.
@@ -920,7 +917,7 @@ public abstract class CompositeEditorPanel extends JPanel
 	}
 
 	/**
-	 * Add the object to the droppable component. The DragSrcAdapter calls this method from its 
+	 * Add the object to the droppable component. The DragSrcAdapter calls this method from its
 	 * drop() method.
 	 *
 	 * @param p the point of insert
@@ -939,7 +936,7 @@ public abstract class CompositeEditorPanel extends JPanel
 	}
 
 	/**
-	 * Add the object to the droppable component. The DragSrcAdapter calls this method from its 
+	 * Add the object to the droppable component. The DragSrcAdapter calls this method from its
 	 * drop() method.
 	 * 
 	 * @param p the point of insert
@@ -1158,9 +1155,9 @@ public abstract class CompositeEditorPanel extends JPanel
 			BigInteger endIndex = range.getEnd().getIndex();
 			lsm.addSelectionInterval(startIndex.intValue(), endIndex.intValue() - 1);
 		}
-		int column = model.getColumn();
+		int modelColumn = model.getColumn();
-		clsm.setAnchorSelectionIndex(column);
+		int viewColumn = table.convertColumnIndexToView(modelColumn);
-		clsm.setLeadSelectionIndex(column);
+		clsm.setSelectionInterval(viewColumn, viewColumn);
 	}
 
 	private class ComponentStringCellEditor extends ComponentCellEditor {
@@ -1344,18 +1341,15 @@ public abstract class CompositeEditorPanel extends JPanel
 		@Override
 		public boolean stopCellEditing() {
 
-			ListSelectionModel columnSelectionModel = table.getColumnModel().getSelectionModel();
-			columnSelectionModel.setValueIsAdjusting(true);
-
 			int editingColumn = table.getEditingColumn();
 
 			model.setStatus("");
-
 			if (!isEmptyEditorCell() && !validateUserChoice()) {
 				return false;
 			}
 
-			int editingRow = model.getRow();
+			ListSelectionModel columnSelectionModel = table.getColumnModel().getSelectionModel();
+			columnSelectionModel.setValueIsAdjusting(true);
 
 			DataType dataType = (DataType) editor.getCellEditorValue();
 			if (dataType != null) {
@@ -1371,10 +1365,10 @@ public abstract class CompositeEditorPanel extends JPanel
 				fireEditingCanceled();
 			}
 
-			columnSelectionModel.setAnchorSelectionIndex(editingColumn);
+			columnSelectionModel.setSelectionInterval(editingColumn, editingColumn);
-			columnSelectionModel.setLeadSelectionIndex(editingColumn);
 			columnSelectionModel.setValueIsAdjusting(false);
 
+			int editingRow = model.getRow();
 			NavigationDirection navigationDirection = editor.getNavigationDirection();
 			if (navigationDirection == NavigationDirection.BACKWARD) {
 				editPreviousField(editingRow);
@@ -1450,39 +1444,35 @@ public abstract class CompositeEditorPanel extends JPanel
 			int column = table.columnAtPoint(point);
 			int modelColumn = table.convertColumnIndexToModel(column);
 			int clickCount = e.getClickCount();
-			if (!table.isEditing() && (e.getID() == MouseEvent.MOUSE_PRESSED)) {
+			if (!table.isEditing() && e.getID() == MouseEvent.MOUSE_PRESSED) {
-				model.clearStatus(); // Only want to clear status when starting new actions (pressed).
+				model.clearStatus(); // Only clear status when starting new actions (pressed).
 			}
+
 			if (isPopup) {
 				if (!table.isRowSelected(row)) {
 					table.setRowSelectionInterval(row, row);
 				}
 				return;
 			}
-			if ((clickCount < 2) || (e.getButton() != MouseEvent.BUTTON1)) {
+
+			if (clickCount < 2 || e.getButton() != MouseEvent.BUTTON1) {
 				return;
 			}
-			String status = null;
+
 			if (model.isCellEditable(row, modelColumn)) {
 				return;
 			}
 
-			status = model.getColumnName(modelColumn) + " field is not editable";
+			String columnName = model.getColumnName(modelColumn);
-			if ((row >= 0) && (row < model.getNumComponents()) &&
+			String status = columnName + " field is not editable";
-				((modelColumn == model.getNameColumn()) ||
+
-					(modelColumn == model.getCommentColumn()))) {
+			boolean isValidRow = row >= 0 && row < model.getNumComponents();
-				if (!model.isCellEditable(row, modelColumn)) {
+			boolean isStringColumn = modelColumn == model.getNameColumn() ||
-					DataType dt = model.getComponent(row).getDataType();
+				modelColumn == model.getCommentColumn();
-					if (dt == DataType.DEFAULT) {
+			if (isValidRow && isStringColumn) {
-						if (modelColumn == model.getNameColumn()) {
+				DataType dt = model.getComponent(row).getDataType();
-							status = model.getColumnName(modelColumn) +
+				if (dt == DataType.DEFAULT) {
-								" field is not editable for Undefined byte.";
+					status = columnName + " field is not editable for Undefined byte.";
-						}
-						else if (modelColumn == model.getCommentColumn()) {
-							status = model.getColumnName(modelColumn) +
-								" field is not editable for Undefined byte.";
-						}
-					}
 				}
 			}
 
@@ -1491,4 +1481,26 @@ public abstract class CompositeEditorPanel extends JPanel
 			e.consume();
 		}
 	}
+
+	private class CompositeEditorTable extends GTable {
+
+		public CompositeEditorTable(TableModel model) {
+			super(model);
+		}
+
+		@Override
+		protected void installEditKeyBinding() {
+			// We use a tool action instead of the default action.  We must signal to the table to
+			// not use the default action to prevent the table from getting the action.
+
+			// This code will insert a placeholder of 'none' in the table for this keystroke, which
+			// is the default edit keystroke in Swing.  The actual binding for this keystroke is in
+			// a parent input map of the table.  By placing this keystroke in the table's input map,
+			// we prevent the key processing code from traversing into the parent input map's
+			// bindings.
+			KeyStroke keyStroke = KeyStroke.getKeyStroke("pressed F2");
+			KeyBindingUtils.clearKeyBinding(this, keyStroke);
+		}
+	}
+
 }

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

@@ -53,7 +53,7 @@ public abstract class CompositeEditorProvider extends ComponentProviderAdapter
 	protected CompositeEditorActionManager actionMgr;
 
 	/**
-	 * Construct a new stack editor provider. 
+	 * Construct a new stack editor provider.
 	 * @param plugin owner of this provider
 	 */
 	protected CompositeEditorProvider(Plugin plugin) {
@@ -94,6 +94,20 @@ public abstract class CompositeEditorProvider extends ComponentProviderAdapter
 		return editorPanel.getTable();
 	}
 
+	public int getFirstEditableColumn(int row) {
+		if (editorPanel == null) {
+			return -1;
+		}
+		JTable table = editorPanel.getTable();
+		int n = table.getColumnCount();
+		for (int col = 0; col < n; col++) {
+			if (table.isCellEditable(row, col)) {
+				return col;
+			}
+		}
+		return -1;
+	}
+
 	protected void initializeActions() {
 		actionMgr = new CompositeEditorActionManager(this);
 		actionMgr.setEditorActions(createActions());
@@ -281,7 +295,7 @@ public abstract class CompositeEditorProvider extends ComponentProviderAdapter
 	 * Prompts the user if the editor has unsaved changes. Saves the changes if
 	 * the user indicates to do so.
 	 * @param allowCancel true if allowed to cancel
-	 * @return 0 if the user canceled; 1 if the user saved changes; 
+	 * @return 0 if the user canceled; 1 if the user saved changes;
 	 * 2 if the user did not to save changes; 3 if there was an error when
 	 * the changes were applied.
 	 */

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

@@ -34,9 +34,9 @@ import utility.function.Callback;
 abstract class CompositeViewerModel extends AbstractTableModel
 		implements DataTypeManagerChangeListener {
 
-	/** 
+	/**
-	 * Flag indicating that the model is updating the selection and should ignore any attempts to 
+	 * Flag indicating that the model is updating the selection and should ignore any attempts to
-	 * set the selection until it is no longer updating. 
+	 * set the selection until it is no longer updating.
 	 */
 	protected boolean updatingSelection = false;
 
@@ -170,7 +170,7 @@ abstract class CompositeViewerModel extends AbstractTableModel
 	 * Unloads the currently loaded composite data type.
 	 * This should be called when the viewer is removed from view and
 	 * and category/dataType changes no longer need to be listened for.
-	 * It can also be called to unload the current composite before loading 
+	 * It can also be called to unload the current composite before loading
 	 * a new composite data type.
 	 */
 	void unload() {
@@ -238,6 +238,7 @@ abstract class CompositeViewerModel extends AbstractTableModel
 			// clear our notion of the last selected column
 			return;
 		}
+
 		this.column = column;
 	}
 
@@ -299,7 +300,7 @@ abstract class CompositeViewerModel extends AbstractTableModel
 
 	/**
 	 * Return the path of the data category for the structure being viewed
-	 * @return the path 
+	 * @return the path
 	 */
 	public final CategoryPath getOriginalCategoryPath() {
 		if (originalDataTypePath != null) {
@@ -322,7 +323,7 @@ abstract class CompositeViewerModel extends AbstractTableModel
 
 	/**
 	 * Return the size of the structure being viewed in bytes
-	 * @return this size 
+	 * @return this size
 	 */
 	public int getLength() {
 		if (viewComposite != null && !viewComposite.isZeroLength()) {
@@ -332,7 +333,7 @@ abstract class CompositeViewerModel extends AbstractTableModel
 	}
 
 	/**
-	 * Return the size of the structure being viewed in bytes as a hex or decimal string depending 
+	 * Return the size of the structure being viewed in bytes as a hex or decimal string depending
 	 * on the model's current display setting for numbers
 	 * @return the length
 	 */
@@ -360,7 +361,7 @@ abstract class CompositeViewerModel extends AbstractTableModel
 	/**
 	 * Return a header name for the indicated column.
 	 *
-	 * @param columnIndex the index number indicating the component field (column) to get the 
+	 * @param columnIndex the index number indicating the component field (column) to get the
 	 * header for.
 	 */
 	@Override
@@ -371,7 +372,7 @@ abstract class CompositeViewerModel extends AbstractTableModel
 	/**
 	 * Return a header name for the indicated field (column)
 	 *
-	 * @param columnIndex the index number indicating the component field (column) to get the 
+	 * @param columnIndex the index number indicating the component field (column) to get the
 	 * header for
 	 * @return the name
 	 */
@@ -484,8 +485,8 @@ abstract class CompositeViewerModel extends AbstractTableModel
 	}
 
 	/**
-	 * Returns the number of component rows in the viewer. There may be a blank row at the end for 
+	 * Returns the number of component rows in the viewer. There may be a blank row at the end for
-	 * selecting. Therefore this number can be different than the actual number of components 
+	 * selecting. Therefore this number can be different than the actual number of components
 	 * currently in the structure being viewed.
 	 *
 	 * @return the number of rows in the model
@@ -504,8 +505,8 @@ abstract class CompositeViewerModel extends AbstractTableModel
 	}
 
 	/**
-	 * Return the nth component for the structure being viewed. Since the number of rows can exceed 
+	 * Return the nth component for the structure being viewed. Since the number of rows can exceed
-	 * the number of components defined within the composite ({@link Composite#getNumComponents()}) 
+	 * the number of components defined within the composite ({@link Composite#getNumComponents()})
 	 * this method will return null for a blank row.
 	 * 
 	 * @param rowIndex the index of the component to return. First component is index of 0
@@ -519,7 +520,7 @@ abstract class CompositeViewerModel extends AbstractTableModel
 	}
 
 	/**
-	 * Returns the number of columns (display fields) for each component in this structure or 
+	 * Returns the number of columns (display fields) for each component in this structure or
 	 * union.
 	 *
 	 * @return the number of display fields for each component
@@ -635,7 +636,7 @@ abstract class CompositeViewerModel extends AbstractTableModel
 	}
 
 	/**
-	 * Fixes up the original name and category because a program restoration may have changed the 
+	 * Fixes up the original name and category because a program restoration may have changed the
 	 * original composite.
 	 * @param composite the restored copy of our original composite
 	 */
@@ -671,7 +672,7 @@ abstract class CompositeViewerModel extends AbstractTableModel
 	}
 
 	/**
-	 * Called whenever the composite's non-component information changes.  For example, the name, 
+	 * Called whenever the composite's non-component information changes.  For example, the name,
 	 * or description change.
 	 */
 	protected void compositeInfoChanged() {
@@ -686,7 +687,7 @@ abstract class CompositeViewerModel extends AbstractTableModel
 	}
 
 	/**
-	 * Determines the full path name for the composite data type based on the original composite 
+	 * Determines the full path name for the composite data type based on the original composite
 	 * and original category.
 	 * @return the full path name
 	 */
@@ -960,10 +961,10 @@ abstract class CompositeViewerModel extends AbstractTableModel
 
 //=================================================================================================
 // Helper methods for CategoryChangeListener methods.
-//=================================================================================================	
+//=================================================================================================
 
 	/**
-	 * Determines whether the indicated composite data type has any sub-components that are within 
+	 * Determines whether the indicated composite data type has any sub-components that are within
 	 * the indicated category or one of its sub-categories.
 	 * @param parentDt the composite data type
 	 * @param catPath the category's path
@@ -971,7 +972,7 @@ abstract class CompositeViewerModel extends AbstractTableModel
 	 */
 	boolean hasSubDtInCategory(Composite parentDt, String catPath) {
 		DataTypeComponent components[] = parentDt.getDefinedComponents();
-		// FUTURE Add a structure to keep track of which composites were searched so they aren't 
+		// FUTURE Add a structure to keep track of which composites were searched so they aren't
 		// searched multiple times.
 		for (DataTypeComponent component : components) {
 			DataType subDt = component.getDataType();
@@ -989,7 +990,7 @@ abstract class CompositeViewerModel extends AbstractTableModel
 	}
 
 	/**
-	 * Determines whether the indicated composite data type has any sub-components that are the 
+	 * Determines whether the indicated composite data type has any sub-components that are the
 	 * indicated data type.
 	 * @param parentDt the composite data type
 	 * @param dtPath the data type to be detected
@@ -1040,7 +1041,7 @@ abstract class CompositeViewerModel extends AbstractTableModel
 
 	/**
 	 * Returns the number of rows currently selected.
-	 *  
+	 * 
 	 * <p>Note: In unlocked mode this can include the additional blank line.
 	 * 
 	 * @return the selected row count
@@ -1152,7 +1153,7 @@ abstract class CompositeViewerModel extends AbstractTableModel
 	}
 
 	/**
-	 * Returns the selection range containing the specified row index if there is one that contains 
+	 * Returns the selection range containing the specified row index if there is one that contains
 	 * it. Otherwise, returns null.
 	 *
 	 * @param rowIndex the row index
@@ -1214,7 +1215,7 @@ abstract class CompositeViewerModel extends AbstractTableModel
 	}
 
 	/**
-	 * Sets the model's current selection to the indicated selection.  If the selection is empty, 
+	 * Sets the model's current selection to the indicated selection.  If the selection is empty,
 	 * it gets adjusted to the empty last line when in unlocked mode.
 	 * @param selection the new selection
 	 */
@@ -1289,7 +1290,7 @@ abstract class CompositeViewerModel extends AbstractTableModel
 	}
 
 	/**
-	 * A notify method to take the listens to notify, along with the method that should be called 
+	 * A notify method to take the listens to notify, along with the method that should be called
 	 * on each listener.
 	 * 
 	 * @param <T> the type of the listener

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

@@ -17,6 +17,7 @@ package ghidra.app.plugin.core.compositeeditor;
 
 import java.awt.event.KeyEvent;
 
+import javax.swing.JTable;
 import javax.swing.KeyStroke;
 
 import docking.ActionContext;
@@ -54,8 +55,10 @@ public class EditFieldAction extends CompositeEditorTableAction {
 			}
 
 			// just go to the first editable cell, since the current one is not editable
-			int firstEditableColumn = model.getFirstEditableColumn(row);
+			int firstEditableColumn = provider.getFirstEditableColumn(row);
-			model.beginEditingField(row, firstEditableColumn);
+			JTable table = provider.getTable();
+			int modelColumn = table.convertColumnIndexToModel(firstEditableColumn);
+			model.beginEditingField(row, modelColumn);
 		}
 		requestTableFocus();
 	}
@@ -64,9 +67,7 @@ public class EditFieldAction extends CompositeEditorTableAction {
 	public void adjustEnablement() {
 		boolean shouldEnableEdit = false;
 		if (model.isSingleRowSelection()) {
-			int[] rows = model.getSelectedRows();
+			shouldEnableEdit = model.isEditFieldAllowed();
-			int firstEditableColumn = model.getFirstEditableColumn(rows[0]);
-			shouldEnableEdit = model.isEditFieldAllowed(rows[0], firstEditableColumn);
 		}
 		setEnabled(shouldEnableEdit);
 	}

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

@@ -18,16 +18,17 @@ package ghidra.app.plugin.core.compositeeditor;
 import ghidra.app.util.datatype.EmptyCompositeException;
 import ghidra.program.model.data.*;
 import ghidra.util.InvalidNameException;
-import ghidra.util.exception.*;
+import ghidra.util.exception.DuplicateNameException;
+import ghidra.util.exception.UsrException;
 import ghidra.util.task.TaskMonitor;
 
 public interface EditorModel {
 
-	// TODO: This model interface serves no real purpose and could be collapsed into the 
+	// TODO: This model interface serves no real purpose and could be collapsed into the
 	// abstract class CompositeEditorModel implementation.
 
 	/**
-	 * Called when the model is no longer needed. 
+	 * Called when the model is no longer needed.
 	 * This is where all cleanup code for the model should be placed.
 	 */
 	public void dispose();
@@ -119,22 +120,7 @@ public interface EditorModel {
 	 */
 	public boolean isEditComponentAllowed();
 
-	/**
+	public boolean isEditFieldAllowed();
-	 * 
-	 * @param rowIndex
-	 * @param column
-	 * @return
-	 */
-	public boolean isEditFieldAllowed(int rowIndex, int column);
-
-//	/**
-//	 * Returns whether or not insertion of the specified data type is allowed
-//	 * at the specified index.
-//	 *
-//	 * @param index index of the component in the union.
-//	 * @param datatype the data type to be inserted.
-//	 */
-//	public boolean isInsertAllowed(DataType datatype);
 
 	/**
 	 * Returns whether or not insertion of the specified data type is allowed
@@ -155,29 +141,18 @@ public interface EditorModel {
 	 */
 	public boolean isMoveUpAllowed();
 
-//	/**
-//	 * 
-//	 * @param dataType
-//	 * @return
-//	 */
-//	public boolean isReplaceAllowed(DataType dataType);
-
-	/**
-	 * 
-	 * @param rowIndex row index of the component in the composite data type.
-	 * @param dataType
-	 * @return
-	 */
 	public boolean isReplaceAllowed(int rowIndex, DataType dataType);
 
 	/**
 	 * Returns whether the selected component can be unpackaged.
+	 * @return whether the selected component can be unpackaged.
 	 */
 	public boolean isUnpackageAllowed();
 
 	/**
 	 * Returns whether or not the editor has changes that haven't been applied.
 	 * Changes can also mean a new data type that hasn't yet been saved.
+	 * @return if there are changes
 	 */
 	public boolean hasChanges();
 
@@ -187,6 +162,7 @@ public interface EditorModel {
 	 * @param name the new name.
 	 * 
 	 * @throws DuplicateNameException if the name already exists.
+	 * @throws InvalidNameException if the name is invalid
 	 */
 	public void setName(String name) throws DuplicateNameException, InvalidNameException;
 
@@ -199,14 +175,16 @@ public interface EditorModel {
 
 	/**
 	 * Sets the data type for the component at the indicated rowIndex.
-	 * @param rowIndex the row index of the component 
+	 * @param rowIndex the row index of the component
 	 * @param dataTypeObject a String or a DataType
+	 * @return true if changed
+	 * @throws UsrException if the type cannot be used
 	 */
-	public void setComponentDataType(int rowIndex, Object dataTypeObject) throws UsrException;
+	public boolean setComponentDataType(int rowIndex, Object dataTypeObject) throws UsrException;
 
 	/**
 	 * Sets the data type for the component at the indicated row index.
-	 * @param rowIndex the row index of the component 
+	 * @param rowIndex the row index of the component
 	 * @param dt component datatype
 	 * @param length component length
 	 * @throws UsrException if invalid datatype or length specified
@@ -216,18 +194,20 @@ public interface EditorModel {
 
 	/**
 	 * Sets the data type for the component at the indicated index.
-	 * @param rowIndex the row index of the component 
+	 * @param rowIndex the row index of the component
-	 * @param name
+	 * @param name the name
+	 * @return true if a change was made
+	 * @throws InvalidNameException if the name is invalid
 	 */
-	public void setComponentName(int rowIndex, String name)
+	public boolean setComponentName(int rowIndex, String name) throws InvalidNameException;
-			throws InvalidInputException, InvalidNameException, DuplicateNameException;
 
 	/**
 	 * Sets the data type for the component at the indicated index.
-	 * @param rowIndex the row index of the component 
+	 * @param rowIndex the row index of the component
-	 * @param comment
+	 * @param comment the comment
+	 * @return true if a change was made
 	 */
-	public void setComponentComment(int rowIndex, String comment) throws InvalidInputException;
+	public boolean setComponentComment(int rowIndex, String comment);
 
 	/**
 	 *  Returns whether or not the editor is showing undefined bytes.
@@ -235,62 +215,26 @@ public interface EditorModel {
 	 */
 	public boolean isShowingUndefinedBytes();
 
-	/**
+	public boolean beginEditingField(int modelRow, int modelColumn);
-	 * Gets the column number of the first editable field found for the indicated row.
-	 * 
-	 * @param rowIndex the index number of the row
-	 * @return the index number of the editable column or -1 if no fields are editable.
-	 */
-	public int getFirstEditableColumn(int rowIndex);
 
 	/**
-	 * 
+	 * Change the edit state to indicate no longer editing a field.
-	 * @param rowIndex
+	 * @return the edit state to indicate no longer editing a field.
-	 * @param column
-	 * @return
-	 */
-	public boolean beginEditingField(int rowIndex, int column);
-
-	/**
-	 *  Change the edit state to indicate no longer editing a field.
 	 */
 	public boolean endEditingField();
 
 	/**
-	 *  Returns whether the user is currently editing a field's value.
+	 * Returns whether the user is currently editing a field's value.
+	 * @return whether the user is currently editing a field's value.
 	 */
 	public boolean isEditingField();
 
-	/**
-	 * 
-	 */
 	public void endFieldEditing();
 
-	/**
-	 * 
-	 * @param dataType
-	 * @return
-	 * @throws UsrException
-	 */
 	public DataTypeComponent add(DataType dataType) throws UsrException;
 
-	/**
-	 * 
-	 * @param rowIndex
-	 * @param dataType
-	 * @return
-	 * @throws UsrException
-	 */
 	public DataTypeComponent add(int rowIndex, DataType dataType) throws UsrException;
 
-	/**
-	 * 
-	 * @param rowIndex
-	 * @param dt
-	 * @param dtLength
-	 * @return
-	 * @throws UsrException
-	 */
 	public DataTypeComponent add(int rowIndex, DataType dt, int dtLength) throws UsrException;
 
 	/**
@@ -305,21 +249,10 @@ public interface EditorModel {
 
 	public void clearComponent(int rowIndex);
 
-	/**
-	 * 
-	 * @throws UsrException
-	 */
 	public void clearSelectedComponents() throws UsrException;
 
-	/**
-	 * @param cycleGroup
-	 */
 	public void cycleDataType(CycleGroup cycleGroup);
 
-	/**
-	 * Create array component
-	 * @throws UsrException
-	 */
 	public void createArray() throws UsrException;
 
 	/**
@@ -331,7 +264,7 @@ public interface EditorModel {
 
 	/**
 	 * Creates multiple duplicates of the indicated component.
-	 * The duplicates will be created at the index immediately after the 
+	 * The duplicates will be created at the index immediately after the
 	 * indicated component.
 	 * @param rowIndex the index of the row whose component is to be duplicated.
 	 * @param multiple the number of duplicates to create.
@@ -341,77 +274,45 @@ public interface EditorModel {
 	public void duplicateMultiple(int rowIndex, int multiple, TaskMonitor monitor)
 			throws UsrException;
 
-	/**
-	 * 
-	 * @param dataType
-	 * @return
-	 * @throws UsrException
-	 */
 	public DataTypeComponent insert(DataType dataType) throws UsrException;
 
-	/**
-	 * 
-	 * @param rowIndex
-	 * @param dataType
-	 * @return
-	 * @throws UsrException
-	 */
 	public DataTypeComponent insert(int rowIndex, DataType dataType) throws UsrException;
 
-	/**
-	 * 
-	 * @param rowIndex
-	 * @param dt
-	 * @param dtLength
-	 * @return
-	 * @throws UsrException
-	 */
 	public DataTypeComponent insert(int rowIndex, DataType dt, int dtLength) throws UsrException;
 
 	/**
-	 * Moves a contiguous selection of components up by a single position. 
+	 * Moves a contiguous selection of components up by a single position. The component that was
-	 * The component that was immediately above 
+	 * immediately above (at the index immediately preceding the selection) the selection will be
-	 * (at the index immediately preceeding the selection)
+	 * moved below the selection (to what was the maximum selected component index).
-	 * the selection will be moved below the selection 
-	 * (to what was the maximum selected component index).
 	 * @return true if selected components were moved up.
 	 * @throws UsrException if components can't be moved up.
 	 */
 	public boolean moveUp() throws UsrException;
 
 	/**
-	 * Moves a contiguous selection of components down by a single position. 
+	 * Moves a contiguous selection of components down by a single position. The component that was
-	 * The component that was immediately below 
+	 * immediately below (at the index immediately following the selection) the selection will be
-	 * (at the index immediately following the selection)
+	 * moved above the selection (to what was the minimum selected component index).
-	 * the selection will be moved above the selection 
-	 * (to what was the minimum selected component index).
 	 * @return true if selected components were moved down.
 	 * @throws UsrException if components can't be moved down.
 	 */
 	public boolean moveDown() throws UsrException;
 
-	/**
-	 * 
-	 * @param rowIndex
-	 * @param dt
-	 * @param dtLength
-	 * @return
-	 * @throws UsrException
-	 */
 	public DataTypeComponent replace(int rowIndex, DataType dt, int dtLength) throws UsrException;
 
 	/**
 	 * Gets the maximum number of bytes available for a data type that is added at the indicated
-	 * index. This can vary based on whether or not it is in a selection. 
+	 * index. This can vary based on whether or not it is in a selection.
 	 *
 	 * @param rowIndex index of the row in the editor's composite data type.
+	 * @return the length
 	 */
 	public int getMaxAddLength(int rowIndex);
 
 	/**
-	 * Determine the maximum number of duplicates that can be created for 
+	 * Determine the maximum number of duplicates that can be created for
 	 * the component at the indicated index. The duplicates would follow
-	 * the component. The number allowed depends on how many fit based on 
+	 * the component. The number allowed depends on how many fit based on
 	 * the current lock/unlock state of the editor.
 	 * <br>Note: This method doesn't care whether there is a selection or not.
 	 *
@@ -421,7 +322,7 @@ public interface EditorModel {
 	public int getMaxDuplicates(int rowIndex);
 
 	/**
-	 * Determine the maximum number of array elements that can be created for 
+	 * Determine the maximum number of array elements that can be created for
 	 * the current selection. The array data type is assumed to become the
 	 * data type of the first component in the selection. The current selection
 	 * must be contiguous or 0 is returned.
@@ -431,9 +332,9 @@ public interface EditorModel {
 	public int getMaxElements();
 
 	/**
-	 * Gets the maximum number of bytes available for a new data type that 
+	 * Gets the maximum number of bytes available for a new data type that
 	 * will replace the current data type at the indicated component index.
-	 * If there isn't a component with the indicated index, the max length 
+	 * If there isn't a component with the indicated index, the max length
 	 * will be determined by the lock mode.
 	 *
 	 * @param rowIndex index of the row for the component to replace.
@@ -442,21 +343,21 @@ public interface EditorModel {
 	public int getMaxReplaceLength(int rowIndex);
 
 	/**
-	 * Return the last number of bytes the user entered when prompted for 
+	 * Return the last number of bytes the user entered when prompted for
 	 * a data type size.
 	 * @return the number of bytes
 	 */
 	public int getLastNumBytes();
 
 	/**
-	 * Return the last number of duplicates the user entered when prompted for 
+	 * Return the last number of duplicates the user entered when prompted for
 	 * creating duplicates of a component.
 	 * @return the number of duplicates
 	 */
 	public int getLastNumDuplicates();
 
 	/**
-	 * Return the last number of elements the user entered when prompted for 
+	 * Return the last number of elements the user entered when prompted for
 	 * creating an array.
 	 * @return the number of elements
 	 */

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

@@ -254,7 +254,7 @@ class StructureEditorModel extends CompEditorModel {
 		if ((rowIndex < 0) || (rowIndex >= getRowCount())) {
 			return false;
 		}
-		// There shouldn't be a selection when this is called.
+
 		switch (columnIndex) {
 			case DATATYPE:
 				return true;

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/stackeditor/StackEditorModel.java

@@ -104,6 +104,7 @@ public class StackEditorModel extends CompositeEditorModel {
 		super.load(dataType);
 	}
 
+	@Override
 	protected Composite createViewCompositeFromOriginalComposite(Composite original) {
 		return (Composite) original.copy(original.getDataTypeManager());
 	}
@@ -208,7 +209,7 @@ public class StackEditorModel extends CompositeEditorModel {
 				if (fieldName == null) {
 					fieldName = "";
 				}
-//				if ((fieldName.length() == 0) 
+//				if ((fieldName.length() == 0)
 //				 && (element.getOffset() == ((StackFrameDataType)viewComposite).getReturnAddressOffset())) {
 //					return "<RETURN_ADDRESS>";
 //				}
@@ -632,7 +633,7 @@ public class StackEditorModel extends CompositeEditorModel {
 		boolean existingPointer = (compDt instanceof Pointer);
 		boolean isPointer = (dataType instanceof Pointer) || existingPointer;
 		int newLength = dataType.getLength();
-		// NOTE : Allow the generic pointer, but don't allow -1 length data 
+		// NOTE : Allow the generic pointer, but don't allow -1 length data
 		//        types (i.e. string) except on pointers.
 		if (!isPointer && (newLength <= 0)) {
 			return false;
@@ -797,7 +798,8 @@ public class StackEditorModel extends CompositeEditorModel {
 	}
 
 	@Override
-	public void setComponentDataTypeInstance(int index, DataType dt, int length) throws UsrException {
+	public void setComponentDataTypeInstance(int index, DataType dt, int length)
+			throws UsrException {
 		checkIsAllowableDataType(dt);
 		((StackFrameDataType) viewComposite).setDataType(index, dt, length);
 	}
@@ -811,27 +813,19 @@ public class StackEditorModel extends CompositeEditorModel {
 	}
 
 	@Override
-	public void setComponentName(int rowIndex, String newName)
+	public boolean setComponentName(int rowIndex, String newName)
-			throws InvalidInputException, InvalidNameException, DuplicateNameException {
+			throws InvalidNameException {
 
 		if (newName.trim().length() == 0) {
 			newName = null;
 		}
-//		if (nameExistsElsewhere(newName, currentIndex)) {
-//			throw new InvalidNameException("Name \"" + newName + "\" already exists.");
-//		}
-//		try {
-//			getComponent(currentIndex).setFieldName(newName);
-//		} catch (DuplicateNameException exc) {
-//		throw new InvalidNameException(exc.getMessage());
-//		}
 
 		// prevent user names that are default values, unless the value is the original name
 		String nameInEditor = (String) getValueAt(rowIndex, NAME);
 		StackFrameDataType stackFrameDataType = ((StackFrameDataType) viewComposite);
 		if (stackFrameDataType.isDefaultName(newName) && !isOriginalFieldName(newName, rowIndex)) {
-			if (SystemUtilities.isEqual(nameInEditor, newName)) {
+			if (Objects.equals(nameInEditor, newName)) {
-				return; // same as current name in the table; do nothing
+				return false; // same as current name in the table; do nothing
 			}
 			throw new InvalidNameException("Cannot set a stack variable name to a default value");
 		}
@@ -840,7 +834,9 @@ public class StackEditorModel extends CompositeEditorModel {
 			updateAndCheckChangeState();
 			fireTableCellUpdated(rowIndex, getNameColumn());
 			notifyCompositeChanged();
+			return true;
 		}
+		return false;
 	}
 
 	/** Gets the original field name within the parent data type for a given row in the editor */
@@ -851,12 +847,14 @@ public class StackEditorModel extends CompositeEditorModel {
 	}
 
 	@Override
-	public void setComponentComment(int currentIndex, String comment) throws InvalidInputException {
+	public boolean setComponentComment(int currentIndex, String comment) {
 		if (((StackFrameDataType) viewComposite).setComment(currentIndex, comment)) {
 			updateAndCheckChangeState();
 			fireTableRowsUpdated(currentIndex, currentIndex);
 			componentDataChanged();
+			return true;
 		}
+		return false;
 	}
 
 	@Override
@@ -912,7 +910,7 @@ public class StackEditorModel extends CompositeEditorModel {
 	@Override
 	public boolean apply() throws EmptyCompositeException, InvalidDataTypeException {
 
-		// commit changes for any fields under edit 
+		// commit changes for any fields under edit
 		if (isEditingField()) {
 			endFieldEditing();
 		}
@@ -940,7 +938,7 @@ public class StackEditorModel extends CompositeEditorModel {
 			original.setLocalSize(edited.getLocalSize());
 			original.setReturnAddressOffset(edited.getReturnAddressOffset());
 
-			// first-pass: remove deleted params from end of param list if possible 
+			// first-pass: remove deleted params from end of param list if possible
 			// to avoid custom storage enablement
 			Parameter[] origParams = function.getParameters();
 			for (int i = origParams.length - 1; i >= 0; --i) {
@@ -1214,7 +1212,7 @@ public class StackEditorModel extends CompositeEditorModel {
 				return;
 			}
 
-			// Don't try to actually rename, since we shouldn't get name change on a 
+			// Don't try to actually rename, since we shouldn't get name change on a
 			// fabricated stack data type.
 			OffsetPairs offsetSelection = getRelOffsetSelection();
 			fireTableDataChanged();
@@ -1285,7 +1283,7 @@ public class StackEditorModel extends CompositeEditorModel {
 
 	//**************************************************************************
 	// The methods below were overridden to prevent data types with a length of
-	// -1 from being applied in the stack editor. We also don't want to get 
+	// -1 from being applied in the stack editor. We also don't want to get
 	// prompted for a length when the user tries to apply a -1 length data type.
 	//**************************************************************************
 	//vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
@@ -1299,7 +1297,7 @@ public class StackEditorModel extends CompositeEditorModel {
 	}
 
 	/**
-	 * This method overrides the CompositeEditorModel to wrap the resolve of the data type 
+	 * This method overrides the CompositeEditorModel to wrap the resolve of the data type
 	 * in a transaction.
 	 */
 	@Override

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

@@ -15,7 +15,13 @@
  */
 package docking;
 
+import java.awt.Component;
+import java.awt.KeyboardFocusManager;
 import java.util.*;
+import java.util.stream.Collectors;
+
+import javax.swing.JComponent;
+import javax.swing.SwingUtilities;
 
 import docking.action.DockingActionIf;
 
@@ -290,13 +296,23 @@ class PlaceholderManager {
 		String name = newInfo.getName();
 		String group = newInfo.getGroup();
 
-		for (ComponentPlaceholder placeholder : activePlaceholders) {
+		KeyboardFocusManager kfm = KeyboardFocusManager.getCurrentKeyboardFocusManager();
-			if (name.equals(placeholder.getName()) && group.equals(placeholder.getGroup())) {
+		Component focusOwner = kfm.getFocusOwner();
-				return placeholder;
+		List<ComponentPlaceholder> matching = activePlaceholders.stream()
+				.filter(p -> name.equals(p.getName()) && group.equals(p.getGroup()))
+				.collect(Collectors.toList());
+
+		// prefer using the focused window
+		for (ComponentPlaceholder placeholder : matching) {
+			JComponent component = placeholder.getProviderComponent();
+			if (focusOwner != null && component != null) {
+				if (SwingUtilities.isDescendingFrom(focusOwner, component)) {
+					return placeholder;
+				}
 			}
 		}
 
-		return null;
+		return matching.stream().findAny().orElse(null);
 	}
 
 	private ComponentPlaceholder findBestUnusedPlaceholder(

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

@@ -419,6 +419,13 @@ public class KeyBindingUtils {
 
 		KeyStroke keyStroke = null;
 		KeyStroke[] keys = inputMap.allKeys();
+		if (keys == null) {
+			Msg.debug(KeyBindingUtils.class,
+				"Cannot remove action by name; does not exist: '" + actionName + "' " +
+					"on component: " + component.getClass().getSimpleName());
+			return;
+		}
+
 		for (KeyStroke ks : keys) {
 			Object object = inputMap.get(ks);
 			if (actionName.equals(object)) {

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

@@ -419,7 +419,7 @@ public class GTable extends JTable {
 		putClientProperty("JTable.autoStartsEdit", allowAutoEdit);
 	}
 
-	private void installEditKeyBinding() {
+	protected void installEditKeyBinding() {
 		AbstractAction action = new AbstractAction("StartEdit") {
 			@Override
 			public void actionPerformed(ActionEvent ev) {

Ghidra/Framework/Generic/src/main/java/ghidra/util/exception/DuplicateNameException.java

@@ -1,6 +1,5 @@
 /* ###
  * 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.
@@ -16,24 +15,23 @@
  */
 package ghidra.util.exception;
 
-
 /**
  * Exception thrown whenever a method tries give something a name and that name is already used.
  */
 public class DuplicateNameException extends UsrException {
-	
+
-    /**
+	/**
-     * constructs a new DuplicatenameException with a default message.
+	 * constructs a new DuplicatenameException with a default message.
-     */
+	 */
-    public DuplicateNameException() {
+	public DuplicateNameException() {
 		super("That name is already in use.");
 	}
 
-    /**
+	/**
-     * construct a new DuplicateNameException with a given message.
+	 * construct a new DuplicateNameException with a given message.
-     *
+	 *
-     * @param usrMessage overides the default message.
+	 * @param usrMessage overrides the default message.
-     */
+	 */
 	public DuplicateNameException(String usrMessage) {
 		super(usrMessage);
 	}