From ac22ed69f28be132b35e533cd388c26227775eee Mon Sep 17 00:00:00 2001 From: dragonmacher <48328597+dragonmacher@users.noreply.github.com> Date: Tue, 19 Nov 2024 18:07:54 -0500 Subject: [PATCH] Test fixes --- .../StructureEditorUnlockedCellEdit2Test.java | 7 +- .../core/memory/MemoryMapPluginTest.java | 59 +---- .../core/memory/MemoryMapProvider1Test.java | 27 +-- .../docking/widgets/DropDownTextField.java | 214 ++++++++---------- .../table/AbstractSortedTableModel.java | 12 +- 5 files changed, 112 insertions(+), 207 deletions(-) diff --git a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/compositeeditor/StructureEditorUnlockedCellEdit2Test.java b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/compositeeditor/StructureEditorUnlockedCellEdit2Test.java index 4494afcaab..8e921a7d73 100644 --- a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/compositeeditor/StructureEditorUnlockedCellEdit2Test.java +++ b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/compositeeditor/StructureEditorUnlockedCellEdit2Test.java @@ -4,9 +4,9 @@ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. @@ -207,8 +207,7 @@ public class StructureEditorUnlockedCellEdit2Test extends AbstractStructureEdito assertEquals(model.getDataTypeColumn(), model.getColumn()); // Bad value allows escape. - escape(); - escape(); + escape(); // cancel editing waitForSwing(); assertTrue(!model.isEditingField()); assertEquals(1, model.getNumSelectedRows()); diff --git a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/memory/MemoryMapPluginTest.java b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/memory/MemoryMapPluginTest.java index 132e1341c5..53fc7a2865 100644 --- a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/memory/MemoryMapPluginTest.java +++ b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/memory/MemoryMapPluginTest.java @@ -4,9 +4,9 @@ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. @@ -19,7 +19,6 @@ import static org.junit.Assert.*; import java.awt.*; import java.awt.event.MouseEvent; -import java.util.Set; import javax.swing.JTable; import javax.swing.JTextField; @@ -27,8 +26,6 @@ import javax.swing.table.TableModel; import org.junit.*; -import docking.ActionContext; -import docking.DefaultActionContext; import docking.action.DockingActionIf; import ghidra.app.cmd.memory.*; import ghidra.app.plugin.core.codebrowser.CodeBrowserPlugin; @@ -93,58 +90,6 @@ public class MemoryMapPluginTest extends AbstractGhidraHeadedIntegrationTest { assertTrue(action.isEnabled()); } - @Test - public void testOpenProgram() throws Exception { - env.close(program); - program = buildProgram("sdk"); - env.open(program); - Set actions = getActionsByOwner(tool, plugin.getName()); - for (DockingActionIf action : actions) { - String name = action.getName(); - if (name.equals("Add Block") || name.equals("Set Image Base") || - name.equals("Memory Map") || name.equals("Close Window") || - name.contains("Table")) { - assertActionEnabled(action, getActionContext(), true); - } - else { - assertActionEnabled(action, getActionContext(), false); - } - } - - } - - @Test - public void testCloseProgram() { - env.close(program); - JTable table = provider.getTable(); - assertEquals(0, table.getModel().getRowCount()); - Set actions = getActionsByOwner(tool, plugin.getName()); - for (DockingActionIf action : actions) { - String name = action.getName(); - if (name.equals("Memory Map") || name.equals("Close Window") || - name.equals("Local Menu")) { - continue; - } - assertActionEnabled(action, getActionContext(), false); - } - } - - private void assertActionEnabled(DockingActionIf action, ActionContext context, - boolean shouldBeEnabled) { - - String text = shouldBeEnabled ? "should be enabled" : "should be disabled"; - assertEquals("Action " + text + ", but is not: '" + action.getFullName() + "'", - shouldBeEnabled, action.isEnabledForContext(context)); - } - - private ActionContext getActionContext() { - ActionContext context = provider.getActionContext(null); - if (context == null) { - return new DefaultActionContext(); - } - return context; - } - @Test public void testBlockNameChanged() throws Exception { MemoryBlock block = memory.getBlock(program.getMinAddress()); diff --git a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/memory/MemoryMapProvider1Test.java b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/memory/MemoryMapProvider1Test.java index 406f3083d4..c7f183bedd 100644 --- a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/memory/MemoryMapProvider1Test.java +++ b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/memory/MemoryMapProvider1Test.java @@ -4,9 +4,9 @@ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. @@ -124,29 +124,6 @@ public class MemoryMapProvider1Test extends AbstractGhidraHeadedIntegrationTest } } - @Test - public void testMultiSelection() { - - table.addRowSelectionInterval(0, 1); - assertEquals(2, table.getSelectedRowCount()); - Set actions = getActionsByOwner(tool, plugin.getName()); - for (DockingActionIf action : actions) { - String name = action.getName(); - if (name.equals("Add Block") || name.equals("Merge Blocks") || - name.equals("Delete Block") || name.equals("Set Image Base") || - name.equals("Memory Map") || name.equals("Close Window") || - name.equals("Make Selection")) { - assertTrue("Action should be enabled for a multi-row selection - '" + name + "'", - action.isEnabled()); - } - else { - assertFalse( - "Action should not be enabled for a multi-row selection - '" + name + "'", - action.isEnabled()); - } - } - } - @Test public void testGoToAddress() { Rectangle rect = table.getCellRect(2, MemoryMapModel.START, true); diff --git a/Ghidra/Framework/Docking/src/main/java/docking/widgets/DropDownTextField.java b/Ghidra/Framework/Docking/src/main/java/docking/widgets/DropDownTextField.java index 3a3878ffda..1a6bc418d7 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/widgets/DropDownTextField.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/widgets/DropDownTextField.java @@ -4,9 +4,9 @@ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. @@ -177,79 +177,6 @@ public class DropDownTextField extends JTextField implements GComponent { return previewLabel; } - // overridden to grab the Escape and enter key events before our parent window gets them - @Override - protected boolean processKeyBinding(KeyStroke ks, KeyEvent e, int condition, boolean pressed) { - - if (handleEscapeKey(ks, e, condition, pressed)) { - return true; - } - - if (handleEnterKey(ks, e, condition, pressed)) { - return true; - } - return super.processKeyBinding(ks, e, condition, pressed); - } - - private boolean handleEscapeKey(KeyStroke ks, KeyEvent e, int condition, boolean pressed) { - if ((condition == JComponent.WHEN_FOCUSED) && (ks.getKeyCode() == KeyEvent.VK_ESCAPE)) { - - if (getMatchingWindow().isShowing()) { - hideMatchingWindow(); - e.consume(); - return true; - } - else if (pressed) { - // do not return after this call so that the event will continue to be processed - fireEditingCancelled(); - } - } - - return false; - } - - private boolean handleEnterKey(KeyStroke ks, KeyEvent e, int condition, boolean pressed) { - if ((condition != JComponent.WHEN_FOCUSED) || (ks.getKeyCode() != KeyEvent.VK_ENTER)) { - return false; // enter key not pressed! - } - - if (ignoreEnterKeyPress) { - return false; - } - - // O.K., if we are consuming key presses, then we only want to do so when the selection - // window is showing. This will close the selection window and not send the Enter event up - // to our parent component. - boolean listShowing = isMatchingListShowing(); - if (consumeEnterKeyPress) { - if (listShowing) { - setTextFromListOnEnterPress(); - validateChosenItemAgainstText(true); - e.consume(); - return true; // don't let our parent see the event - } - else if (pressed) { - validateChosenItemAgainstText(false); - fireEditingStopped(); - } - - // Return false, even though 'consumeEnterKeyPress' is set, so that our - // parent can process the event. - return false; - } - - // When we aren't consuming Enter key presses, then we just take the user's selection - // and signal that editing is finished, while letting our parent component handle the event - if (pressed) { - setTextFromListOnEnterPress(); - validateChosenItemAgainstText(listShowing); - fireEditingStopped(); - return true; - } - - return false; - } - private void validateChosenItemAgainstText(boolean isListShowing) { // // If the text differs from that of the chosen item, then the implication is the user has @@ -884,62 +811,117 @@ public class DropDownTextField extends JTextField implements GComponent { KeyEvent.VK_KP_DOWN)) { //@formatter:on - if (!getMatchingWindow().isShowing()) { - updateDisplayContents(getText()); - event.consume(); - } - else { // update the window if it is showing - if (keyCode == KeyEvent.VK_UP || keyCode == KeyEvent.VK_KP_UP) { - decrementListSelection(); - } - else { - incrementListSelection(); - } - event.consume(); - setTextFromSelectedListItemAndKeepMatchingWindowOpen(); - } + handleArrowKey(event); + } + else if (keyCode == KeyEvent.VK_ENTER) { + handleEnterKey(event); + } + else if (keyCode == KeyEvent.VK_ESCAPE) { + handleEscapeKey(event); } setToolTipText(getToolTipText()); } - private void incrementListSelection() { - int index = list.getSelectedIndex(); - int listSize = list.getModel().getSize(); - - if (index < 0) { // no selection - index = 0; + private void handleEscapeKey(KeyEvent event) { + if (getMatchingWindow().isShowing()) { + hideMatchingWindow(); } - else if (index == listSize - 1) { // last element selected - wrap - index = 0; + else { + fireEditingCancelled(); } - else { // just increment - index++; - } - - list.setSelectedIndex(index); - list.ensureIndexIsVisible(index); + event.consume(); } - private void decrementListSelection() { - int index = list.getSelectedIndex(); - int listSize = list.getModel().getSize(); + private void handleEnterKey(KeyEvent event) { - if (index < 0) { // no selection - index = 0; - } - else if (index == 0) { // first element - wrap - index = listSize - 1; - } - else { // just decrement - index--; + if (ignoreEnterKeyPress) { + return; } - list.setSelectedIndex(index); - list.ensureIndexIsVisible(index); + // O.K., if we are consuming key presses, then we only want to do so when the selection + // window is showing. This will close the selection window and not send the Enter event up + // to our parent component. + boolean listShowing = isMatchingListShowing(); + if (consumeEnterKeyPress) { + if (listShowing) { + setTextFromListOnEnterPress(); + validateChosenItemAgainstText(true); + event.consume(); + return; // don't let our parent see the event + } + + validateChosenItemAgainstText(false); + fireEditingStopped(); + + // Even though 'consumeEnterKeyPress' is set, do not consume the event so that our + // parent can process the event. + return; + } + + // When we aren't consuming Enter key presses, then just take the user's selection and + // signal that editing is finished, while letting our parent component handle the event + setTextFromListOnEnterPress(); + validateChosenItemAgainstText(listShowing); + fireEditingStopped(); } } + private void handleArrowKey(KeyEvent event) { + + int keyCode = event.getKeyCode(); + if (!getMatchingWindow().isShowing()) { + updateDisplayContents(getText()); + event.consume(); + } + else { // update the window if it is showing + if (keyCode == KeyEvent.VK_UP || keyCode == KeyEvent.VK_KP_UP) { + decrementListSelection(); + } + else { + incrementListSelection(); + } + event.consume(); + setTextFromSelectedListItemAndKeepMatchingWindowOpen(); + } + } + + private void incrementListSelection() { + int index = list.getSelectedIndex(); + int listSize = list.getModel().getSize(); + + if (index < 0) { // no selection + index = 0; + } + else if (index == listSize - 1) { // last element selected - wrap + index = 0; + } + else { // just increment + index++; + } + + list.setSelectedIndex(index); + list.ensureIndexIsVisible(index); + } + + private void decrementListSelection() { + int index = list.getSelectedIndex(); + int listSize = list.getModel().getSize(); + + if (index < 0) { // no selection + index = 0; + } + else if (index == 0) { // first element - wrap + index = listSize - 1; + } + else { // just decrement + index--; + } + + list.setSelectedIndex(index); + list.ensureIndexIsVisible(index); + } + // we know the cast is safe because we put the items in the list protected void setTextFromSelectedListItemAndKeepMatchingWindowOpen() { T selectedItem = list.getSelectedValue(); diff --git a/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/AbstractSortedTableModel.java b/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/AbstractSortedTableModel.java index 1f8e18051a..c44d33f504 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/AbstractSortedTableModel.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/AbstractSortedTableModel.java @@ -4,9 +4,9 @@ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. @@ -245,10 +245,12 @@ public abstract class AbstractSortedTableModel extends AbstractGTableModel protected TableSortingContext createSortingContext(TableSortState newSortState) { if (!isValidSortState(newSortState)) { - Msg.error(this, """ - "Table '%s' sort is invalid. Assuming columns have been removed. \ - Setting unsorted.""".formatted(getName())); newSortState = TableSortState.createUnsortedSortState(); + if (!isDisposed) { + Msg.error(this, """ + "Table '%s' sort is invalid. Assuming columns have been removed. \ + Setting unsorted.""".formatted(getName())); + } } return new TableSortingContext<>(newSortState, getComparatorChain(newSortState));