NationalSecurityAgency / ghidra

Ghidra is a software reverse engineering (SRE) framework
https://www.nsa.gov/ghidra
Apache License 2.0
52.26k stars 5.92k forks source link

Inactive dialog "Specify the Structure's Name" upon opening #4066

Closed Wall-AF closed 1 year ago

Wall-AF commented 2 years ago

Describe the bug Inside the Structure Editor dialog, trying to create a sub-structure from a selection of fields opens the dialog "Specify the Structure's Name" without any keyboard focus only when using the keyboard to bring up the context menu and choosing Create Structure from Selection. (Using the mouse works!)

To Reproduce Steps to reproduce the behavior:

  1. Create a structure (with several fields)
  2. Select a few contiguous fields
  3. Use the keyboard to bring up the context menu and choose Create Structure from Selection
  4. Experience issue

Expected behavior The provided name (struct) should be immediately modifyable, just as it is when using the mouse.

Screenshots N/A

Attachments N/A

Environment (please complete the following information):

ghidra1 commented 2 years ago

Works fine with Linux and Java 11.

ghidra1 commented 2 years ago

I had another user here try it with a Windows 10 variant (Windows Server 2016) with Java 11 and he does not see the problem you describe. @Wall-AF Is it possible for you to try using Java 11 and see if you can reproduce the problem.

ghidra1 commented 2 years ago

