From dcfe7692f0f34c42ad3c6d51d3afae055afe2820 Mon Sep 17 00:00:00 2001 From: dragonmacher <48328597+dragonmacher@users.noreply.github.com> Date: Thu, 13 Nov 2025 13:16:28 -0500 Subject: [PATCH] Defined Strings - Fixed bug that caused the table to jump as users selected rows Fixes #8603 --- ...ontext.java => DefinedStringsContext.java} | 14 ++--- ...sPlugin.java => DefinedStringsPlugin.java} | 13 +++-- ...vider.java => DefinedStringsProvider.java} | 53 ++++++++++--------- ...del.java => DefinedStringsTableModel.java} | 21 ++++---- .../java/docking/widgets/table/GTable.java | 8 +++ ...a => DefinedStringsPluginScreenShots.java} | 14 ++--- 6 files changed, 66 insertions(+), 57 deletions(-) rename Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/strings/{ViewStringsContext.java => DefinedStringsContext.java} (88%) rename Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/strings/{ViewStringsPlugin.java => DefinedStringsPlugin.java} (94%) rename Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/strings/{ViewStringsProvider.java => DefinedStringsProvider.java} (86%) rename Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/strings/{ViewStringsTableModel.java => DefinedStringsTableModel.java} (96%) rename Ghidra/Test/IntegrationTest/src/screen/java/help/screenshot/{ViewStringsPluginScreenShots.java => DefinedStringsPluginScreenShots.java} (89%) diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/strings/ViewStringsContext.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/strings/DefinedStringsContext.java similarity index 88% rename from Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/strings/ViewStringsContext.java rename to Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/strings/DefinedStringsContext.java index f90617930b..ca9b6d2094 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/strings/ViewStringsContext.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/strings/DefinedStringsContext.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. @@ -28,14 +28,14 @@ import ghidra.program.util.ProgramLocation; import ghidra.program.util.ProgramSelection; import ghidra.util.table.GhidraTable; -public class ViewStringsContext extends DefaultActionContext implements DataLocationListContext { +public class DefinedStringsContext extends DefaultActionContext implements DataLocationListContext { - private final ViewStringsProvider viewStringsProvider; + private final DefinedStringsProvider viewStringsProvider; private final GhidraTable table; - private final ViewStringsTableModel tableModel; + private final DefinedStringsTableModel tableModel; - ViewStringsContext(ViewStringsProvider provider, GhidraTable table, - ViewStringsTableModel tableModel) { + DefinedStringsContext(DefinedStringsProvider provider, GhidraTable table, + DefinedStringsTableModel tableModel) { super(provider, table); this.viewStringsProvider = provider; this.table = table; diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/strings/ViewStringsPlugin.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/strings/DefinedStringsPlugin.java similarity index 94% rename from Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/strings/ViewStringsPlugin.java rename to Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/strings/DefinedStringsPlugin.java index 4fc6aa71af..f3492770fe 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/strings/ViewStringsPlugin.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/strings/DefinedStringsPlugin.java @@ -58,7 +58,7 @@ import resources.ResourceManager; servicesRequired = { GoToService.class } ) //@formatter:on -public class ViewStringsPlugin extends ProgramPlugin implements DomainObjectListener { +public class DefinedStringsPlugin extends ProgramPlugin implements DomainObjectListener { private static Icon REFRESH_ICON = Icons.REFRESH_ICON; private static Icon REFRESH_NOT_NEEDED_ICON = @@ -66,10 +66,10 @@ public class ViewStringsPlugin extends ProgramPlugin implements DomainObjectList private DockingAction refreshAction; private SelectionNavigationAction linkNavigationAction; - private ViewStringsProvider provider; + private DefinedStringsProvider provider; private SwingUpdateManager reloadUpdateMgr; - public ViewStringsPlugin(PluginTool tool) { + public DefinedStringsPlugin(PluginTool tool) { super(tool); } @@ -81,7 +81,7 @@ public class ViewStringsPlugin extends ProgramPlugin implements DomainObjectList protected void init() { super.init(); - provider = new ViewStringsProvider(this); + provider = new DefinedStringsProvider(this); reloadUpdateMgr = new SwingUpdateManager(100, 60000, this::doReload); createActions(); } @@ -105,7 +105,7 @@ public class ViewStringsPlugin extends ProgramPlugin implements DomainObjectList tool.addLocalAction(provider, linkNavigationAction); new ActionBuilder("Data Settings", getName()) // create pop-up menu item "Settings..." - .withContext(ViewStringsContext.class) + .withContext(DefinedStringsContext.class) .popupMenuPath("Settings...") .popupMenuGroup("R") .helpLocation(new HelpLocation("DataPlugin", "Data_Settings")) @@ -128,7 +128,7 @@ public class ViewStringsPlugin extends ProgramPlugin implements DomainObjectList .buildAndInstallLocal(provider); new ActionBuilder("Default Settings", getName()) // create pop-up menu item "Default Settings..." - .withContext(ViewStringsContext.class) + .withContext(DefinedStringsContext.class) .popupMenuPath("Default Settings...") .popupMenuGroup("R") .helpLocation(new HelpLocation("DataPlugin", "Default_Settings")) @@ -211,7 +211,6 @@ public class ViewStringsPlugin extends ProgramPlugin implements DomainObjectList } break; default: - //Msg.info(this, "Unhandled event type: " + doRecord.getEventType()); break; } } diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/strings/ViewStringsProvider.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/strings/DefinedStringsProvider.java similarity index 86% rename from Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/strings/ViewStringsProvider.java rename to Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/strings/DefinedStringsProvider.java index c9c1fdc67b..52b880ee53 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/strings/ViewStringsProvider.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/strings/DefinedStringsProvider.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. @@ -38,20 +38,20 @@ import ghidra.util.table.*; /** * Provider for the defined strings table. */ -public class ViewStringsProvider extends ComponentProviderAdapter { +public class DefinedStringsProvider extends ComponentProviderAdapter { public static final Icon ICON = new GIcon("icon.plugin.viewstrings.provider"); private GhidraThreadedTablePanel threadedTablePanel; private GhidraTableFilterPanel filterPanel; private GhidraTable table; - private ViewStringsTableModel stringModel; + private DefinedStringsTableModel stringModel; private JComponent mainPanel; private Program currentProgram; private HelpLocation helpLocation; private AtomicReference delayedShowProgramLocation = new AtomicReference<>(); - ViewStringsProvider(ViewStringsPlugin plugin) { + DefinedStringsProvider(DefinedStringsPlugin plugin) { super(plugin.getTool(), "Defined Strings", plugin.getName()); mainPanel = createWorkPanel(); setIcon(ICON); @@ -70,8 +70,8 @@ public class ViewStringsProvider extends ComponentProviderAdapter { } @Override - public ViewStringsContext getActionContext(MouseEvent event) { - return new ViewStringsContext(this, table, stringModel); + public DefinedStringsContext getActionContext(MouseEvent event) { + return new DefinedStringsContext(this, table, stringModel); } @Override @@ -79,9 +79,6 @@ public class ViewStringsProvider extends ComponentProviderAdapter { return mainPanel; } - /* - * @see ghidra.framework.docking.HelpTopic#getHelpLocation() - */ @Override public HelpLocation getHelpLocation() { return helpLocation; @@ -107,12 +104,16 @@ public class ViewStringsProvider extends ComponentProviderAdapter { private JComponent createWorkPanel() { - stringModel = new ViewStringsTableModel(tool); + stringModel = new DefinedStringsTableModel(tool); threadedTablePanel = new GhidraThreadedTablePanel<>(stringModel, 1000); table = threadedTablePanel.getTable(); table.setPreferredScrollableViewportSize(new Dimension(350, 150)); - table.getSelectionModel().addListSelectionListener(e -> notifyContextChanged()); + table.getSelectionModel().addListSelectionListener(e -> { + if (!e.getValueIsAdjusting()) { + notifyContextChanged(); + } + }); stringModel.addTableModelListener(e -> { int rowCount = stringModel.getRowCount(); @@ -148,7 +149,7 @@ public class ViewStringsProvider extends ComponentProviderAdapter { } }); TableColumn stringRepCol = table.getColumnModel() - .getColumn(ViewStringsTableModel.COLUMNS.STRING_REP_COL.ordinal()); + .getColumn(DefinedStringsTableModel.COLUMNS.STRING_REP_COL.ordinal()); stringRepCol.setCellEditor(new StringRepCellEditor()); @@ -201,23 +202,23 @@ public class ViewStringsProvider extends ComponentProviderAdapter { return table; } - public ViewStringsTableModel getModel() { + public DefinedStringsTableModel getModel() { return stringModel; } - private void doShowProgramLocation(ProgramLocation loc) { - ProgramLocation realLoc = stringModel.findEquivProgramLocation(loc); - if (realLoc != null) { - int rowIndex = stringModel.getViewIndex(realLoc); - if (rowIndex >= 0) { - table.selectRow(rowIndex); - table.scrollToSelectedRow(); - } - else { - getTool().setStatusInfo( - "String at " + realLoc.getAddress() + " is filtered out of table view", false); - } + private void doShowProgramLocation(ProgramLocation pl) { + ProgramLocation modelLocation = stringModel.findEquivProgramLocation(pl); + if (modelLocation == null) { + return; } + + int newRow = stringModel.getViewIndex(modelLocation); + if (newRow < 0) { + return; + } + + table.selectRow(newRow); + table.scrollToSelectedRow(); } public void showProgramLocation(ProgramLocation loc) { diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/strings/ViewStringsTableModel.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/strings/DefinedStringsTableModel.java similarity index 96% rename from Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/strings/ViewStringsTableModel.java rename to Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/strings/DefinedStringsTableModel.java index 83796b2e65..9627364b07 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/strings/ViewStringsTableModel.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/strings/DefinedStringsTableModel.java @@ -47,7 +47,7 @@ import ghidra.util.task.TaskMonitor; * This implementation keeps a local index of Address to row object (which are ProgramLocations) * so that DomainObjectChangedEvent events can be efficiently handled. */ -class ViewStringsTableModel extends AddressBasedTableModel { +class DefinedStringsTableModel extends AddressBasedTableModel { private Map rowsIndexedByAddress = new HashMap<>(); @@ -66,7 +66,7 @@ class ViewStringsTableModel extends AddressBasedTableModel { TRANSLATED_VALUE } - ViewStringsTableModel(PluginTool tool) { + DefinedStringsTableModel(PluginTool tool) { super("Defined String Table", tool, null, null); } @@ -136,9 +136,9 @@ class ViewStringsTableModel extends AddressBasedTableModel { } public void removeDataInstanceAt(Address addr) { - ProgramLocation progLoc = rowsIndexedByAddress.get(addr); - if (progLoc != null) { - removeObject(progLoc); + ProgramLocation pl = rowsIndexedByAddress.get(addr); + if (pl != null) { + removeObject(pl); rowsIndexedByAddress.remove(addr); } } @@ -149,17 +149,18 @@ class ViewStringsTableModel extends AddressBasedTableModel { public void addDataInstance(Program localProgram, Data data) { for (Data stringInstance : DefinedStringIterator.forDataInstance(data)) { - addObject(createIndexedStringInstanceLocation(localProgram, stringInstance)); + ProgramLocation pl = createIndexedStringInstanceLocation(localProgram, stringInstance); + addObject(pl); } } @Override public ProgramSelection getProgramSelection(int[] rows) { AddressSet set = new AddressSet(); - for (int element : rows) { - ProgramLocation progLoc = filteredData.get(element); - Data data = getProgram().getListing().getDataContaining(progLoc.getAddress()); - data = data.getComponent(progLoc.getComponentPath()); + for (int row : rows) { + ProgramLocation pl = filteredData.get(row); + Data data = getProgram().getListing().getDataContaining(pl.getAddress()); + data = data.getComponent(pl.getComponentPath()); set.addRange(data.getMinAddress(), data.getMaxAddress()); } return new ProgramSelection(set); diff --git a/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/GTable.java b/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/GTable.java index 145a0faf37..feac5590cb 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/GTable.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/GTable.java @@ -1011,6 +1011,10 @@ public class GTable extends JTable { } } + /** + * Scrolls the selected row into the view center. This call will not scroll if the selected row + * is already in the view. + */ public void scrollToSelectedRow() { Container parent = getParent(); if (!(parent instanceof JViewport viewport)) { @@ -1027,6 +1031,10 @@ public class GTable extends JTable { int row = selectedRows[0]; Rectangle cellRect = getCellRect(row, 0, true); Rectangle visibleRect = getVisibleRect(); + if (visibleRect.contains(cellRect)) { + return; + } + cellRect.x = visibleRect.x; // use the view x to prevent side scrolling cellRect.width = visibleRect.width; diff --git a/Ghidra/Test/IntegrationTest/src/screen/java/help/screenshot/ViewStringsPluginScreenShots.java b/Ghidra/Test/IntegrationTest/src/screen/java/help/screenshot/DefinedStringsPluginScreenShots.java similarity index 89% rename from Ghidra/Test/IntegrationTest/src/screen/java/help/screenshot/ViewStringsPluginScreenShots.java rename to Ghidra/Test/IntegrationTest/src/screen/java/help/screenshot/DefinedStringsPluginScreenShots.java index abe168b13d..ce3c583cfe 100644 --- a/Ghidra/Test/IntegrationTest/src/screen/java/help/screenshot/ViewStringsPluginScreenShots.java +++ b/Ghidra/Test/IntegrationTest/src/screen/java/help/screenshot/DefinedStringsPluginScreenShots.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. @@ -22,15 +22,15 @@ import javax.swing.table.TableColumn; import org.junit.Test; -import ghidra.app.plugin.core.strings.ViewStringsProvider; +import ghidra.app.plugin.core.strings.DefinedStringsProvider; import ghidra.app.services.ProgramManager; import ghidra.program.model.data.*; import ghidra.program.model.listing.Data; import ghidra.test.ToyProgramBuilder; -public class ViewStringsPluginScreenShots extends GhidraScreenShotGenerator { +public class DefinedStringsPluginScreenShots extends GhidraScreenShotGenerator { - public ViewStringsPluginScreenShots() { + public DefinedStringsPluginScreenShots() { super(); } @@ -82,14 +82,14 @@ public class ViewStringsPluginScreenShots extends GhidraScreenShotGenerator { @Test public void testDefined_String_Table() { - ViewStringsProvider provider = showProvider(ViewStringsProvider.class); + DefinedStringsProvider provider = showProvider(DefinedStringsProvider.class); TableColumn addrCol = provider.getTable().getColumnModel().getColumn(0); addrCol.setMaxWidth(200); TableColumn dataTypeCol = provider.getTable().getColumnModel().getColumn(3); dataTypeCol.setMaxWidth(200); - captureIsolatedProvider(ViewStringsProvider.class, 600, 300); + captureIsolatedProvider(DefinedStringsProvider.class, 600, 300); } }