GP-4491 fixed references dialog to support keyboard navigation and also added accessible descriptions to all its fields

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/references/EditExternalReferencePanel.java

@@ -71,6 +71,7 @@ class EditExternalReferencePanel extends EditReferencePanel {
 
 		topPanel.add(new GLabel("Name:", SwingConstants.RIGHT));
 		extLibName = new GhidraComboBox<>();
+		extLibName.getAccessibleContext().setAccessibleDescription("Choose external program name");
 		extLibName.setEditable(true);
 		extLibName.addDocumentListener(new DocumentListener() {
 			@Override
@@ -136,10 +137,12 @@ class EditExternalReferencePanel extends EditReferencePanel {
 
 		bottomPanel.add(new GLabel("Label:", SwingConstants.RIGHT));
 		extLabel = new JTextField();
+		extLabel.getAccessibleContext().setAccessibleName("External Label");
 		bottomPanel.add(extLabel);
 
 		bottomPanel.add(new GLabel("Address:", SwingConstants.RIGHT));
 		extAddr = new AddressInput();
+		extAddr.getAccessibleContext().setAccessibleName("External Address");
 		bottomPanel.add(extAddr);
 
 		setLayout(new VerticalLayout(5));

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/references/EditMemoryReferencePanel.java

@@ -35,6 +35,7 @@ import docking.widgets.checkbox.GCheckBox;
 import docking.widgets.combobox.GhidraComboBox;
 import docking.widgets.label.GDLabel;
 import docking.widgets.label.GLabel;
+import docking.widgets.table.GTable;
 import generic.theme.GColor;
 import generic.theme.GThemeDefaults.Colors;
 import ghidra.app.util.AddressInput;
@@ -79,7 +80,7 @@ class EditMemoryReferencePanel extends EditReferencePanel {
 	private long defaultOffset;
 	private JWindow historyWin;
 	private HistoryTableModel model;
-	private JTable displayTable;
+	private GTable displayTable;
 
 	private boolean isValidState;
 
@@ -98,9 +99,13 @@ class EditMemoryReferencePanel extends EditReferencePanel {
 		setLayout(new PairLayout(10, 10, 160));
 
 		offsetCheckbox = new GCheckBox("Offset:");
+		offsetCheckbox.getAccessibleContext()
+				.setAccessibleDescription(
+					"Selecting this checkbox allows entering a refernce offset");
 		offsetCheckbox.setHorizontalAlignment(SwingConstants.RIGHT);
 		offsetCheckbox.addChangeListener(e -> enableOffsetField(offsetCheckbox.isSelected()));
 		offsetField = new JTextField();
+		offsetField.getAccessibleContext().setAccessibleName("Enter Offset");
 
 		addrLabel = new GDLabel("Base Address:");
 		addrLabel.setHorizontalAlignment(SwingConstants.RIGHT);
@@ -108,19 +113,12 @@ class EditMemoryReferencePanel extends EditReferencePanel {
 		addrLabel.setPreferredSize(d);
 
 		toAddressField = new AddressInput();
+		addrLabel.setLabelFor(toAddressField);
 
 		addrHistoryButton = new GButton(MENU_ICON);
-		addrHistoryButton.addMouseListener(new MouseAdapter() {
-			@Override
-			public void mousePressed(MouseEvent e) {
-				if (addrHistoryButton.isEnabled()) {
-					toggleAddressHistoryPopup();
-				}
-			}
-		});
+		addrHistoryButton.addActionListener(e -> toggleAddressHistoryPopup());
 		addrHistoryButton.setText(null);
 		addrHistoryButton.setMargin(new Insets(0, 0, 0, 0));
-		addrHistoryButton.setFocusable(false);
 		addrHistoryButton.setToolTipText("Address History");
 
 		includeOtherOverlaysCheckbox = new JCheckBox("Include OTHER overlay spaces",
@@ -128,6 +126,7 @@ class EditMemoryReferencePanel extends EditReferencePanel {
 		includeOtherOverlaysCheckbox.addChangeListener(e -> refreshToAddressField());
 
 		refTypes = new GhidraComboBox<>(MEM_REF_TYPES);
+		refTypes.getAccessibleContext().setAccessibleName("Memory Ref Types");
 
 		JPanel addrPanel = new JPanel(new BorderLayout());
 		addrPanel.add(toAddressField, BorderLayout.CENTER);
@@ -554,25 +553,27 @@ class EditMemoryReferencePanel extends EditReferencePanel {
 			return;
 		}
 
-		List<Address> list = addrHistoryMap.get(fromCodeUnit.getProgram());
-		Address[] addrs = new Address[list.size()];
-		list.toArray(addrs);
-
 		JPanel panel = new JPanel(new BorderLayout(0, 0));
 
 		model = new HistoryTableModel(fromCodeUnit.getProgram());
-		displayTable = new JTable(model);
+		displayTable = new GTable(model);
 		displayTable.setTableHeader(null);
 		displayTable.setBorder(new LineBorder(Colors.BORDER));
 		displayTable.setSelectionMode(ListSelectionModel.SINGLE_SELECTION);
+		displayTable.addKeyListener(new KeyAdapter() {
+			@Override
+			public void keyPressed(KeyEvent e) {
+				int keyCode = e.getKeyCode();
+				if (keyCode == KeyEvent.VK_ENTER || keyCode == KeyEvent.VK_SPACE) {
+					e.consume();
+				}
+			}
+		});
 
 		displayTable.addMouseListener(new MouseAdapter() {
 			@Override
 			public void mouseClicked(MouseEvent e) {
-				int row = displayTable.getSelectedRow();
-				Address addr = model.getAddress(row);
-				toAddressField.setAddress(addr);
-				toggleAddressHistoryPopup();
+				chooseEntry();
 			}
 
 			@Override
@@ -596,13 +597,17 @@ class EditMemoryReferencePanel extends EditReferencePanel {
 
 		panel.add(displayTable, BorderLayout.CENTER);
 
+		// Sets the preferred size to the table inside this popup history window so that its width 
+		// is the same as the text field and button to make it resemble the look of a combo box.
+		// We also had to add a fudge factor to the height to keep it from truncating the last
+		// row in the table.
 		int w = toAddressField.getWidth() + addrHistoryButton.getWidth();
 		Dimension d = displayTable.getPreferredSize();
-		displayTable.setPreferredSize(new Dimension(w, d.height));
+		displayTable.setPreferredSize(new Dimension(w, d.height + 10));
 
 		Window dlgWin = findMyWindow();
 		historyWin = new JWindow(dlgWin);
-		historyWin.getContentPane().setLayout(new BorderLayout(0, 0));
+		historyWin.getContentPane().setLayout(new BorderLayout());
 		historyWin.getContentPane().add(panel, BorderLayout.CENTER);
 		historyWin.pack();
 
@@ -655,6 +660,15 @@ class EditMemoryReferencePanel extends EditReferencePanel {
 		});
 	}
 
+	private void chooseEntry() {
+		int row = displayTable.getSelectedRow();
+		if (row >= 0) {
+			Address addr = model.getAddress(row);
+			toAddressField.setAddress(addr);
+			toggleAddressHistoryPopup();
+		}
+	}
+
 	private void updateTableSelectionForEvent(MouseEvent anEvent) {
 		Point location = anEvent.getPoint();
 		if (displayTable == null) {

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/references/EditReferenceDialog.java

@@ -159,7 +159,6 @@ public class EditReferenceDialog extends ReusableDialogComponentProvider {
 
 		bottomPanelLayout = new CardLayout();
 		bottomPanel = new JPanel(bottomPanelLayout);
-		bottomPanel.setFocusCycleRoot(true);
 		bottomPanel.setPreferredSize(new Dimension(PREFERRED_PANEL_WIDTH, PREFERRED_PANEL_HEIGHT));
 		bottomPanel.setBorder(new EmptyBorder(0, 2, 0, 2));
 
@@ -235,7 +234,6 @@ public class EditReferenceDialog extends ReusableDialogComponentProvider {
 			activeRefPanel = extRefPanel;
 		}
 		bottomPanelLayout.show(bottomPanel, activeRefPanel.getName());
-		activeRefPanel.requestFocus();
 	}
 
 	public void initDialog(CodeUnit cu, int opIndex, int subIndex, Reference ref) {
@@ -252,7 +250,6 @@ public class EditReferenceDialog extends ReusableDialogComponentProvider {
 		}
 
 		initializing = false;
-		activeRefPanel.requestFocus();
 	}
 
 	private void configureAddReference(int opIndex, int subIndex) {

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/references/EditRegisterReferencePanel.java

@@ -56,9 +56,10 @@ class EditRegisterReferencePanel extends EditReferencePanel {
 		setBorder(new EmptyBorder(0, 5, 5, 5));
 
 		regList = new GhidraComboBox<>();
+		regList.getAccessibleContext().setAccessibleName("Registers");
 
 		refTypes = new GhidraComboBox<>(REGISTER_REF_TYPES);
-
+		refTypes.getAccessibleContext().setAccessibleName("Ref Types");
 		add(new GLabel("Register:", SwingConstants.RIGHT));
 		add(regList);
 		add(new GLabel("Ref-Type:", SwingConstants.RIGHT));
@@ -180,8 +181,9 @@ class EditRegisterReferencePanel extends EditReferencePanel {
 			return false;
 		}
 
-		Function f = fromCodeUnit.getProgram().getFunctionManager().getFunctionContaining(
-			fromCodeUnit.getMinAddress());
+		Function f = fromCodeUnit.getProgram()
+				.getFunctionManager()
+				.getFunctionContaining(fromCodeUnit.getMinAddress());
 		if (f == null) {
 			return false;
 		}
@@ -207,8 +209,9 @@ class EditRegisterReferencePanel extends EditReferencePanel {
 			throw new IllegalStateException();
 		}
 
-		Function f = fromCodeUnit.getProgram().getFunctionManager().getFunctionContaining(
-			fromCodeUnit.getMinAddress());
+		Function f = fromCodeUnit.getProgram()
+				.getFunctionManager()
+				.getFunctionContaining(fromCodeUnit.getMinAddress());
 		if (f == null) {
 			// Function no longer exists
 			showInputErr("Register reference not permitted!\nAddress " +

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/references/EditStackReferencePanel.java

@@ -58,8 +58,10 @@ class EditStackReferencePanel extends EditReferencePanel {
 		setBorder(new EmptyBorder(0, 5, 5, 5));
 
 		stackOffset = new JTextField();
+		stackOffset.getAccessibleContext().setAccessibleName("Stack Offset");
 
 		refTypes = new GhidraComboBox<>(STACK_REF_TYPES);
+		refTypes.getAccessibleContext().setAccessibleName("Ref Types");
 
 		add(new GLabel("Stack Offset:", SwingConstants.RIGHT));
 		add(stackOffset);
@@ -129,8 +131,9 @@ class EditStackReferencePanel extends EditReferencePanel {
 			return false;
 		}
 
-		Function f = fromCodeUnit.getProgram().getFunctionManager().getFunctionContaining(
-			fromCodeUnit.getMinAddress());
+		Function f = fromCodeUnit.getProgram()
+				.getFunctionManager()
+				.getFunctionContaining(fromCodeUnit.getMinAddress());
 		if (f == null) {
 			return false;
 		}

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/references/InstructionPanel.java

@@ -32,6 +32,7 @@ import docking.actions.KeyBindingUtils;
 import docking.dnd.DropTgtAdapter;
 import docking.dnd.Droppable;
 import docking.widgets.label.GDLabel;
+import generic.theme.GColor;
 import generic.theme.GThemeDefaults.Colors;
 import generic.theme.Gui;
 import ghidra.app.util.*;
@@ -44,10 +45,10 @@ import ghidra.program.model.mem.Memory;
 import ghidra.program.model.symbol.*;
 
 class InstructionPanel extends JPanel implements ChangeListener {
-
+	private static Color FOCUS_COLOR = new GColor("color.palette.yellow");
 	private static final int BORDER_SIZE = 2;
-	private static final Border EMPTY_BORDER = new EmptyBorder(BORDER_SIZE,
-		BORDER_SIZE, BORDER_SIZE, BORDER_SIZE);
+	private static final Border EMPTY_BORDER =
+		new EmptyBorder(BORDER_SIZE, BORDER_SIZE, BORDER_SIZE, BORDER_SIZE);
 	private static final Border ETCHED_BORDER = new EtchedBorder();
 
 	private final static Color NOT_IN_MEMORY_COLOR = Colors.ERROR;
@@ -75,79 +76,13 @@ class InstructionPanel extends JPanel implements ChangeListener {
 
 	private boolean dropSupported;
 	private DropTgtAdapter dropTargetAdapter;
-	private Droppable dropHandler = new Droppable() {
-
-		/**
-		 * Set drag feedback according to the ok parameter.
-		 * @param ok true means the drop action is OK
-		 * @param e event that has current state of drag and drop operation
-		 */
-		@Override
-		public void dragUnderFeedback(boolean ok, DropTargetDragEvent e) {
-			// stub
-		}
-
-		/**
-		 * Return true if is OK to drop the transferable at the location
-		 * specified the event.
-		 * @param e event that has current state of drag and drop operation
-		 */
-		@Override
-		public boolean isDropOk(DropTargetDragEvent e) {
-
-			Component targetComp = e.getDropTargetContext().getComponent();
-			if (targetComp instanceof JLabel) {
-
-				updateLabels(getLabelIndex((JLabel) targetComp), -1);
-
-				try {
-					Object data = e.getTransferable()
-							.getTransferData(
-								SelectionTransferable.localProgramSelectionFlavor);
-					AddressSetView view = ((SelectionTransferData) data).getAddressSet();
-					if (memory.contains(view)) {
-						return true;
-					}
-				}
-				catch (UnsupportedFlavorException e1) {
-					// return false
-				}
-				catch (IOException e1) {
-					// return false
-				}
-			}
-			return false;
-		}
-
-		@Override
-		public void undoDragUnderFeedback() {
-			// stub
-		}
-
-		/**
-		 * Add the object to the droppable component. The DropTargetAdapter
-		 * calls this method from its drop() method.
-		 * @param obj Transferable object that is to be dropped; in this
-		 * case, it is an AddressSetView
-		 * @param e  has current state of drop operation
-		 * @param f represents the opaque concept of a data format as
-		 * would appear on a clipboard, during drag and drop.
-		 */
-		@Override
-		public void add(Object obj, DropTargetDropEvent e, DataFlavor f) {
-			AddressSetView view = ((SelectionTransferData) obj).getAddressSet();
-			if (view.getNumAddressRanges() == 0) {
-				return;
-			}
-			listener.selectionDropped(view, currentCodeUnit, activeIndex);
-		}
-
-	};
+	private Droppable dropHandler = new InstructionPanelDroppable();
+	private int nOperands;
 
 	InstructionPanel(int topPad, int leftPad, int bottomPad, int rightPad,
 			DockingAction goHomeAction, ReferencesPlugin plugin,
 			InstructionPanelListener listener) {
-		super();
+		super(new BorderLayout());
 		this.dropSupported = listener != null ? listener.dropSupported() : false;
 		this.goHomeAction = goHomeAction;
 		this.symbolInspector = plugin.getSymbolInspector();
@@ -156,13 +91,39 @@ class InstructionPanel extends JPanel implements ChangeListener {
 		create(topPad, leftPad, bottomPad, rightPad);
 	}
 
+	private int getNextIndex() {
+		if (operandLabels.length == 0) {
+			return ReferenceManager.MNEMONIC;
+		}
+		if (activeIndex == ReferenceManager.MNEMONIC) {
+			return 0;
+		}
+		if (activeIndex < nOperands - 1) {
+			return activeIndex + 1;
+		}
+		return ReferenceManager.MNEMONIC;
+	}
+
+	private int getPreviousIndex() {
+		if (operandLabels.length == 0) {
+			return ReferenceManager.MNEMONIC;
+		}
+		if (activeIndex == ReferenceManager.MNEMONIC) {
+			return nOperands - 1;
+		}
+		if (activeIndex > 0) {
+			return activeIndex - 1;
+		}
+		return ReferenceManager.MNEMONIC;
+	}
+
 	CodeUnit getCurrentCodeUnit() {
 		return currentCodeUnit;
 	}
 
 	@Override
 	public void stateChanged(ChangeEvent e) {
-		updateLabels(activeIndex, activeSubIndex);
+		updateActiveIndex(activeIndex, activeSubIndex);
 	}
 
 	void setCodeUnitLocation(CodeUnit cu, int opIndex, int subIndex, boolean locked) {
@@ -180,12 +141,12 @@ class InstructionPanel extends JPanel implements ChangeListener {
 		}
 		currentCodeUnit = cu;
 		activeIndex = ReferenceManager.MNEMONIC - 1; // force updateLabels to work
-		updateLabels(opIndex, subIndex);
+		updateActiveIndex(opIndex, subIndex);
 		updateDropTargets(cu != null ? cu.getNumOperands() : -1);
 	}
 
 	void setSelectedOpIndex(int index, int subIndex) {
-		updateLabels(index, subIndex);
+		updateActiveIndex(index, subIndex);
 	}
 
 	int getSelectedOpIndex() {
@@ -200,9 +161,6 @@ class InstructionPanel extends JPanel implements ChangeListener {
 	 * Create the components for this panel.
 	 */
 	private void create(int topPad, int leftPad, int bottomPad, int rightPad) {
-		setLayout(new BorderLayout());
-		//setBorder(new EmptyBorder(topPad, leftPad, bottomPad, rightPad));
-
 		Border border = new TitledBorder(new EtchedBorder(), "Source");
 		setBorder(border);
 
@@ -226,15 +184,20 @@ class InstructionPanel extends JPanel implements ChangeListener {
 		innerPanel = new JPanel();
 		BoxLayout bl = new BoxLayout(innerPanel, BoxLayout.X_AXIS);
 		innerPanel.setLayout(bl);
+		setName("Instruction Panel");
+		setToolTipText("This component selects which instruction piece is active for this dialog");
+		innerPanel.getAccessibleContext()
+				.setAccessibleDescription("Use left or right arrows to choose which mnemonic or" +
+					" operand piece this dialog applies to");
+		updateAccessibleInfo();
 
 		if (goHomeAction != null) {
 			Action action = KeyBindingUtils.adaptDockingActionToNonContextAction(goHomeAction);
 			JButton homeButton = new JButton(action);
 			homeButton.setText(null);
 			homeButton.setMargin(new Insets(0, 0, 0, 0));
-			homeButton.setFocusable(false);
 			innerPanel.add(Box.createHorizontalStrut(5));
-			innerPanel.add(homeButton);
+			add(homeButton, BorderLayout.WEST);
 		}
 
 		innerPanel.add(Box.createHorizontalStrut(5));
@@ -242,6 +205,7 @@ class InstructionPanel extends JPanel implements ChangeListener {
 		innerPanel.add(Box.createHorizontalStrut(20));
 		innerPanel.add(mnemonicLabel);
 		innerPanel.add(Box.createHorizontalStrut(10));
+		innerPanel.setBorder(BorderFactory.createEmptyBorder(1, 1, 1, 1));
 
 		for (JLabel operandLabel : operandLabels) {
 			innerPanel.add(operandLabel);
@@ -267,6 +231,35 @@ class InstructionPanel extends JPanel implements ChangeListener {
 			}
 
 		}
+		innerPanel.setFocusable(true);
+		innerPanel.addKeyListener(new KeyAdapter() {
+			@Override
+			public void keyPressed(KeyEvent e) {
+				switch (e.getKeyCode()) {
+					case KeyEvent.VK_LEFT:
+						updateActiveIndex(getPreviousIndex(), -1);
+						e.consume();
+						break;
+					case KeyEvent.VK_RIGHT:
+						updateActiveIndex(getNextIndex(), -1);
+						e.consume();
+						break;
+				}
+			}
+		});
+		innerPanel.addFocusListener(new FocusListener() {
+
+			@Override
+			public void focusLost(FocusEvent e) {
+				innerPanel.setBorder(BorderFactory.createEmptyBorder(1, 1, 1, 1));
+			}
+
+			@Override
+			public void focusGained(FocusEvent e) {
+				innerPanel.setBorder(BorderFactory.createLineBorder(FOCUS_COLOR, 1));
+			}
+		});
+
 	}
 
 	/**
@@ -286,10 +279,30 @@ class InstructionPanel extends JPanel implements ChangeListener {
 	/**
 	 * Method updateLabels.
 	 */
-	private void updateLabels(int index, int subIndex) {
+	private void updateActiveIndex(int index, int subIndex) {
 		int prevIndex = activeIndex;
 		activeIndex = index;
 		activeSubIndex = subIndex;
+		updateLabels();
+		updateAccessibleInfo();
+		if (activeIndex != prevIndex && listener != null) {
+			listener.operandSelected(activeIndex, activeSubIndex);
+		}
+		updateAccessibleInfo();
+	}
+
+	private void updateAccessibleInfo() {
+		String accessibleName = "Instruction Reference Panel";
+		if (activeIndex < 0) {
+			accessibleName += ", mnemonic selected";
+		}
+		else {
+			accessibleName += ", operand " + activeIndex + " selected";
+		}
+		innerPanel.getAccessibleContext().setAccessibleName(accessibleName);
+	}
+
+	private void updateLabels() {
 		for (JLabel operandLabel : operandLabels) {
 			operandLabel.setText("");
 			operandLabel.setBorder(EMPTY_BORDER);
@@ -297,7 +310,7 @@ class InstructionPanel extends JPanel implements ChangeListener {
 		}
 		if (currentCodeUnit != null) {
 
-			int nOperands = currentCodeUnit.getNumOperands();
+			nOperands = currentCodeUnit.getNumOperands();
 			for (int i = 0; i < nOperands; i++) {
 				String opRep = cuFormat.getOperandRepresentationString(currentCodeUnit, i);
 				if (i < nOperands - 1) {
@@ -315,9 +328,6 @@ class InstructionPanel extends JPanel implements ChangeListener {
 		}
 		innerPanel.invalidate();
 		repaint();
-		if (activeIndex != prevIndex && listener != null) {
-			listener.operandSelected(activeIndex, activeSubIndex);
-		}
 	}
 
 	/**
@@ -418,9 +428,75 @@ class InstructionPanel extends JPanel implements ChangeListener {
 		public void mousePressed(MouseEvent e) {
 			if (!locked) {
 				JLabel label = (JLabel) e.getSource();
-				updateLabels(getLabelIndex(label), -1);
+				updateActiveIndex(getLabelIndex(label), -1);
 			}
 		}
 	}
 
+	private class InstructionPanelDroppable implements Droppable {
+		/**
+		 * Set drag feedback according to the ok parameter.
+		 * @param ok true means the drop action is OK
+		 * @param e event that has current state of drag and drop operation
+		 */
+		@Override
+		public void dragUnderFeedback(boolean ok, DropTargetDragEvent e) {
+			// stub
+		}
+
+		/**
+		 * Return true if is OK to drop the transferable at the location
+		 * specified the event.
+		 * @param e event that has current state of drag and drop operation
+		 */
+		@Override
+		public boolean isDropOk(DropTargetDragEvent e) {
+
+			Component targetComp = e.getDropTargetContext().getComponent();
+			if (targetComp instanceof JLabel) {
+
+				updateActiveIndex(getLabelIndex((JLabel) targetComp), -1);
+
+				try {
+					Object data = e.getTransferable()
+							.getTransferData(SelectionTransferable.localProgramSelectionFlavor);
+					AddressSetView view = ((SelectionTransferData) data).getAddressSet();
+					if (memory.contains(view)) {
+						return true;
+					}
+				}
+				catch (UnsupportedFlavorException e1) {
+					// return false
+				}
+				catch (IOException e1) {
+					// return false
+				}
+			}
+			return false;
+		}
+
+		@Override
+		public void undoDragUnderFeedback() {
+			// stub
+		}
+
+		/**
+		 * Add the object to the droppable component. The DropTargetAdapter
+		 * calls this method from its drop() method.
+		 * @param obj Transferable object that is to be dropped; in this
+		 * case, it is an AddressSetView
+		 * @param e  has current state of drop operation
+		 * @param f represents the opaque concept of a data format as
+		 * would appear on a clipboard, during drag and drop.
+		 */
+		@Override
+		public void add(Object obj, DropTargetDropEvent e, DataFlavor f) {
+			AddressSetView view = ((SelectionTransferData) obj).getAddressSet();
+			if (view.getNumAddressRanges() == 0) {
+				return;
+			}
+			listener.selectionDropped(view, currentCodeUnit, activeIndex);
+		}
+
+	}
 }