Merge branch 'multiline-enter-fix'

Fix handling Enter in multiline text controls in wxMSW.

And other improvements, notably don't send wxEVT_TEXT_ENTER for controls
without wxTE_PROCESS_ENTER style.

Closes #18491.

See https://github.com/wxWidgets/wxWidgets/pull/1539

docs/changes.txt

@@ -73,6 +73,11 @@ Changes in behaviour not resulting in compilation errors
 - wxDC::DrawCheckMark() draws the same shape under all platforms now, use the
   new wxRendererNative::DrawCheckMark() to draw MSW-specific themed check mark.
 
+- wxTE_PROCESS_ENTER must be used to receive wxEVT_TEXT_ENTER events from even
+  multiline wxTextCtrl, conforming to the documentation, but contrary to the
+  previous behaviour in wxMSW, when these events were always generated in this
+  case. Please add wxTE_PROCESS_ENTER style if you relied on the old behaviour.
+
 
 Changes in behaviour which may result in build errors
 -----------------------------------------------------

samples/dialogs/dialogs.cpp

@@ -2581,6 +2581,34 @@ wxBEGIN_EVENT_TABLE(TestDefaultActionDialog, wxDialog)
     EVT_TEXT_ENTER(wxID_ANY,                 TestDefaultActionDialog::OnTextEnter)
 wxEND_EVENT_TABLE()
 
+// TODO-C++11: We can't declare this class inside TestDefaultActionDialog
+//             itself when using C++98, so we have to do it here instead.
+namespace
+{
+
+// We have to define a new class in order to actually handle pressing
+// Enter, if we didn't do it, pressing it would still close the dialog.
+class EnterHandlingTextCtrl : public wxTextCtrl
+{
+public:
+    EnterHandlingTextCtrl(wxWindow* parent, int id, const wxString& value)
+        : wxTextCtrl(parent, id, value,
+                     wxDefaultPosition, wxDefaultSize, wxTE_PROCESS_ENTER)
+    {
+        Bind(wxEVT_TEXT_ENTER, &EnterHandlingTextCtrl::OnEnter, this);
+
+        SetInitialSize(GetSizeFromTextSize(GetTextExtent(value).x));
+    }
+
+private:
+    void OnEnter(wxCommandEvent& WXUNUSED(event))
+    {
+        wxLogMessage("Enter pressed");
+    }
+};
+
+} // anonymous namespace
+
 TestDefaultActionDialog::TestDefaultActionDialog( wxWindow *parent ) :
   wxDialog( parent, -1, "Test default action" )
 {
@@ -2588,7 +2616,8 @@ TestDefaultActionDialog::TestDefaultActionDialog( wxWindow *parent ) :
 
     wxBoxSizer *main_sizer = new wxBoxSizer( wxVERTICAL );
 
-    wxFlexGridSizer *grid_sizer = new wxFlexGridSizer( 2, 5, 5 );
+    const int border = wxSizerFlags::GetDefaultBorder();
+    wxFlexGridSizer *grid_sizer = new wxFlexGridSizer(2, wxSize(border, border));
 
 #if wxUSE_LISTBOX
     wxListBox *listbox = new wxListBox( this, ID_LISTBOX );
@@ -2599,22 +2628,37 @@ TestDefaultActionDialog::TestDefaultActionDialog( wxWindow *parent ) :
     grid_sizer->Add( listbox );
 #endif // wxUSE_LISTBOX
 
-    grid_sizer->Add( new wxCheckBox( this, ID_CATCH_LISTBOX_DCLICK, "Catch DoubleClick from wxListBox" ), 0, wxALIGN_CENTRE_VERTICAL );
+    grid_sizer->Add(new wxCheckBox(this, ID_CATCH_LISTBOX_DCLICK, "Catch DoubleClick from wxListBox"),
+                    wxSizerFlags().CentreVertical());
 
-    grid_sizer->Add( new wxTextCtrl( this, -1, "", wxDefaultPosition, wxSize(80,-1), 0 ), 0, wxALIGN_CENTRE_VERTICAL );
+    grid_sizer->Add(new wxTextCtrl(this, wxID_ANY, "Enter here closes the dialog"),
-    grid_sizer->Add( new wxStaticText( this, -1, "wxTextCtrl without wxTE_PROCESS_ENTER" ), 0, wxALIGN_CENTRE_VERTICAL );
+                    wxSizerFlags().Expand().CentreVertical());
+    grid_sizer->Add(new wxStaticText(this, wxID_ANY, "wxTextCtrl without wxTE_PROCESS_ENTER"),
+                    wxSizerFlags().CentreVertical());
 
-    grid_sizer->Add( new wxTextCtrl( this, -1, "", wxDefaultPosition, wxSize(80,-1), wxTE_PROCESS_ENTER ), 0, wxALIGN_CENTRE_VERTICAL );
+    grid_sizer->Add(new EnterHandlingTextCtrl(this, wxID_ANY, "Enter here is handled by the application"),
-    grid_sizer->Add( new wxStaticText( this, -1, "wxTextCtrl with wxTE_PROCESS_ENTER" ), 0, wxALIGN_CENTRE_VERTICAL );
+                    wxSizerFlags().CentreVertical());
+    grid_sizer->Add(new wxStaticText(this, wxID_ANY, "wxTextCtrl with wxTE_PROCESS_ENTER"),
+                    wxSizerFlags().CentreVertical());
 
-    grid_sizer->Add( new wxCheckBox(this, ID_DISABLE_OK, "Disable \"OK\""), 0, wxALIGN_CENTRE_VERTICAL );
+    grid_sizer->Add(new wxTextCtrl(this, wxID_ANY,
-    grid_sizer->Add( new wxCheckBox(this, ID_DISABLE_CANCEL, "Disable \"Cancel\""), 0, wxALIGN_CENTRE_VERTICAL );
+                                   "Enter here adds another line,\n"
+                                   "while Ctrl-Enter closes the dialog",
+                                   wxDefaultPosition, wxDefaultSize, wxTE_MULTILINE),
+                    wxSizerFlags().Expand());
+    grid_sizer->Add(new wxStaticText(this, wxID_ANY, "wxTextCtrl without wxTE_PROCESS_ENTER"),
+                    wxSizerFlags().CentreVertical());
 
-    main_sizer->Add( grid_sizer, 0, wxALL, 10 );
+    grid_sizer->Add(new wxCheckBox(this, ID_DISABLE_OK, "Disable \"OK\""),
+                    wxSizerFlags().CentreVertical());
+    grid_sizer->Add(new wxCheckBox(this, ID_DISABLE_CANCEL, "Disable \"Cancel\""),
+                    wxSizerFlags().CentreVertical());
+
+    main_sizer->Add(grid_sizer, wxSizerFlags().DoubleBorder());
 
     wxSizer *button_sizer = CreateSeparatedButtonSizer( wxOK|wxCANCEL );
     if ( button_sizer )
-        main_sizer->Add( button_sizer, 0, wxALL|wxGROW, 5 );
+        main_sizer->Add(button_sizer, wxSizerFlags().Expand().Border());
 
     SetSizerAndFit( main_sizer );
 }

