diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/compositeeditor/CompositeEditorPanel.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/compositeeditor/CompositeEditorPanel.java index 3d6756c789..a928905f67 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/compositeeditor/CompositeEditorPanel.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/compositeeditor/CompositeEditorPanel.java @@ -578,6 +578,14 @@ public abstract class CompositeEditorPanel extends JPanel return; } model.setSelection(table.getSelectedRows()); + + }); + + table.getColumnModel().getSelectionModel().addListSelectionListener(e -> { + if (e.getValueIsAdjusting()) { + return; + } + model.setColumn(e.getFirstIndex()); }); JPanel tablePanel = new JPanel(new BorderLayout()); diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/compositeeditor/EditFieldAction.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/compositeeditor/EditFieldAction.java index 3b86fd3f58..a923f8535a 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/compositeeditor/EditFieldAction.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/compositeeditor/EditFieldAction.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. @@ -18,7 +17,6 @@ package ghidra.app.plugin.core.compositeeditor; import java.awt.event.KeyEvent; -import javax.swing.JTable; import javax.swing.KeyStroke; import docking.ActionContext; @@ -49,9 +47,7 @@ public class EditFieldAction extends CompositeEditorTableAction { public void actionPerformed(ActionContext context) { if (model != null) { int row = model.getRow(); - JTable table = provider.getTable(); - int column = table.getSelectedColumn(); - + int column = model.getColumn(); if (model.isCellEditable(row, column)) { model.beginEditingField(row, column); return; diff --git a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/compositeeditor/AbstractStructureEditorTest.java b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/compositeeditor/AbstractStructureEditorTest.java index 6da9a27c4c..fb3236d068 100644 --- a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/compositeeditor/AbstractStructureEditorTest.java +++ b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/compositeeditor/AbstractStructureEditorTest.java @@ -15,7 +15,9 @@ */ package ghidra.app.plugin.core.compositeeditor; -import static org.junit.Assert.assertEquals; +import static org.junit.Assert.*; + +import javax.swing.JTextField; import org.junit.Assert; @@ -178,4 +180,8 @@ public abstract class AbstractStructureEditorTest extends AbstractEditorTest { return (String) model.getValueAt(index, model.getMnemonicColumn()); } + protected void setText(String s) { + JTextField tf = getActiveEditorTextField(); + setText(tf, s); + } } diff --git a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/compositeeditor/StructureEditorLockedCellEditTest.java b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/compositeeditor/StructureEditorLockedCellEditTest.java index fc4e5da313..6faa392580 100644 --- a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/compositeeditor/StructureEditorLockedCellEditTest.java +++ b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/compositeeditor/StructureEditorLockedCellEditTest.java @@ -22,7 +22,6 @@ import java.awt.event.KeyEvent; import java.io.File; import javax.swing.JTable; -import javax.swing.JTextField; import org.junit.*; @@ -781,9 +780,4 @@ public class StructureEditorLockedCellEditTest extends AbstractStructureEditorTe escape(); assertNotEditingField(); } - - protected void setText(String s) { - JTextField tf = getActiveEditorTextField(); - setText(tf, s); - } } diff --git a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/compositeeditor/StructureEditorProviderTest.java b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/compositeeditor/StructureEditorProviderTest.java index f9b3fb335a..741028b91a 100644 --- a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/compositeeditor/StructureEditorProviderTest.java +++ b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/compositeeditor/StructureEditorProviderTest.java @@ -101,36 +101,6 @@ public class StructureEditorProviderTest extends AbstractStructureEditorTest { } } -// public void testReplaceDataType() throws Exception { -// try { -// txId = program.startTransaction("Replace DataType"); -// assertEquals(87, complexStructure.getComponent(12).getDataType().getLength()); -// assertEquals(29, complexStructure.getComponent(15).getDataType().getLength()); -// assertEquals(29, complexStructure.getComponent(20).getDataType().getLength()); -// assertEquals(87, complexStructure.getComponent(12).getLength()); -// assertEquals(29, complexStructure.getComponent(15).getLength()); -// assertEquals(29, complexStructure.getComponent(20).getLength()); -// assertEquals(87, complexStructure.getLength()); -// assertEquals(21, complexStructure.getNumComponents()); -// final Structure newSimpleStructure = new StructureDataType(new CategoryPath("/aa/bb"), "simpleStructure", 10); -// newSimpleStructure.add(new PointerDataType(), 8); -// newSimpleStructure.replace(2, new AsciiDataType(), 1); -// int newStructLen = newSimpleStructure.getLength(); -// // Change the struct. simpleStructure was 29 bytes. -// programDTM.replaceDataType(simpleStructure, newSimpleStructure, true); -// assertEquals(54, complexStructure.getComponent(12).getDataType().getLength()); -// assertEquals(18, complexStructure.getComponent(15).getDataType().getLength()); -// assertEquals(18, complexStructure.getComponent(20).getDataType().getLength()); -// assertEquals(54, complexStructure.getComponent(12).getLength()); -// assertEquals(18, complexStructure.getComponent(15).getLength()); -// assertEquals(18, complexStructure.getComponent(20).getLength()); -// assertEquals(56, complexStructure.getLength()); -// assertEquals(21, complexStructure.getNumComponents()); -// } finally { -// program.endTransaction(txId, true); -// } -// } -// // Test Undo / Redo of program. @Test public void testModifiedDtAndProgramRestored() throws Exception { @@ -140,7 +110,7 @@ public class StructureEditorProviderTest extends AbstractStructureEditorTest { init(complexStructure, pgmTestCat, false); program.addListener(restoreListener); - // Change the union. + // Change the structure runSwingLater(() -> { getTable().requestFocus(); setSelection(new int[] { 4, 5 }); @@ -476,7 +446,7 @@ public class StructureEditorProviderTest extends AbstractStructureEditorTest { init(complexStructure, pgmTestCat, false); program.addListener(restoreListener); - // Change the union. + // Change the structure runSwingLater(() -> { getTable().requestFocus(); setSelection(new int[] { 4, 5 }); @@ -531,7 +501,7 @@ public class StructureEditorProviderTest extends AbstractStructureEditorTest { init(complexStructure, pgmTestCat, false); DataType oldDt = model.viewComposite.clone(null); - // Change the union. + // Change the structure runSwingLater(() -> { getTable().requestFocus(); setSelection(new int[] { 4, 5 }); @@ -567,7 +537,7 @@ public class StructureEditorProviderTest extends AbstractStructureEditorTest { init(complexStructure, pgmTestCat, false); DataType oldDt = model.viewComposite.clone(null); - // Change the union. + // Change the structure runSwing(() -> { getTable().requestFocus(); setSelection(new int[] { 4, 5 }); @@ -601,7 +571,7 @@ public class StructureEditorProviderTest extends AbstractStructureEditorTest { Window dialog; init(complexStructure, pgmTestCat, false); - // Change the union. + // Change the structure runSwingLater(() -> { getTable().requestFocus(); setSelection(new int[] { 4, 5 }); @@ -630,8 +600,37 @@ public class StructureEditorProviderTest extends AbstractStructureEditorTest { assertTrue(newDt.isEquivalent(model.viewComposite)); } - private void runSwingLater(Runnable r) { - runSwing(r, false); + @Test + public void testEditWillReEditLastColumnWhenPressingKeyboardEditAction() throws Exception { + // + // This is a regression test. The user would edit a field by pressing the keyboard edit + // action (F2). The user would finish the edit by pressing Enter. The UI would show the + // current table cell as the cell that was just edited (this is correct). When the user + // again presses the edit actions, the first editable cell in the current row would start + // to be edited (this was incorrect). This test ensures that the currently selected cell + // is edited in this case. + // + + init(simpleStructure, pgmBbCat, false); + + int row = 3; + int column = model.getNameColumn(); + assertNull(getComment(3)); + clickTableCell(getTable(), row, column, 1); + performAction(editFieldAction, provider, true); + + assertIsEditingField(row, column); + + setText("Wow"); + enter(); + + assertNotEditingField(); + assertEquals(1, model.getNumSelectedRows()); + assertEquals(3, model.getMinIndexSelected()); + assertEquals("Wow", getFieldName(3)); + + performAction(editFieldAction, provider, true); + assertIsEditingField(row, column); } @Test @@ -729,4 +728,8 @@ public class StructureEditorProviderTest extends AbstractStructureEditorTest { assertEquals("325", model.getLengthAsString()); } + private void runSwingLater(Runnable r) { + runSwing(r, false); + } + }