From 8746da90fe790dc336df0aba4fbb2e87fb55715c Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 14 Jul 2019 01:31:45 +0200 Subject: [PATCH 1/7] Add wxTextEntry::ClickDefaultButtonIfPossible() to wxMSW Factor out this function from wxTextCtrl to allow its reuse in wxComboBox in the upcoming commit. No real changes, this is a pure refactoring. --- include/wx/msw/textentry.h | 4 ++++ src/msw/textctrl.cpp | 5 ++--- src/msw/textentry.cpp | 11 +++++++++++ 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/include/wx/msw/textentry.h b/include/wx/msw/textentry.h index 0b11a2ffa1..a5f2305125 100644 --- a/include/wx/msw/textentry.h +++ b/include/wx/msw/textentry.h @@ -81,6 +81,10 @@ protected: virtual bool DoAutoCompleteCustom(wxTextCompleter *completer) wxOVERRIDE; #endif // wxUSE_OLE + // Helper for wxTE_PROCESS_ENTER handling: activates the default button in + // the dialog containing this control if any. + bool ClickDefaultButtonIfPossible(); + private: // implement this to return the HWND of the EDIT control virtual WXHWND GetEditHWND() const = 0; diff --git a/src/msw/textctrl.cpp b/src/msw/textctrl.cpp index 9b62c686a6..888489fd01 100644 --- a/src/msw/textctrl.cpp +++ b/src/msw/textctrl.cpp @@ -2191,10 +2191,9 @@ wxTextCtrl::MSWHandleMessage(WXLRESULT *rc, if ( nMsg == WM_CHAR && !processed && HasFlag(wxTE_PROCESS_ENTER) && - wParam == VK_RETURN && - !wxIsAnyModifierDown() ) + wParam == VK_RETURN ) { - MSWClickButtonIfPossible(MSWGetDefaultButtonFor(this)); + ClickDefaultButtonIfPossible(); processed = true; } diff --git a/src/msw/textentry.cpp b/src/msw/textentry.cpp index 49c7601ddd..7bb30d0702 100644 --- a/src/msw/textentry.cpp +++ b/src/msw/textentry.cpp @@ -1036,4 +1036,15 @@ wxPoint wxTextEntry::DoGetMargins() const return wxPoint(left, top); } +// ---------------------------------------------------------------------------- +// input handling +// ---------------------------------------------------------------------------- + +bool wxTextEntry::ClickDefaultButtonIfPossible() +{ + return !wxIsAnyModifierDown() && + wxWindow::MSWClickButtonIfPossible( + wxWindow::MSWGetDefaultButtonFor(GetEditableWindow())); +} + #endif // wxUSE_TEXTCTRL || wxUSE_COMBOBOX From 17f9a71586dbbe50ab129f71bbb7d3fcae367ae0 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 14 Jul 2019 01:33:09 +0200 Subject: [PATCH 2/7] Don't mark WM_CHAR for VK_RETURN as processed if it wasn't This doesn't seem to change anything in practice, but it's more logical to not pretend that we processed the message if we didn't do anything at all with it. --- src/msw/textctrl.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/msw/textctrl.cpp b/src/msw/textctrl.cpp index 888489fd01..80eec63138 100644 --- a/src/msw/textctrl.cpp +++ b/src/msw/textctrl.cpp @@ -2193,9 +2193,8 @@ wxTextCtrl::MSWHandleMessage(WXLRESULT *rc, HasFlag(wxTE_PROCESS_ENTER) && wParam == VK_RETURN ) { - ClickDefaultButtonIfPossible(); - - processed = true; + if ( ClickDefaultButtonIfPossible() ) + processed = true; } switch ( nMsg ) From c43e0fa123a47cf4f5d79e88336544d6fc5de399 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 14 Jul 2019 01:40:13 +0200 Subject: [PATCH 3/7] Move WM_CHAR handling inside the switch in wxTextCtrl code No real changes, just check for WM_CHAR inside the existing "switch" instead of writing a separate "if" statement just for it. Note that removing HasFlag(wxTE_PROCESS_ENTER) test shouldn't matter as we should be only getting WM_CHAR for VK_RETURN when this flag is used. --- src/msw/textctrl.cpp | 37 ++++++++++++++++++------------------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/src/msw/textctrl.cpp b/src/msw/textctrl.cpp index 80eec63138..678a04b09c 100644 --- a/src/msw/textctrl.cpp +++ b/src/msw/textctrl.cpp @@ -2178,27 +2178,26 @@ wxTextCtrl::MSWHandleMessage(WXLRESULT *rc, { bool processed = wxTextCtrlBase::MSWHandleMessage(rc, nMsg, wParam, lParam); - // Handle the special case of "Enter" key: the user code needs to specify - // wxTE_PROCESS_ENTER style to get it in the first place, but if this flag - // is used, then even if the wxEVT_TEXT_ENTER handler skips the event, the - // normal action of this key is not performed because IsDialogMessage() is - // not called and, also, an annoying beep is generated by EDIT default - // WndProc. - // - // Fix these problems by explicitly performing the default function of this - // key (which would be done by MSWProcessMessage() if we didn't have - // wxTE_PROCESS_ENTER) and preventing the default WndProc from getting it. - if ( nMsg == WM_CHAR && - !processed && - HasFlag(wxTE_PROCESS_ENTER) && - wParam == VK_RETURN ) - { - if ( ClickDefaultButtonIfPossible() ) - processed = true; - } - switch ( nMsg ) { + case WM_CHAR: + // Handle the special case of "Enter" key: the user code needs to specify + // wxTE_PROCESS_ENTER style to get it in the first place, but if this flag + // is used, then even if the wxEVT_TEXT_ENTER handler skips the event, the + // normal action of this key is not performed because IsDialogMessage() is + // not called and, also, an annoying beep is generated by EDIT default + // WndProc. + // + // Fix these problems by explicitly performing the default function of this + // key (which would be done by MSWProcessMessage() if we didn't have + // wxTE_PROCESS_ENTER) and preventing the default WndProc from getting it. + if ( !processed && wParam == VK_RETURN ) + { + if ( ClickDefaultButtonIfPossible() ) + processed = true; + } + break; + case WM_GETDLGCODE: { // Ensure that the result value is initialized even if the base From ef03d3bb9346e732f8c8721a2534779474501c0f Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 14 Jul 2019 01:34:23 +0200 Subject: [PATCH 4/7] Fix "Enter" in wxComboBox with wxTE_PROCESS_ENTER in wxMSW Activate the default button of the dialog containing the wxComboBox if wxEVT_TEXT_ENTER handler didn't process the even, just as it was already done for wxTextCtrl. See #18273. --- src/msw/combobox.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/msw/combobox.cpp b/src/msw/combobox.cpp index e2072b31cc..afb0a7f06d 100644 --- a/src/msw/combobox.cpp +++ b/src/msw/combobox.cpp @@ -243,6 +243,9 @@ bool wxComboBox::MSWProcessEditSpecialKey(WXWPARAM vkey) // beep if it gets it return true; } + + if ( ClickDefaultButtonIfPossible() ) + return true; } break; From f7ead9f8444dcec57e0227af7a05637e9ee5117f Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 14 Jul 2019 01:56:54 +0200 Subject: [PATCH 5/7] Fix "Enter" behaviour when wxEVT_TEXT_ENTER is skipped in wxGTK Bring wxGTK in sync with wxMSW and wxMac by activating the default button manually if wxEVT_TEXT_ENTER handler skips the event. --- include/wx/gtk/textentry.h | 4 ++++ src/gtk/combobox.cpp | 6 ++++++ src/gtk/textctrl.cpp | 6 ++++++ src/gtk/textentry.cpp | 31 +++++++++++++++++++++++++++++++ 4 files changed, 47 insertions(+) diff --git a/include/wx/gtk/textentry.h b/include/wx/gtk/textentry.h index aaf777a515..3410ec3924 100644 --- a/include/wx/gtk/textentry.h +++ b/include/wx/gtk/textentry.h @@ -86,6 +86,10 @@ protected: static int GTKGetEntryTextLength(GtkEntry* entry); + // Helper for wxTE_PROCESS_ENTER handling: activates the default button in + // the dialog containing this control if any. + bool ClickDefaultButtonIfPossible(); + private: // implement this to return the associated GtkEntry or another widget // implementing GtkEditable diff --git a/src/gtk/combobox.cpp b/src/gtk/combobox.cpp index 578088c0fd..aa533ec34a 100644 --- a/src/gtk/combobox.cpp +++ b/src/gtk/combobox.cpp @@ -241,6 +241,12 @@ void wxComboBox::OnChar( wxKeyEvent &event ) // down list upon RETURN. return; } + + // We disable built-in default button activation when + // wxTE_PROCESS_ENTER is used, but we still should activate it + // if the event wasn't handled, so do it from here. + if ( ClickDefaultButtonIfPossible() ) + return; } break; } diff --git a/src/gtk/textctrl.cpp b/src/gtk/textctrl.cpp index 54af9c1def..13fa13196c 100644 --- a/src/gtk/textctrl.cpp +++ b/src/gtk/textctrl.cpp @@ -1693,6 +1693,12 @@ void wxTextCtrl::OnChar( wxKeyEvent &key_event ) event.SetString(GetValue()); if ( HandleWindowEvent(event) ) return; + + // We disable built-in default button activation when + // wxTE_PROCESS_ENTER is used, but we still should activate it + // if the event wasn't handled, so do it from here. + if ( ClickDefaultButtonIfPossible() ) + return; } } diff --git a/src/gtk/textentry.cpp b/src/gtk/textentry.cpp index 598d7593a0..eb6e91c0eb 100644 --- a/src/gtk/textentry.cpp +++ b/src/gtk/textentry.cpp @@ -986,4 +986,35 @@ wxString wxTextEntry::GetHint() const } #endif // __WXGTK3__ +bool wxTextEntry::ClickDefaultButtonIfPossible() +{ + GtkWidget* const widget = GTK_WIDGET(GetEntry()); + + // This does the same thing as gtk_entry_real_activate() in GTK itself. + // + // Note: in GTK 4 we should probably just use gtk_widget_activate_default(). + GtkWidget* const toplevel = gtk_widget_get_toplevel(widget); + if ( GTK_IS_WINDOW (toplevel) ) + { + GtkWindow* const window = GTK_WINDOW(toplevel); + + if ( window ) + { + GtkWidget* const default_widget = gtk_window_get_default_widget(window); + GtkWidget* const focus_widget = gtk_window_get_focus(window); + + if ( widget != default_widget && + !(widget == focus_widget && + (!default_widget || + !gtk_widget_get_sensitive(default_widget))) ) + { + if ( gtk_window_activate_default(window) ) + return true; + } + } + } + + return false; +} + #endif // wxUSE_TEXTCTRL || wxUSE_COMBOBOX From 13aa2a6721d6e8a325d0d8b59c8f3c4f4a1d8bcd Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 14 Jul 2019 01:03:11 +0200 Subject: [PATCH 6/7] Add unit tests for checking wxTE_PROCESS_ENTER handling Verify that pressing Enter in a dialog activates its default button when a text-like (i.e. wxTextCtrl or wxComboBox) has focus either if it doesn't have wxTE_PROCESS_ENTER style or if it does, but its handler skips the event, but not if the style is used and the event is handled. --- tests/controls/comboboxtest.cpp | 18 +++++ tests/controls/textctrltest.cpp | 15 ++++ tests/controls/textentrytest.cpp | 123 +++++++++++++++++++++++++++++++ tests/controls/textentrytest.h | 7 ++ 4 files changed, 163 insertions(+) diff --git a/tests/controls/comboboxtest.cpp b/tests/controls/comboboxtest.cpp index c72373321e..dd78119ead 100644 --- a/tests/controls/comboboxtest.cpp +++ b/tests/controls/comboboxtest.cpp @@ -231,4 +231,22 @@ void ComboBoxTestCase::IsEmpty() #endif } +TEST_CASE("wxComboBox::ProcessEnter", "[wxComboBox][enter]") +{ + struct ComboBoxCreator + { + static wxControl* Do(wxWindow* parent, int style) + { + const wxString choices[] = { "foo", "bar", "baz" }; + + return new wxComboBox(parent, wxID_ANY, wxString(), + wxDefaultPosition, wxDefaultSize, + WXSIZEOF(choices), choices, + style); + } + }; + + TestProcessEnter(&ComboBoxCreator::Do); +} + #endif //wxUSE_COMBOBOX diff --git a/tests/controls/textctrltest.cpp b/tests/controls/textctrltest.cpp index 75fb647c48..2fb9d181f9 100644 --- a/tests/controls/textctrltest.cpp +++ b/tests/controls/textctrltest.cpp @@ -1249,4 +1249,19 @@ void TextCtrlTestCase::XYToPositionSingleLine() } } +TEST_CASE("wxTextCtrl::ProcessEnter", "[wxTextCtrl][enter]") +{ + struct TextCtrlCreator + { + static wxControl* Do(wxWindow* parent, int style) + { + return new wxTextCtrl(parent, wxID_ANY, wxString(), + wxDefaultPosition, wxDefaultSize, + style); + } + }; + + TestProcessEnter(&TextCtrlCreator::Do); +} + #endif //wxUSE_TEXTCTRL diff --git a/tests/controls/textentrytest.cpp b/tests/controls/textentrytest.cpp index 0bda950169..54e8295d10 100644 --- a/tests/controls/textentrytest.cpp +++ b/tests/controls/textentrytest.cpp @@ -10,8 +10,12 @@ #ifndef WX_PRECOMP #include "wx/app.h" + #include "wx/dialog.h" #include "wx/event.h" + #include "wx/sizer.h" + #include "wx/textctrl.h" #include "wx/textentry.h" + #include "wx/timer.h" #include "wx/window.h" #endif // WX_PRECOMP @@ -364,3 +368,122 @@ void TextEntryTestCase::UndoRedo() } } } + +#if wxUSE_UIACTIONSIMULATOR + +namespace +{ + +enum ProcessEnter +{ + ProcessEnter_No, + ProcessEnter_ButSkip, + ProcessEnter_WithoutSkipping +}; + +class TestDialog : public wxDialog +{ +public: + explicit TestDialog(TextLikeControlCreator controlCreator, + ProcessEnter processEnter) + : wxDialog(wxTheApp->GetTopWindow(), wxID_ANY, "Test dialog"), + m_control((*controlCreator)(this, + processEnter == ProcessEnter_No + ? 0 + : wxTE_PROCESS_ENTER)), + m_processEnter(processEnter), + m_gotEnter(false) + { + // We can't always bind this handler because wx will helpfully + // complain if we bind it without using wxTE_PROCESS_ENTER. + if ( processEnter != ProcessEnter_No ) + m_control->Bind(wxEVT_TEXT_ENTER, &TestDialog::OnTextEnter, this); + + wxSizer* const sizer = new wxBoxSizer(wxVERTICAL); + sizer->Add(m_control, wxSizerFlags().Expand()); + sizer->Add(CreateStdDialogButtonSizer(wxOK)); + SetSizerAndFit(sizer); + + CallAfter(&TestDialog::SimulateEnter); + + m_timer.Bind(wxEVT_TIMER, &TestDialog::OnTimeOut, this); + m_timer.StartOnce(2000); + } + + bool GotEnter() const { return m_gotEnter; } + +private: + void OnTextEnter(wxCommandEvent& e) + { + m_gotEnter = true; + + switch ( m_processEnter ) + { + case ProcessEnter_No: + FAIL("Shouldn't be getting wxEVT_TEXT_ENTER at all"); + break; + + case ProcessEnter_ButSkip: + e.Skip(); + break; + + case ProcessEnter_WithoutSkipping: + // Close the dialog with a different exit code than what + // pressing the OK button would have generated. + EndModal(wxID_APPLY); + break; + } + } + + void OnTimeOut(wxTimerEvent&) + { + EndModal(wxID_CANCEL); + } + + void SimulateEnter() + { + wxUIActionSimulator sim; + m_control->SetFocus(); + sim.Char(WXK_RETURN); + } + + wxControl* const m_control; + const ProcessEnter m_processEnter; + wxTimer m_timer; + bool m_gotEnter; +}; + +} // anonymous namespace + +void TestProcessEnter(TextLikeControlCreator controlCreator) +{ + SECTION("Without wxTE_PROCESS_ENTER") + { + TestDialog dlg(controlCreator, ProcessEnter_No); + REQUIRE( dlg.ShowModal() == wxID_OK ); + CHECK( !dlg.GotEnter() ); + } + + SECTION("With wxTE_PROCESS_ENTER but skipping") + { + TestDialog dlgProcessEnter(controlCreator, ProcessEnter_ButSkip); + REQUIRE( dlgProcessEnter.ShowModal() == wxID_OK ); + CHECK( dlgProcessEnter.GotEnter() ); + } + + SECTION("With wxTE_PROCESS_ENTER without skipping") + { + TestDialog dlgProcessEnter(controlCreator, ProcessEnter_WithoutSkipping); + REQUIRE( dlgProcessEnter.ShowModal() == wxID_APPLY ); + CHECK( dlgProcessEnter.GotEnter() ); + } +} + +#else // !wxUSE_UIACTIONSIMULATOR + +void TestProcessEnter(TextLikeControlCreator WXUNUSED(controlCreator)) +{ + WARN("Skipping wxTE_PROCESS_ENTER tests: wxUIActionSimulator not available"); +} + +#endif // wxUSE_UIACTIONSIMULATOR/!wxUSE_UIACTIONSIMULATOR diff --git a/tests/controls/textentrytest.h b/tests/controls/textentrytest.h index fc784c9d6f..33879e6346 100644 --- a/tests/controls/textentrytest.h +++ b/tests/controls/textentrytest.h @@ -75,4 +75,11 @@ private: wxDECLARE_NO_COPY_CLASS(TextEntryTestCase); }; +// Function to call to test that wxTE_PROCESS_ENTER is handled correctly for +// the controls of the type created by the given creator function when they're +// placed in a dialog with a default button. +typedef wxControl* (*TextLikeControlCreator)(wxWindow* parent, int style); + +void TestProcessEnter(TextLikeControlCreator controlCreator); + #endif // _WX_TESTS_CONTROLS_TEXTENTRYTEST_H_ From 81a2984ba910884fdf91c0c48dc2bb026c30c802 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 14 Jul 2019 16:54:00 +0200 Subject: [PATCH 7/7] Improve wxTE_PROCESS_ENTER documentation Explicitly mention that if no handler for wxEVT_TEXT_ENTER is defined (or if a handler exists, but skips the event), the default handling of "Enter" still takes place. --- interface/wx/combobox.h | 10 +++++++--- interface/wx/textctrl.h | 10 +++++++--- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/interface/wx/combobox.h b/interface/wx/combobox.h index d6728a7efa..7c67126551 100644 --- a/interface/wx/combobox.h +++ b/interface/wx/combobox.h @@ -41,9 +41,13 @@ Sorts the entries in the list alphabetically. Notice that this style is not currently implemented in wxOSX. @style{wxTE_PROCESS_ENTER} - The control will generate the event @c wxEVT_TEXT_ENTER - (otherwise pressing Enter key is either processed internally by the - control or used for navigation between dialog controls). + The control will generate the event @c wxEVT_TEXT_ENTER that can be + handled by the program. Otherwise, i.e. either if this style not + specified at all, or it is used, but there is no event handler for + this event or the event handler called wxEvent::Skip() to avoid + overriding the default handling, pressing Enter key is either + processed internally by the control or used to activate the default + button of the dialog, if any. @endStyleTable @beginEventEmissionTable{wxCommandEvent} diff --git a/interface/wx/textctrl.h b/interface/wx/textctrl.h index a0b6b7464f..0495cb1393 100644 --- a/interface/wx/textctrl.h +++ b/interface/wx/textctrl.h @@ -941,9 +941,13 @@ public: @beginStyleTable @style{wxTE_PROCESS_ENTER} - The control will generate the event @c wxEVT_TEXT_ENTER - (otherwise pressing Enter key is either processed internally by the - control or used to activate the default button of the dialog, if any). + The control will generate the event @c wxEVT_TEXT_ENTER that can be + handled by the program. Otherwise, i.e. either if this style not + specified at all, or it is used, but there is no event handler for + this event or the event handler called wxEvent::Skip() to avoid + overriding the default handling, pressing Enter key is either + processed internally by the control or used to activate the default + button of the dialog, if any. @style{wxTE_PROCESS_TAB} Normally, TAB key is used for keyboard navigation and pressing it in a control switches focus to the next one. With this style, this