src/msw/textctrl.cpp

@@ -2075,14 +2075,18 @@ void wxTextCtrl::OnChar(wxKeyEvent& event)
     switch ( event.GetKeyCode() )
     {
         case WXK_RETURN:
+            // Single line controls only get this key code if they have
+            // wxTE_PROCESS_ENTER style, but multiline ones always get it
+            // because they need it for themselves. However we shouldn't
+            // generate wxEVT_TEXT_ENTER for the controls without this style,
+            // so test for it explicitly.
+            if ( HasFlag(wxTE_PROCESS_ENTER) )
             {
                 wxCommandEvent evt(wxEVT_TEXT_ENTER, m_windowId);
                 InitCommandEvent(evt);
                 evt.SetString(GetValue());
                 if ( HandleWindowEvent(evt) )
-                    if ( !HasFlag(wxTE_MULTILINE) )
                     return;
-                    //else: multiline controls need Enter for themselves
             }
             break;
 
@@ -2212,7 +2216,7 @@ wxTextCtrl::MSWHandleMessage(WXLRESULT *rc,
             // 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 ( !processed && wParam == VK_RETURN && IsSingleLine() )
             {
                 if ( ClickDefaultButtonIfPossible() )
                     processed = true;

tests/controls/comboboxtest.cpp

@@ -233,9 +233,10 @@ void ComboBoxTestCase::IsEmpty()
 
 TEST_CASE("wxComboBox::ProcessEnter", "[wxComboBox][enter]")
 {
-    struct ComboBoxCreator
+    class ComboBoxCreator : public TextLikeControlCreator
     {
-        static wxControl* Do(wxWindow* parent, int style)
+    public:
+        virtual wxControl* Create(wxWindow* parent, int style) const wxOVERRIDE
         {
             const wxString choices[] = { "foo", "bar", "baz" };
 
@@ -246,7 +247,7 @@ TEST_CASE("wxComboBox::ProcessEnter", "[wxComboBox][enter]")
         }
     };
 
-    TestProcessEnter(&ComboBoxCreator::Do);
+    TestProcessEnter(ComboBoxCreator());
 }
 
 #endif //wxUSE_COMBOBOX

tests/controls/textctrltest.cpp

@@ -1271,17 +1271,31 @@ void TextCtrlTestCase::XYToPositionSingleLine()
 
 TEST_CASE("wxTextCtrl::ProcessEnter", "[wxTextCtrl][enter]")
 {
-    struct TextCtrlCreator
+    class TextCtrlCreator : public TextLikeControlCreator
     {
-        static wxControl* Do(wxWindow* parent, int style)
+    public:
+        explicit TextCtrlCreator(int styleToAdd = 0)
+            : m_styleToAdd(styleToAdd)
+        {
+        }
+
+        virtual wxControl* Create(wxWindow* parent, int style) const wxOVERRIDE
         {
             return new wxTextCtrl(parent, wxID_ANY, wxString(),
                                   wxDefaultPosition, wxDefaultSize,
-                                  style);
+                                  style | m_styleToAdd);
         }
+
+        virtual TextLikeControlCreator* CloneAsMultiLine() const wxOVERRIDE
+        {
+            return new TextCtrlCreator(wxTE_MULTILINE);
+        }
+
+    private:
+        int m_styleToAdd;
     };
 
-    TestProcessEnter(&TextCtrlCreator::Do);
+    TestProcessEnter(TextCtrlCreator());
 }
 
 #endif //wxUSE_TEXTCTRL

tests/controls/textentrytest.cpp

@@ -21,6 +21,8 @@
 
 #include "textentrytest.h"
 #include "testableframe.h"
+
+#include "wx/scopedptr.h"
 #include "wx/uiaction.h"
 
 void TextEntryTestCase::SetValue()
@@ -384,21 +386,20 @@ enum ProcessEnter
 class TestDialog : public wxDialog
 {
 public:
-    explicit TestDialog(TextLikeControlCreator controlCreator,
+    explicit TestDialog(const TextLikeControlCreator& controlCreator,
                         ProcessEnter processEnter)
         : wxDialog(wxTheApp->GetTopWindow(), wxID_ANY, "Test dialog"),
-          m_control((*controlCreator)(this,
+          m_control
-                                      processEnter == ProcessEnter_No
+          (
-                                        ? 0
+              controlCreator.Create
-                                        : wxTE_PROCESS_ENTER)),
+              (
+               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));
@@ -435,6 +436,25 @@ private:
         }
     }
 
+    void OnText(wxCommandEvent& WXUNUSED(e))
+    {
+        // This should only happen for the multiline text controls.
+        switch ( m_processEnter )
+        {
+            case ProcessEnter_No:
+            case ProcessEnter_ButSkip:
+                // We consider that the text succeeded, but in a different way,
+                // so use a different ID to be able to distinguish between this
+                // scenario and Enter activating the default button.
+                EndModal(wxID_CLOSE);
+                break;
+
+            case ProcessEnter_WithoutSkipping:
+                FAIL("Shouldn't be getting wxEVT_TEXT if handled");
+                break;
+        }
+    }
+
     void OnTimeOut(wxTimerEvent&)
     {
         EndModal(wxID_CANCEL);
@@ -451,11 +471,22 @@ private:
     const ProcessEnter m_processEnter;
     wxTimer m_timer;
     bool m_gotEnter;
+
+    wxDECLARE_EVENT_TABLE();
 };
 
+// Note that we must use event table macros here instead of Bind() because
+// binding wxEVT_TEXT_ENTER handler for a control without wxTE_PROCESS_ENTER
+// style would fail with an assertion failure, due to wx helpfully complaining
+// about it.
+wxBEGIN_EVENT_TABLE(TestDialog, wxDialog)
+    EVT_TEXT(wxID_ANY, TestDialog::OnText)
+    EVT_TEXT_ENTER(wxID_ANY, TestDialog::OnTextEnter)
+wxEND_EVENT_TABLE()
+
 } // anonymous namespace
 
-void TestProcessEnter(TextLikeControlCreator controlCreator)
+void TestProcessEnter(const TextLikeControlCreator& controlCreator)
 {
     if ( !EnableUITests() )
     {
@@ -483,11 +514,47 @@ void TestProcessEnter(TextLikeControlCreator controlCreator)
         REQUIRE( dlgProcessEnter.ShowModal() == wxID_APPLY );
         CHECK( dlgProcessEnter.GotEnter() );
     }
+
+    SECTION("Without wxTE_PROCESS_ENTER but with wxTE_MULTILINE")
+    {
+        wxScopedPtr<TextLikeControlCreator>
+            multiLineCreator(controlCreator.CloneAsMultiLine());
+        if ( !multiLineCreator )
+            return;
+
+        TestDialog dlg(*multiLineCreator, ProcessEnter_No);
+        REQUIRE( dlg.ShowModal() == wxID_CLOSE );
+        CHECK( !dlg.GotEnter() );
+    }
+
+    SECTION("With wxTE_PROCESS_ENTER and wxTE_MULTILINE but skipping")
+    {
+        wxScopedPtr<TextLikeControlCreator>
+            multiLineCreator(controlCreator.CloneAsMultiLine());
+        if ( !multiLineCreator )
+            return;
+
+        TestDialog dlg(*multiLineCreator, ProcessEnter_ButSkip);
+        REQUIRE( dlg.ShowModal() == wxID_CLOSE );
+        CHECK( dlg.GotEnter() );
+    }
+
+    SECTION("With wxTE_PROCESS_ENTER and wxTE_MULTILINE without skipping")
+    {
+        wxScopedPtr<TextLikeControlCreator>
+            multiLineCreator(controlCreator.CloneAsMultiLine());
+        if ( !multiLineCreator )
+            return;
+
+        TestDialog dlg(*multiLineCreator, ProcessEnter_WithoutSkipping);
+        REQUIRE( dlg.ShowModal() == wxID_APPLY );
+        CHECK( dlg.GotEnter() );
+    }
 }
 
 #else // !wxUSE_UIACTIONSIMULATOR
 
-void TestProcessEnter(TextLikeControlCreator WXUNUSED(controlCreator))
+void TestProcessEnter(const TextLikeControlCreator& WXUNUSED(controlCreator))
 {
     WARN("Skipping wxTE_PROCESS_ENTER tests: wxUIActionSimulator not available");
 }

tests/controls/textentrytest.h

@@ -75,11 +75,32 @@ private:
     wxDECLARE_NO_COPY_CLASS(TextEntryTestCase);
 };
 