Re-opened pending your feedback. Developer was thinking of when you first requested support for the Shift-F10 (right mouse-click) support (#2811)

Wall-AF commented 2 years ago

I have just tried it using Java 11.0.2 with the same result (i.e. no focus and must use mouse (or mousekeys) to get focus, using Alt+Tab doesn't refocus either). FYI, I'm running Ghidra from within eclipse.

dragonmacher commented 2 years ago

When in this state, press the secret focus reporting key binding combination: Ctrl-Alt-Shift-F2

This will dump a summary of focus info.

Wall-AF commented 2 years ago

When in this state, press the secret focus reporting key binding combination: Ctrl-Alt-Shift-F2

This will dump a summary of focus info.

Mmm. No success here. Can't see any dump anywhere!

dragonmacher commented 2 years ago

Hmm... It should go to the console / log file. However, there may be a chicken / egg problem here. It's possible that Java is not detecting your keyboard input at all, rendering that command useless.

Wall-AF commented 2 years ago

I use StickyKeys to place a hold on the modifier keys (Alt, Ctrl, Shift & Win).

Wall-AF commented 1 year ago

@dragonmacher @ghidra1 any progress???

dragonmacher commented 1 year ago

I still cannot reproduce the issue. This makes my normal troubleshooting ineffective. I have some things I could try to debug the issue, but not way of knowing the effect.

Do you run from a distribution of Ghidra, or from a pull of the repo master branch? If running from our repo, then you have the ability to try code changes. If that is the case, then I can offer code snippets for testing.

Wall-AF commented 1 year ago

I'm running it from inside Eclipse.

dragonmacher commented 1 year ago

Ok. Are you able to change Ghidra source code, such as the ghidra.app.plugin.core.decompile.actions.FillOutStructureCmd and then run with those changes?

Wall-AF commented 1 year ago

I can, but which changes?

dragonmacher commented 1 year ago

I can, but which changes?

Inside of FillOutStructureCmd, change this:

public CreateInternalStructureAction(StructureEditorProvider provider) {
        super(provider, EDIT_ACTION_PREFIX + ACTION_NAME, GROUP_NAME, POPUP_PATH, null, ICON);
        setDescription(DESCRIPTION);
        adjustEnablement();
    }

    @Override
    public void actionPerformed(ActionContext context) {
        int[] selectedComponentRows = model.getSelectedComponentRows();
        boolean hasComponentSelection = model.hasComponentSelection();

to this:

public CreateInternalStructureAction(StructureEditorProvider provider) {
        super(provider, EDIT_ACTION_PREFIX + ACTION_NAME, GROUP_NAME, POPUP_PATH, null, ICON);
        setDescription(DESCRIPTION);
        adjustEnablement();
    }

    @Override
    public void actionPerformed(ActionContext context) {
        Swing.runLater(this::doActionPerformed);
    }

    public void doActionPerformed() {
        int[] selectedComponentRows = model.getSelectedComponentRows();
        boolean hasComponentSelection = model.hasComponentSelection();

Basically, this just adds one new method to delegate to the original code 'later'.

Wall-AF commented 1 year ago

Inside of FillOutStructureCmd, change this:

public CreateInternalStructureAction(StructureEditorProvider provider) {
      super(provider, EDIT_ACTION_PREFIX + ACTION_NAME, GROUP_NAME, POPUP_PATH, null, ICON);
      setDescription(DESCRIPTION);
      adjustEnablement();
  }

  @Override
  public void actionPerformed(ActionContext context) {
      int[] selectedComponentRows = model.getSelectedComponentRows();
      boolean hasComponentSelection = model.hasComponentSelection();

only exists in CreateInternalStructureAction.java, and I changed it there, but it made no difference.

To reproduce (on a Windows box):

  1. Open the Structure Editor
  2. Highlight one or more components
  3. Use the Shift+F10 equivalent key to bring up the context menu
  4. Use the arrow keys to highlight the Create Structure From Selection option
  5. Press <Enter> to choose the option (do not use the mouse)
dragonmacher commented 1 year ago

only exists in CreateInternalStructureAction.java, and I changed it there, but it made no difference.

Yes, I'm sorry, I had the wrong class name

Ok. You can undo that change. Something else to try. Inside of DockingDialog, change this:

    private Runnable requestFocusRunnable = () -> {
        if (focusComponent != null) {
            focusComponent.requestFocus();
            hasBeenFocused = true;
        }
    };

to this:

    private Runnable requestFocusRunnable = () -> {

        Msg.debug(this, "calling request focus on: " + focusComponent);

        if (focusComponent != null) {
            focusComponent.requestFocusInWindow();
            hasBeenFocused = true;
        }
    };

Note the added print statement and the change to requestFocusInWindow();

(This likely will not fix the issue, but it may help provide more info.)

Wall-AF commented 1 year ago

Mmm, interesting! The debug message does not appear when just using the <Enter> key (step 5 above), but does if using the mouse, suggesting that method private Runnable requestFocusRunnable isn't even being called.

Wall-AF commented 1 year ago

Upon further investigation, when using the mouse, the windowActivated override gets called twice whereas using the keyboard it only gets called once! Only on the second occasion does component.getFocusComponent() return a non-null response.

With this version:

@Override
public void windowActivated(WindowEvent e) {
    ghidra.util.Msg.debug(this, "calling windowActivated on: " + focusComponent
            + " :  hasBeenFocused=" + hasBeenFocused);
    if (!hasBeenFocused) {
        Component newFocusComponent = component.getFocusComponent();
        ghidra.util.Msg.debug(this, "calling windowActivated, component= " + newFocusComponent);
        if (newFocusComponent != null) {
            focusComponent = newFocusComponent;
            SwingUtilities.invokeLater(requestFocusRunnable);
        }
    }
}

Output when using <Enter> key:

DEBUG calling windowActivated on: docking.widgets.dialogs.InputDialog$MyTextField[input.dialog.text.field.0,123,5,166x20,invalid,layout=javax.swing.plaf.basic.BasicTextUI$UpdateHandler,alignmentX=0.0,alignmentY=0.0,border=com.sun.java.swing.plaf.windows.XPStyle$XPFillBorder@622ac4cc,flags=296,maximumSize=,minimumSize=,preferredSize=,caretColor=#000000,disabledTextColor=#6d6d6d,editable=true,margin=javax.swing.plaf.InsetsUIResource[top=2,left=2,bottom=2,right=2],selectedTextColor=#ffffff,selectionColor=#0078d7,columns=20,columnWidth=8,command=,horizontalAlignment=LEADING] :  hasBeenFocused=false   (DockingDialog.java:204) 
DEBUG calling windowActivated, component= null   (DockingDialog.java:208) 

Output when using the mouse:

DEBUG calling windowActivated on: docking.widgets.dialogs.InputDialog$MyTextField[input.dialog.text.field.0,123,5,166x20,invalid,layout=javax.swing.plaf.basic.BasicTextUI$UpdateHandler,alignmentX=0.0,alignmentY=0.0,border=com.sun.java.swing.plaf.windows.XPStyle$XPFillBorder@622ac4cc,flags=296,maximumSize=,minimumSize=,preferredSize=,caretColor=#000000,disabledTextColor=#6d6d6d,editable=true,margin=javax.swing.plaf.InsetsUIResource[top=2,left=2,bottom=2,right=2],selectedTextColor=#ffffff,selectionColor=#0078d7,columns=20,columnWidth=8,command=,horizontalAlignment=LEADING] :  hasBeenFocused=false   (DockingDialog.java:204) 
DEBUG calling windowActivated, component= null   (DockingDialog.java:208) 
DEBUG calling windowActivated on: docking.widgets.dialogs.InputDialog$MyTextField[input.dialog.text.field.0,123,5,166x20,invalid,layout=javax.swing.plaf.basic.BasicTextUI$UpdateHandler,alignmentX=0.0,alignmentY=0.0,border=com.sun.java.swing.plaf.windows.XPStyle$XPFillBorder@622ac4cc,flags=296,maximumSize=,minimumSize=,preferredSize=,caretColor=#000000,disabledTextColor=#6d6d6d,editable=true,margin=javax.swing.plaf.InsetsUIResource[top=2,left=2,bottom=2,right=2],selectedTextColor=#ffffff,selectionColor=#0078d7,columns=20,columnWidth=8,command=,horizontalAlignment=LEADING] :  hasBeenFocused=false   (DockingDialog.java:204) 
DEBUG calling windowActivated, component= docking.widgets.dialogs.InputDialog$MyTextField[input.dialog.text.field.0,123,5,166x20,layout=javax.swing.plaf.basic.BasicTextUI$UpdateHandler,alignmentX=0.0,alignmentY=0.0,border=com.sun.java.swing.plaf.windows.XPStyle$XPFillBorder@622ac4cc,flags=296,maximumSize=,minimumSize=,preferredSize=,caretColor=#000000,disabledTextColor=#6d6d6d,editable=true,margin=javax.swing.plaf.InsetsUIResource[top=2,left=2,bottom=2,right=2],selectedTextColor=#ffffff,selectionColor=#0078d7,columns=20,columnWidth=8,command=,horizontalAlignment=LEADING]   (DockingDialog.java:208) 
DEBUG calling request focus on: docking.widgets.dialogs.InputDialog$MyTextField[input.dialog.text.field.0,123,5,166x20,layout=javax.swing.plaf.basic.BasicTextUI$UpdateHandler,alignmentX=0.0,alignmentY=0.0,border=com.sun.java.swing.plaf.windows.XPStyle$XPFillBorder@622ac4cc,flags=296,maximumSize=,minimumSize=,preferredSize=,caretColor=#000000,disabledTextColor=#6d6d6d,editable=true,margin=javax.swing.plaf.InsetsUIResource[top=2,left=2,bottom=2,right=2],selectedTextColor=#ffffff,selectionColor=#0078d7,columns=20,columnWidth=8,command=,horizontalAlignment=LEADING]   (DockingDialog.java:48) 
dragonmacher commented 1 year ago

Now let's see what Java thinks the focus state of the system is after everything has settled down.

Update the requestFocusRunnable to look like this:

    private Runnable requestFocusRunnable = () -> {

        Msg.debug(this, "\tcalling request focus on: " + focusComponent);

        if (focusComponent != null) {
            boolean didFocus = focusComponent.requestFocusInWindow();

            Msg.debug(this, "\tdid focus?: " + didFocus);

            GTimer.scheduleRunnable(2000, () -> {

                KeyboardFocusManager kfm = KeyboardFocusManager.getCurrentKeyboardFocusManager();
                Window window = kfm.getActiveWindow();
                Window focusedWindow = kfm.getFocusedWindow();
                Component focusOwner = kfm.getFocusOwner();
                Msg.debug(this, "active window: " + window + "\n\tfocused window: " +
                    focusedWindow + "\n\tfocus owner: " + focusOwner);
            });

            hasBeenFocused = true;
        }
    };

This is using a 2 second timer to show the focus info.

Wall-AF commented 1 year ago

That makes no difference because the SwingUtilities.invokeLater(requestFocusRunnable); isn't called from: https://github.com/NationalSecurityAgency/ghidra/blob/e5a8f26347e480292580f252806bda9a4e49b814/Ghidra/Framework/Docking/src/main/java/docking/DockingDialog.java#L200-L212 As I tried to explain https://github.com/NationalSecurityAgency/ghidra/issues/4066#issuecomment-1401730084.

dragonmacher commented 1 year ago

The use of the timer was to allow any buffered events to settle down. (Sometimes system events cause more events to be buffered and execute at a later time, which an invokeLater() would not handle.

Also, the timer's use was not to fix your issue, but to provide diagnostics relating to what Java considers the focused element.

Wall-AF commented 1 year ago

I understood that, but that timer doesn't even get started.

dragonmacher commented 1 year ago

I see. I missed that earlier. In that case, I am curious to know the value of component when the null value is returned (thus preventing the notification). This would look something like this:

@Override
public void windowActivated(WindowEvent e) {
    ghidra.util.Msg.debug(this, "calling windowActivated on: " + focusComponent
            + " :  hasBeenFocused=" + hasBeenFocused);
    if (!hasBeenFocused) {
        Component newFocusComponent = component.getFocusComponent();
        ghidra.util.Msg.debug(this, "calling windowActivated, component= " + component + "; focused comp="+newFocusComponent);
        if (newFocusComponent != null) {
            focusComponent = newFocusComponent;
            SwingUtilities.invokeLater(requestFocusRunnable);
        }
    }
}
Wall-AF commented 1 year ago
DEBUG calling windowActivated on: null :  hasBeenFocused=false   (DockingDialog.java:228) 
DEBUG calling windowActivated, component= Editor: Create Structure From Selection; focused comp=null   (DockingDialog.java:232) 
dragonmacher commented 1 year ago

The issue appears to be that Java is not correctly generating a window activated event for the new "Specify the Structure's Name" dialog. On my system, I am seeing the correct event, as you do when using the mouse.

It appears as though you have not tried Java 17. It is unlikely that will fix the issue, but something worth trying.

There is a more invasive way to track the exchange / handshake for window activated and focused events. That would require debug information in a different class, with much more output to interpret. Let me know if you are still interested in debugging this further.

Wall-AF commented 1 year ago

I'm using:

Windows 11 Pro (64-bit) version 10.0.22621.819

D:\GIT_Sources\Ghidra\ghidra>java -version
java version "17.0.3.1" 2022-04-22 LTS
Java(TM) SE Runtime Environment (build 17.0.3.1+2-LTS-6)
Java HotSpot(TM) 64-Bit Server VM (build 17.0.3.1+2-LTS-6, mixed mode, sharing)
dragonmacher commented 1 year ago

If you'd like to keep debugging, the next step I would take is to see the series of events given to us by Java. Firstly, are they different in your 2 use cases? Secondly, if they are the same from Java, is Ghidra doing something to drop the problem case?

To debug this, I use DockingWindowManager.propertyChange(). This is the method we use to get notified of all focus changes. We attempt to keep our system in sync with Java in this method. It is quite complicated. But, you can print the events received and the windows and components involved. This will allow you to see the actors involved in both use cases.

Wall-AF commented 1 year ago

Where and how? Are you sure you can't replicate???

dragonmacher commented 1 year ago

We have tried on multiple OSes virtual and native. It works consistently for us.

The debugging I suggested would just be more print statements, similar to what we have already done. The location is the docking.DockingWindowManager in the method propertyChange(). I understand that this is quite annoying. There is nothing else I can think of to troubleshoot this on our end.

Wall-AF commented 1 year ago

Also, clearly there's a difference as shown in the output of https://github.com/NationalSecurityAgency/ghidra/issues/4066#issuecomment-1401730084 where the windowActivated is called once when using the keyboard to select the option and twice when using the mouse.

dragonmacher commented 1 year ago

Yes. I believe Java is doing something wrong here. Based on the fact that you have tried multiple Java versions, the issue seems not to be a Java bug. My guess would be that you have some configuration causing the issue (some sort of combination of OS settings or installed software). If there is something Ghidra is doing to break Java, then we can fix that. If not, this may be something that you have to fix external to Java / Ghidra.

Unfortunately, diagnosing focus issues is difficult and even more so if we cannot reproduce the issue.

Wall-AF commented 1 year ago

Added several Msg.debug to propertyChange and in the mouse version 3 additional permanentFocusOwner property messages appear that have the InputDialog as the newValue.

FYI, prior to those 3 messages, 2 others from windowActivated occurred (and that is what's setting the focus correctly). How do you determine what triggered those?

dragonmacher commented 1 year ago

The events are being generated from within the jvm. At some point that becomes a black box.

I wonder if this has something to do with the task dialog that is appearing. That is the "Create Structure From Selection" dialog. For me, that is behind the newly opened input dialog. There may be some sort of handshake / parenting problem with when we are showing that dialog. I will see if I can change that timing and let you know if there is something you can try in that regard.

Do those windows appear in the same order in both scenarios? The initial dialog is behind the input dialog.

Wall-AF commented 1 year ago

Yes, they do. Interesting though, although the textbox containing the default text struct doesn't have the focus and no amount of tabbing alters that, I can press Alt+Space to get the System Menu of the InputDialog!

dragonmacher commented 1 year ago

I am not seeing the "Create Structure From Selection" appear in the propertyChange() focus events. Do you see that dialog in these events?

Wall-AF commented 1 year ago

These messages are common to keyboard or mouse activation:

DEBUG propertyChange evt=java.beans.PropertyChangeEvent[propertyName=permanentFocusOwner; oldValue=ghidra.app.plugin.core.compositeeditor.CompositeEditorPanel$CompositeTable[,0,0,755x1496,alignmentX=0.0,alignmentY=0.0,border=,flags=234881384,maximumSize=,minimumSize=,preferredSize=,autoCreateColumnsFromModel=true,autoResizeMode=AUTO_RESIZE_LAST_COLUMN,cellSelectionEnabled=false,editingColumn=-1,editingRow=-1,gridColor=#808080,preferredViewportSize=java.awt.Dimension[width=610,height=250],rowHeight=17,rowMargin=0,rowSelectionAllowed=true,selectionBackground=#0078d7,selectionForeground=#ffffff,showHorizontalLines=false,showVerticalLines=false]; newValue=null; propagationId=null; source=java.awt.DefaultKeyboardFocusManager@1203515a]   (DockingWindowManager.java:1491) 
DEBUG propertyChange evt=java.beans.PropertyChangeEvent[propertyName=permanentFocusOwner; oldValue=ghidra.app.plugin.core.compositeeditor.CompositeEditorPanel$CompositeTable[,0,0,755x1496,alignmentX=0.0,alignmentY=0.0,border=,flags=234881384,maximumSize=,minimumSize=,preferredSize=,autoCreateColumnsFromModel=true,autoResizeMode=AUTO_RESIZE_LAST_COLUMN,cellSelectionEnabled=false,editingColumn=-1,editingRow=-1,gridColor=#808080,preferredViewportSize=java.awt.Dimension[width=610,height=250],rowHeight=17,rowMargin=0,rowSelectionAllowed=true,selectionBackground=#0078d7,selectionForeground=#ffffff,showHorizontalLines=false,showVerticalLines=false]; newValue=null; propagationId=null; source=java.awt.DefaultKeyboardFocusManager@1203515a]   (DockingWindowManager.java:1491) 
DEBUG nfc=null   (DockingWindowManager.java:1501) 
DEBUG propertyChange evt=java.beans.PropertyChangeEvent[propertyName=permanentFocusOwner; oldValue=ghidra.app.plugin.core.compositeeditor.CompositeEditorPanel$CompositeTable[,0,0,755x1496,alignmentX=0.0,alignmentY=0.0,border=,flags=234881384,maximumSize=,minimumSize=,preferredSize=,autoCreateColumnsFromModel=true,autoResizeMode=AUTO_RESIZE_LAST_COLUMN,cellSelectionEnabled=false,editingColumn=-1,editingRow=-1,gridColor=#808080,preferredViewportSize=java.awt.Dimension[width=610,height=250],rowHeight=17,rowMargin=0,rowSelectionAllowed=true,selectionBackground=#0078d7,selectionForeground=#ffffff,showHorizontalLines=false,showVerticalLines=false]; newValue=null; propagationId=null; source=java.awt.DefaultKeyboardFocusManager@1203515a]   (DockingWindowManager.java:1491) 
DEBUG propertyChange evt=java.beans.PropertyChangeEvent[propertyName=permanentFocusOwner; oldValue=null; newValue=ghidra.app.plugin.core.compositeeditor.CompositeEditorPanel$CompositeTable[,0,0,755x1496,alignmentX=0.0,alignmentY=0.0,border=,flags=234881384,maximumSize=,minimumSize=,preferredSize=,autoCreateColumnsFromModel=true,autoResizeMode=AUTO_RESIZE_LAST_COLUMN,cellSelectionEnabled=false,editingColumn=-1,editingRow=-1,gridColor=#808080,preferredViewportSize=java.awt.Dimension[width=610,height=250],rowHeight=17,rowMargin=0,rowSelectionAllowed=true,selectionBackground=#0078d7,selectionForeground=#ffffff,showHorizontalLines=false,showVerticalLines=false]; propagationId=null; source=java.awt.DefaultKeyboardFocusManager@1203515a]   (DockingWindowManager.java:1491) 
DEBUG propertyChange evt=java.beans.PropertyChangeEvent[propertyName=permanentFocusOwner; oldValue=null; newValue=ghidra.app.plugin.core.compositeeditor.CompositeEditorPanel$CompositeTable[,0,0,755x1496,alignmentX=0.0,alignmentY=0.0,border=,flags=234881384,maximumSize=,minimumSize=,preferredSize=,autoCreateColumnsFromModel=true,autoResizeMode=AUTO_RESIZE_LAST_COLUMN,cellSelectionEnabled=false,editingColumn=-1,editingRow=-1,gridColor=#808080,preferredViewportSize=java.awt.Dimension[width=610,height=250],rowHeight=17,rowMargin=0,rowSelectionAllowed=true,selectionBackground=#0078d7,selectionForeground=#ffffff,showHorizontalLines=false,showVerticalLines=false]; propagationId=null; source=java.awt.DefaultKeyboardFocusManager@1203515a]   (DockingWindowManager.java:1491) 
DEBUG nfc=ghidra.app.plugin.core.compositeeditor.CompositeEditorPanel$CompositeTable[,0,0,755x1496,alignmentX=0.0,alignmentY=0.0,border=,flags=234881384,maximumSize=,minimumSize=,preferredSize=,autoCreateColumnsFromModel=true,autoResizeMode=AUTO_RESIZE_LAST_COLUMN,cellSelectionEnabled=false,editingColumn=-1,editingRow=-1,gridColor=#808080,preferredViewportSize=java.awt.Dimension[width=610,height=250],rowHeight=17,rowMargin=0,rowSelectionAllowed=true,selectionBackground=#0078d7,selectionForeground=#ffffff,showHorizontalLines=false,showVerticalLines=false]   (DockingWindowManager.java:1501) 
DEBUG dc=Structure Editor - Copy_1_of_DgnLanguageModel_0xc2_t_beforechangingfld0x26 (DRAGON)   (DockingWindowManager.java:1508) 
DEBUG propertyChange evt=java.beans.PropertyChangeEvent[propertyName=permanentFocusOwner; oldValue=null; newValue=ghidra.app.plugin.core.compositeeditor.CompositeEditorPanel$CompositeTable[,0,0,755x1496,alignmentX=0.0,alignmentY=0.0,border=,flags=234881384,maximumSize=,minimumSize=,preferredSize=,autoCreateColumnsFromModel=true,autoResizeMode=AUTO_RESIZE_LAST_COLUMN,cellSelectionEnabled=false,editingColumn=-1,editingRow=-1,gridColor=#808080,preferredViewportSize=java.awt.Dimension[width=610,height=250],rowHeight=17,rowMargin=0,rowSelectionAllowed=true,selectionBackground=#0078d7,selectionForeground=#ffffff,showHorizontalLines=false,showVerticalLines=false]; propagationId=null; source=java.awt.DefaultKeyboardFocusManager@1203515a]   (DockingWindowManager.java:1491) 
DEBUG calling windowActivated on: docking.widgets.dialogs.InputDialog$MyTextField[input.dialog.text.field.0,123,5,166x20,invalid,layout=javax.swing.plaf.basic.BasicTextUI$UpdateHandler,alignmentX=0.0,alignmentY=0.0,border=com.sun.java.swing.plaf.windows.XPStyle$XPFillBorder@43005f1f,flags=296,maximumSize=,minimumSize=,preferredSize=,caretColor=#000000,disabledTextColor=#6d6d6d,editable=true,margin=javax.swing.plaf.InsetsUIResource[top=2,left=2,bottom=2,right=2],selectedTextColor=#ffffff,selectionColor=#0078d7,columns=20,columnWidth=8,command=,horizontalAlignment=LEADING] :  hasBeenFocused=false   (DockingDialog.java:228) 
DEBUG calling windowActivated, component= Editor: Create Structure From Selection; focused comp=null   (DockingDialog.java:232) 

These additional messages appear when using the mouse:

DEBUG calling windowActivated on: docking.widgets.dialogs.InputDialog$MyTextField[input.dialog.text.field.0,123,5,166x20,invalid,layout=javax.swing.plaf.basic.BasicTextUI$UpdateHandler,alignmentX=0.0,alignmentY=0.0,border=com.sun.java.swing.plaf.windows.XPStyle$XPFillBorder@43005f1f,flags=296,maximumSize=,minimumSize=,preferredSize=,caretColor=#000000,disabledTextColor=#6d6d6d,editable=true,margin=javax.swing.plaf.InsetsUIResource[top=2,left=2,bottom=2,right=2],selectedTextColor=#ffffff,selectionColor=#0078d7,columns=20,columnWidth=8,command=,horizontalAlignment=LEADING] :  hasBeenFocused=false   (DockingDialog.java:228) 
DEBUG calling windowActivated, component= Specify the Structure's Name; focused comp=docking.widgets.dialogs.InputDialog$MyTextField[input.dialog.text.field.0,123,5,166x20,layout=javax.swing.plaf.basic.BasicTextUI$UpdateHandler,alignmentX=0.0,alignmentY=0.0,border=com.sun.java.swing.plaf.windows.XPStyle$XPFillBorder@43005f1f,flags=296,maximumSize=,minimumSize=,preferredSize=,caretColor=#000000,disabledTextColor=#6d6d6d,editable=true,margin=javax.swing.plaf.InsetsUIResource[top=2,left=2,bottom=2,right=2],selectedTextColor=#ffffff,selectionColor=#0078d7,columns=20,columnWidth=8,command=,horizontalAlignment=LEADING]   (DockingDialog.java:232) 
DEBUG   calling request focus on: docking.widgets.dialogs.InputDialog$MyTextField[input.dialog.text.field.0,123,5,166x20,layout=javax.swing.plaf.basic.BasicTextUI$UpdateHandler,alignmentX=0.0,alignmentY=0.0,border=com.sun.java.swing.plaf.windows.XPStyle$XPFillBorder@43005f1f,flags=296,maximumSize=,minimumSize=,preferredSize=,caretColor=#000000,disabledTextColor=#6d6d6d,editable=true,margin=javax.swing.plaf.InsetsUIResource[top=2,left=2,bottom=2,right=2],selectedTextColor=#ffffff,selectionColor=#0078d7,columns=20,columnWidth=8,command=,horizontalAlignment=LEADING]   (DockingDialog.java:51) 
DEBUG   did focus?: true   (DockingDialog.java:56) 
DEBUG propertyChange evt=java.beans.PropertyChangeEvent[propertyName=permanentFocusOwner; oldValue=ghidra.app.plugin.core.compositeeditor.CompositeEditorPanel$CompositeTable[,0,0,755x1496,alignmentX=0.0,alignmentY=0.0,border=,flags=234881384,maximumSize=,minimumSize=,preferredSize=,autoCreateColumnsFromModel=true,autoResizeMode=AUTO_RESIZE_LAST_COLUMN,cellSelectionEnabled=false,editingColumn=-1,editingRow=-1,gridColor=#808080,preferredViewportSize=java.awt.Dimension[width=610,height=250],rowHeight=17,rowMargin=0,rowSelectionAllowed=true,selectionBackground=#0078d7,selectionForeground=#ffffff,showHorizontalLines=false,showVerticalLines=false]; newValue=docking.widgets.dialogs.InputDialog$MyTextField[input.dialog.text.field.0,123,5,166x20,layout=javax.swing.plaf.basic.BasicTextUI$UpdateHandler,alignmentX=0.0,alignmentY=0.0,border=com.sun.java.swing.plaf.windows.XPStyle$XPFillBorder@43005f1f,flags=296,maximumSize=,minimumSize=,preferredSize=,caretColor=#000000,disabledTextColor=#6d6d6d,editable=true,margin=javax.swing.plaf.InsetsUIResource[top=2,left=2,bottom=2,right=2],selectedTextColor=#ffffff,selectionColor=#0078d7,columns=20,columnWidth=8,command=,horizontalAlignment=LEADING]; propagationId=null; source=java.awt.DefaultKeyboardFocusManager@1203515a]   (DockingWindowManager.java:1491) 
DEBUG propertyChange evt=java.beans.PropertyChangeEvent[propertyName=permanentFocusOwner; oldValue=ghidra.app.plugin.core.compositeeditor.CompositeEditorPanel$CompositeTable[,0,0,755x1496,alignmentX=0.0,alignmentY=0.0,border=,flags=234881384,maximumSize=,minimumSize=,preferredSize=,autoCreateColumnsFromModel=true,autoResizeMode=AUTO_RESIZE_LAST_COLUMN,cellSelectionEnabled=false,editingColumn=-1,editingRow=-1,gridColor=#808080,preferredViewportSize=java.awt.Dimension[width=610,height=250],rowHeight=17,rowMargin=0,rowSelectionAllowed=true,selectionBackground=#0078d7,selectionForeground=#ffffff,showHorizontalLines=false,showVerticalLines=false]; newValue=docking.widgets.dialogs.InputDialog$MyTextField[input.dialog.text.field.0,123,5,166x20,layout=javax.swing.plaf.basic.BasicTextUI$UpdateHandler,alignmentX=0.0,alignmentY=0.0,border=com.sun.java.swing.plaf.windows.XPStyle$XPFillBorder@43005f1f,flags=296,maximumSize=,minimumSize=,preferredSize=,caretColor=#000000,disabledTextColor=#6d6d6d,editable=true,margin=javax.swing.plaf.InsetsUIResource[top=2,left=2,bottom=2,right=2],selectedTextColor=#ffffff,selectionColor=#0078d7,columns=20,columnWidth=8,command=,horizontalAlignment=LEADING]; propagationId=null; source=java.awt.DefaultKeyboardFocusManager@1203515a]   (DockingWindowManager.java:1491) 
DEBUG propertyChange evt=java.beans.PropertyChangeEvent[propertyName=permanentFocusOwner; oldValue=ghidra.app.plugin.core.compositeeditor.CompositeEditorPanel$CompositeTable[,0,0,755x1496,alignmentX=0.0,alignmentY=0.0,border=,flags=234881384,maximumSize=,minimumSize=,preferredSize=,autoCreateColumnsFromModel=true,autoResizeMode=AUTO_RESIZE_LAST_COLUMN,cellSelectionEnabled=false,editingColumn=-1,editingRow=-1,gridColor=#808080,preferredViewportSize=java.awt.Dimension[width=610,height=250],rowHeight=17,rowMargin=0,rowSelectionAllowed=true,selectionBackground=#0078d7,selectionForeground=#ffffff,showHorizontalLines=false,showVerticalLines=false]; newValue=docking.widgets.dialogs.InputDialog$MyTextField[input.dialog.text.field.0,123,5,166x20,layout=javax.swing.plaf.basic.BasicTextUI$UpdateHandler,alignmentX=0.0,alignmentY=0.0,border=com.sun.java.swing.plaf.windows.XPStyle$XPFillBorder@43005f1f,flags=296,maximumSize=,minimumSize=,preferredSize=,caretColor=#000000,disabledTextColor=#6d6d6d,editable=true,margin=javax.swing.plaf.InsetsUIResource[top=2,left=2,bottom=2,right=2],selectedTextColor=#ffffff,selectionColor=#0078d7,columns=20,columnWidth=8,command=,horizontalAlignment=LEADING]; propagationId=null; source=java.awt.DefaultKeyboardFocusManager@1203515a]   (DockingWindowManager.java:1491) 
DEBUG active window: Specify the Structure's Name
    focused window: Specify the Structure's Name
    focus owner: docking.widgets.dialogs.InputDialog$MyTextField[input.dialog.text.field.0,123,5,166x20,layout=javax.swing.plaf.basic.BasicTextUI$UpdateHandler,alignmentX=0.0,alignmentY=0.0,border=com.sun.java.swing.plaf.windows.XPStyle$XPFillBorder@43005f1f,flags=296,maximumSize=,minimumSize=,preferredSize=,caretColor=#000000,disabledTextColor=#6d6d6d,editable=true,margin=javax.swing.plaf.InsetsUIResource[top=2,left=2,bottom=2,right=2],selectedTextColor=#ffffff,selectionColor=#0078d7,columns=20,columnWidth=8,command=,horizontalAlignment=LEADING]   (DockingDialog.java:64) 
Wall-AF commented 1 year ago

By replacing: https://github.com/NationalSecurityAgency/ghidra/blob/e5a8f26347e480292580f252806bda9a4e49b814/Ghidra/Framework/Docking/src/main/java/docking/DockingWindowManager.java#L1772-L1777 in DockingWindowManager.doShowDialog(...) with Swing.runIfSwingOrRunLater(r); it worked!

However, what's the consequence??

dragonmacher commented 1 year ago

However, what's the consequence??

That is some of the most complicated and sensitive code in the system. We keep an uneasy truce with the way that code handles modal dialogs.

This is more information to support the idea that the modal dialog timing with the input dialog is not working correctly for you. I'll keep chewing on this.

dragonmacher commented 1 year ago

I starting seeing odd behavior related to focus. After trying many things, I needed to update the action that shows the context menu in order for my system to start behaving again.

I'm curious is my change has any effect on your original issue. When you get a chance, edit ShowContextMenuAction to use a Swing.runLater(), like so:

        if (focusOwner != null) {
            Swing.runLater(() -> {
                DockingWindowManager.showContextMenu(focusOwner);
            });
        }
Wall-AF commented 1 year ago

Darn! No effect!

Wall-AF commented 1 year ago

Just a reminder, it's the difference between pressing <Enter> on the keyboard and clicking with the mouse to select the menu item that's causing this anomaly, not displaying the context menu initially.

dragonmacher commented 1 year ago

I am aware of the distinction. I ran into a separate issue where my menu focus stopped working for some reason.

At this point, I am fairly confident that your particular issue has to do with the Java handshake between the various windows that get shown during window activation. My guess is that by clicking the menu item, there is a focus change in Java that keeps the handshake consistent.

To help rule out the task code vs the handshake issue, try this change inside of CreateInternalStructureAction:

        private void doCreate(TaskMonitor monitor) {

        InputDialog dialog = new InputDialog("Title", "Text: ");
        DockingWindowManager.showDialog(dialog);
        String text = dialog.getValue();
        Msg.debug(this, "Input dialog text: " + text);

        try {
            ((StructureEditorModel) model).createInternalStructure(monitor);
        }
        catch (CancelledException e) {
            // user cancelled
        }
        catch (UsrException e) {
            model.setStatus(e.getMessage(), true);
        }
    }

I'd like to know if the new dialog that appears has focus. Also, after that dialog is shown, do any follow-on dialogs get focused correctly?

Wall-AF commented 1 year ago

The new dialog suffers the exact same issue. However once closed, the next one's do have the correct focus.

dragonmacher commented 1 year ago

Next test, inside of actionPerformed(), replace this:

    if (maxRow < numComponents) {
        TaskLauncher.launchModal(getName(), this::doCreate);
    }

with this:

    if (maxRow < numComponents) {

        InputDialog dialog = new InputDialog("Title", "Text: ");
        DockingWindowManager.showDialog(dialog);
        String text = dialog.getValue();
        Msg.debug(this, "Input dialog text: " + text);

        TaskLauncher.launchModal(getName(), this::doCreate);
    }

As before, I'd like to know the focus behavior with this dialog and the follow-on dialogs.

Wall-AF commented 1 year ago

This time, your new Text dialog is displayed with the focus correct. Unfortunately, subsequent dialogs fail!

dragonmacher commented 1 year ago

Thanks for the help with this. To solve this particular issue, I plan on pulling the prompts out of the task. This should allow you to interact as needed, with the long-running work being done in a task without the need for user interaction.

Wall-AF commented 1 year ago

I really appreciate your persistence in looikng into this. Thank you very much.