Opened 19 months ago

Closed 11 months ago

Last modified 8 months ago

#18025 closed defect (fixed)

[OSX] wxTextCtrl not triggering default button on Enter for non-wxTE_PROCESS_ENTER

Reported by: tessman Owned by:
Priority: normal Milestone:
Component: wxOSX Version: dev-latest
Keywords: wxTextCtrl, wxTE_PROCESS_ENTER Cc: ktessman@…, ewasylishen@…
Blocked By: Blocking:
Patch: no

Description

Without wxTE_PROCESS_ENTER, when Enter is pressed in a wxTextCtrl where the containing dialog has a default button set by SetDefault(), it should process that button event.

This is the behavior on all platforms except for wxCocoa. (It also works properly on wxMac 2.8.)

It looks like there's code in textctrl.mm to handle this in -control:textView:doCommandBySelector for wxNSSecureTextField (for passwords) but not, as I follow it, for a plain wxNSTextField. That said, I tried tracing the event handling for an Enter key event and I'm not sure it gets anywhere near the control.

There's a little more discussion/illustration here:

https://forums.wxwidgets.org/viewtopic.php?f=23&t=44131&p=181085

Change History (9)

comment:1 Changed 19 months ago by tessman

  • Cc ktessman@… added

Here's a (temporary) fix I put together, modifying textctrl_osx.cpp:

void wxTextCtrl::OnKeyDown(wxKeyEvent& event)
{
    if ( event.GetModifiers() == wxMOD_CONTROL )
    {
        switch( event.GetKeyCode() )
        {
            case 'A':
                SelectAll();
                return;
            case 'C':
                if ( CanCopy() )
                    Copy() ;
                return;
            case 'V':
                if ( CanPaste() )
                    Paste() ;
                return;
            case 'X':
                if ( CanCut() )
                    Cut() ;
                return;
            default:
                break;
        }
    }

//HACK:
// Adapted from OnChar, below, since we don't seem to ever get there
if ((event.GetKeyCode() == WXK_RETURN || event.GetKeyCode() == WXK_NUMPAD_ENTER) && event.GetModifiers() == 0)
{
	if ( !(m_windowStyle & wxTE_MULTILINE) )
	{
		wxTopLevelWindow *tlw = wxDynamicCast(wxGetTopLevelParent(this), wxTopLevelWindow);
		if ( tlw && tlw->GetDefaultItem() )
		{
			wxButton *def = wxDynamicCast(tlw->GetDefaultItem(), wxButton);
			if ( def && def->IsEnabled() )
			{
				wxCommandEvent event(wxEVT_BUTTON, def->GetId() );
				event.SetEventObject(def);
				def->Command(event);
				
				return ;
			}
		}
	}
}

    // no, we didn't process it
    event.Skip();
}

It's very much a hack because (a) it's a duplication of the code in OnChar, and (b) I'm not sure at all that this is the right place for it. I do feel like there's something else going on that is behind the need for this in the first place.

comment:2 Changed 13 months ago by ericw

  • Cc ewasylishen@… added

Here is the patch I wrote for TrenchBroom:

diff --git src/osx/textctrl_osx.cpp src/osx/textctrl_osx.cpp
index 1ef8afa420..452d4272cf 100644
--- src/osx/textctrl_osx.cpp
+++ src/osx/textctrl_osx.cpp
@@ -342,6 +342,13 @@ void wxTextCtrl::OnDropFiles(wxDropFilesEvent& event)
 
 void wxTextCtrl::OnKeyDown(wxKeyEvent& event)
 {
+    int key = event.GetKeyCode();
+    if ( key == WXK_RETURN || key == WXK_NUMPAD_ENTER ) {
+        // HACK: Forward these keys to the OnChar handler
+        OnChar(event);
+        return;
+    }
+
     if ( event.GetModifiers() == wxMOD_CONTROL )
     {
         switch( event.GetKeyCode() )

comment:3 Changed 11 months ago by vadz

Could somebody please test if this bug is still present after the changes of 606dc18e5fda93a8ffb741d96e8e84b0856608fc or whether it fixed by them as well?

comment:4 Changed 11 months ago by awi

  • Resolution set to fixed
  • Status changed from new to closed

It seems the issue is fixed by 606dc18e5fda93a8ffb741d96e8e84b0856608fc.

Tested with following code snippet:

  • samples/minimal/minimal.cpp

    diff --git a/samples/minimal/minimal.cpp b/samples/minimal/minimal.cpp
    a b bool MyApp::OnInit() 
    140140// main frame
    141141// ----------------------------------------------------------------------------
    142142
     143class TestDialog : public wxDialog
     144{
     145public:
     146    TestDialog(wxWindow* parent) : wxDialog(parent, wxID_ANY, "Test")
     147    {
     148        wxTextCtrl* tc1 = new wxTextCtrl(this, wxID_ANY, "12345678", wxDefaultPosition, wxDefaultSize, 0);
     149        wxTextCtrl* tc2 = new wxTextCtrl(this, wxID_ANY, "ABCDEFGH", wxDefaultPosition, wxDefaultSize, wxTE_PROCESS_ENTER);
     150        wxButton* btn1 = new wxButton(this, wxID_OK, "OK");
     151        btn1->SetDefault();
     152        wxButton* btn2 = new wxButton(this, wxID_CANCEL, "Cancel");
     153
     154        wxBoxSizer* sizer = new wxBoxSizer(wxVERTICAL);
     155        sizer->Add(tc1, 1, wxEXPAND | wxALL, 10);
     156        sizer->Add(tc2, 1, wxEXPAND | wxALL, 10);
     157        sizer->Add(btn1, 1, wxEXPAND | wxALL, 10);
     158        sizer->Add(btn2, 1, wxEXPAND | wxALL, 10);
     159        SetSizerAndFit(sizer);
     160    }
     161
     162    virtual ~TestDialog() {};
     163
     164};
     165
    143166// frame constructor
    144167MyFrame::MyFrame(const wxString& title)
    145168       : wxFrame(NULL, wxID_ANY, title)
    MyFrame::MyFrame(const wxString& title) 
    177200    CreateStatusBar(2);
    178201    SetStatusText("Welcome to wxWidgets!");
    179202#endif // wxUSE_STATUSBAR
     203
     204    TestDialog dlg(this);
     205    dlg.ShowModal();
    180206}

comment:5 Changed 11 months ago by vadz

Thanks a lot for testing this, awi (and for fixing it in the first place too, of course)!

comment:6 follow-up: Changed 8 months ago by obfuscated

  • Cc fuscated@… added

I'm testing with current master and the problem is still there.
I'm testing a build with 10.14 sdk on Mojave. I'm at commit ae1fa08188
I don't know if older OSes exhibit the same problem.

I've found this when I've decided to try to reproduce the opposite problem we have in Code::Blocks. If the focus is on a wxComboBox control pressing enter doesn't activate the default button in a dialog.

This is the modified patch I use:

  • samples/minimal/minimal.cpp

    diff --git a/samples/minimal/minimal.cpp b/samples/minimal/minimal.cpp
    index f765a70dac..28f325522c 100644
    a b bool MyApp::OnInit() 
    140140// main frame
    141141// ----------------------------------------------------------------------------
    142142
     143class TestDialog : public wxDialog
     144{
     145public:
     146    TestDialog(wxWindow* parent) : wxDialog(parent, wxID_ANY, "Test")
     147    {
     148        wxTextCtrl* tc1 = new wxTextCtrl(this, wxID_ANY, "12345678", wxDefaultPosition, wxDefaultSize, 0);
     149        wxTextCtrl* tc2 = new wxTextCtrl(this, wxID_ANY, "ABCDEFGH", wxDefaultPosition, wxDefaultSize, wxTE_PROCESS_ENTER);
     150        wxComboBox* cb1 = new wxComboBox(this, wxID_ANY, "12345678", wxDefaultPosition, wxDefaultSize, 0);
     151        wxComboBox* cb2 = new wxComboBox(this, wxID_ANY, "12345678", wxDefaultPosition, wxDefaultSize, 0, NULL, wxTE_PROCESS_ENTER);
     152        wxButton* btn1 = new wxButton(this, wxID_OK, "OK");
     153        btn1->SetDefault();
     154        wxButton* btn2 = new wxButton(this, wxID_CANCEL, "Cancel");
     155
     156        wxBoxSizer* sizer = new wxBoxSizer(wxVERTICAL);
     157        sizer->Add(tc1, 1, wxEXPAND | wxALL, 10);
     158        sizer->Add(tc2, 1, wxEXPAND | wxALL, 10);
     159        sizer->Add(cb1, 1, wxEXPAND | wxALL, 10);
     160        sizer->Add(cb2, 1, wxEXPAND | wxALL, 10);
     161        sizer->Add(btn1, 1, wxEXPAND | wxALL, 10);
     162        sizer->Add(btn2, 1, wxEXPAND | wxALL, 10);
     163        SetSizerAndFit(sizer);
     164    }
     165
     166    virtual ~TestDialog() {};
     167
     168};
     169
    143170// frame constructor
    144171MyFrame::MyFrame(const wxString& title)
    145172       : wxFrame(NULL, wxID_ANY, title)
    MyFrame::MyFrame(const wxString& title) 
    177204    CreateStatusBar(2);
    178205    SetStatusText("Welcome to wxWidgets!");
    179206#endif // wxUSE_STATUSBAR
     207
     208    TestDialog dlg(this);
     209    dlg.ShowModal();
    180210}
    181211
    182212
    void MyFrame::OnQuit(wxCommandEvent& WXUNUSED(event)) 
    190220
    191221void MyFrame::OnAbout(wxCommandEvent& WXUNUSED(event))
    192222{
     223    TestDialog dlg(this);
     224    dlg.ShowModal();
     225    return;
     226
    193227    wxMessageBox(wxString::Format
    194228                 (
    195229                    "Welcome to %s!\n"

What I do to reproduce is start the minimal sample and do these:

  1. press enter -> closes the dialog
  2. press tab, enter -> closes the dialog
  3. press tab, tab, enter -> doesn't close the dialog
  4. press tab, tab, tab, enter -> doesn't close the dialog

On gtk2 these steps look like:

  1. press enter -> closes the dialog
  2. press tab, enter -> doesn't close the dialog
  3. press tab, tab, enter -> closes the dialog
  4. press tab, tab, tab, enter -> doesn't close the dialog

Which I think is the correct behaviour. The gtk2 build is made from a similar commit f71e8d077f.

Should I report the combobox problem in a separate issue.

comment:7 in reply to: ↑ 6 Changed 8 months ago by awi

Should I report the combobox problem in a separate issue.

This ticket is about non-wxTE_PROCESS_ENTER wxTextCtrl so if you think there is an issue with wxTextCtrl having wxTE_PROCESS_ENTER option set it would be better if you create separate ticket to avoid mess. Same for wxComboBox behavior.

FYI: Under wxMSW your code works like this:

  1. closes the dialog
  2. closes the dialog
  3. closes the dialog
  4. doesn't close the dialog

comment:8 follow-up: Changed 8 months ago by obfuscated

It is clear that there is an inconsistency. If it is an issue or not is up to wx devs to tell me.

I'll open an issue about wxcombobox.

comment:9 in reply to: ↑ 8 Changed 8 months ago by obfuscated

  • Cc fuscated@… removed

Done #18273

Note: See TracTickets for help on using tickets.