-// Function to call to test that wxTE_PROCESS_ENTER is handled correctly for
+// Helper used for creating the control of the specific type (currently either
-// the controls of the type created by the given creator function when they're
+// wxTextCtrl or wxComboBox) with the given flag.
-// placed in a dialog with a default button.
+class TextLikeControlCreator
-typedef wxControl* (*TextLikeControlCreator)(wxWindow* parent, int style);
+{
+public:
+    TextLikeControlCreator() {}
 
-void TestProcessEnter(TextLikeControlCreator controlCreator);
+    // Create the control of the right type using the given parent and style.
+    virtual wxControl* Create(wxWindow* parent, int style) const = 0;
+
+    // Return another creator similar to this one, but creating multiline
+    // version of the control. If the returned pointer is non-null, it must be
+    // deleted by the caller.
+    virtual TextLikeControlCreator* CloneAsMultiLine() const { return NULL; }
+
+    // Give it a virtual dtor to avoid warnings even though this class is not
+    // supposed to be used polymorphically.
+    virtual ~TextLikeControlCreator() {}
+
+private:
+    wxDECLARE_NO_COPY_CLASS(TextLikeControlCreator);
+};
+
+// Use the given control creator to check that various combinations of
+// specifying and not specifying wxTE_PROCESS_ENTER and handling or not
+// handling the resulting event work as expected.
+void TestProcessEnter(const TextLikeControlCreator& controlCreator);
 
 #endif // _WX_TESTS_CONTROLS_TEXTENTRYTEST_H_