Merge branch 'gtk-datapick-focus'

Fix focus event generation for generic wxDatePickerCtrl and other
wxCompositeWindows.

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

Closes #18120.

include/wx/compositewin.h

@@ -186,8 +186,21 @@ private:
         // support) to hook into its event processing.
 
         wxWindow *child = event.GetWindow();
-        if ( child == this )
-            return; // not a child, we don't want to bind to ourselves
+
+        // Check that it's one of our children: it could also be this window
+        // itself (for which we don't need to handle focus at all) or one of
+        // its grandchildren and we don't want to bind to those as child
+        // controls are supposed to be well-behaved and get their own focus
+        // event if any of their children get focus anyhow, so binding to them
+        // would only result in duplicate events.
+        //
+        // Notice that we can't use GetCompositeWindowParts() here because the
+        // member variables that are typically used in its implementation in
+        // the derived classes would typically not be initialized yet, as this
+        // event is generated by "m_child = new wxChildControl(this, ...)" code
+        // before "m_child" is assigned.
+        if ( child->GetParent() != this )
+            return;
 
         child->Bind(wxEVT_SET_FOCUS, &wxCompositeWindow::OnSetFocus, this);
 

include/wx/generic/datectrl.h

@@ -20,7 +20,7 @@ class WXDLLIMPEXP_FWD_ADV wxCalendarCtrl;
 class WXDLLIMPEXP_FWD_ADV wxCalendarComboPopup;
 
 class WXDLLIMPEXP_ADV wxDatePickerCtrlGeneric
-    : public wxCompositeWindowSettersOnly< wxNavigationEnabled<wxDatePickerCtrlBase> >
+    : public wxCompositeWindow< wxNavigationEnabled<wxDatePickerCtrlBase> >
 {
 public:
     // creating the control
@@ -80,7 +80,6 @@ private:
 
     void OnText(wxCommandEvent &event);
     void OnSize(wxSizeEvent& event);
-    void OnFocus(wxFocusEvent& event);
 
 #ifdef __WXOSX_COCOA__
     virtual void OSXGenerateEvent(const wxDateTime& WXUNUSED(dt)) wxOVERRIDE { }

samples/widgets/searchctrl.cpp

@@ -88,9 +88,6 @@ protected:
     void OnSearch(wxCommandEvent& event);
     void OnSearchCancel(wxCommandEvent& event);
 
-    void OnSetFocus(wxFocusEvent& event);
-    void OnKillFocus(wxFocusEvent& event);
-
     wxMenu* CreateTestMenu();
 
     // (re)create the control
@@ -179,9 +176,6 @@ void SearchCtrlWidgetsPage::CreateControl()
 
     m_srchCtrl = new wxSearchCtrl(this, -1, wxEmptyString, wxDefaultPosition,
                                   FromDIP(wxSize(150, -1)), style);
-
-    m_srchCtrl->Bind(wxEVT_SET_FOCUS, &SearchCtrlWidgetsPage::OnSetFocus, this);
-    m_srchCtrl->Bind(wxEVT_KILL_FOCUS, &SearchCtrlWidgetsPage::OnKillFocus, this);
 }
 
 void SearchCtrlWidgetsPage::RecreateWidget()
@@ -256,18 +250,4 @@ void SearchCtrlWidgetsPage::OnSearchCancel(wxCommandEvent& event)
     event.Skip();
 }
 
-void SearchCtrlWidgetsPage::OnSetFocus(wxFocusEvent& event)
-{
-    wxLogMessage("Search control got focus");
-
-    event.Skip();
-}
-
-void SearchCtrlWidgetsPage::OnKillFocus(wxFocusEvent& event)
-{
-    wxLogMessage("Search control lost focus");
-
-    event.Skip();
-}
-
 #endif  //  wxUSE_SEARCHCTRL

samples/widgets/widgets.cpp

@@ -210,6 +210,10 @@ protected:
     WidgetsPage *CurrentPage();
 
 private:
+    void OnWidgetFocus(wxFocusEvent& event);
+
+    void ConnectToWidgetEvents();
+
     // the panel containing everything
     wxPanel *m_panel;
 
@@ -668,6 +672,19 @@ WidgetsPage *WidgetsFrame::CurrentPage()
     return wxStaticCast(page, WidgetsPage);
 }
 
+void WidgetsFrame::ConnectToWidgetEvents()
+{
+    const Widgets& widgets = CurrentPage()->GetWidgets();
+
+    for ( Widgets::const_iterator it = widgets.begin();
+            it != widgets.end();
+            ++it )
+    {
+        (*it)->Bind(wxEVT_SET_FOCUS, &WidgetsFrame::OnWidgetFocus, this);
+        (*it)->Bind(wxEVT_KILL_FOCUS, &WidgetsFrame::OnWidgetFocus, this);
+    }
+}
+
 WidgetsFrame::~WidgetsFrame()
 {
 #if USE_LOG
@@ -723,7 +740,10 @@ void WidgetsFrame::OnPageChanged(WidgetsBookCtrlEvent& event)
         wxWindowUpdateLocker noUpdates(curPage);
         curPage->CreateContent();
         curPage->Layout();
+
+        ConnectToWidgetEvents();
     }
+
     // re-apply the attributes to the widget(s)
     curPage->SetUpWidget();
 
@@ -890,6 +910,9 @@ void WidgetsFrame::OnSetBorder(wxCommandEvent& event)
     WidgetsPage *page = CurrentPage();
 
     page->RecreateWidget();
+
+    ConnectToWidgetEvents();
+
     // re-apply the attributes to the widget(s)
     page->SetUpWidget();
 }
@@ -1172,6 +1195,14 @@ void WidgetsFrame::OnSetHint(wxCommandEvent& WXUNUSED(event))
 
 #endif // wxUSE_MENUS
 
+void WidgetsFrame::OnWidgetFocus(wxFocusEvent& event)
+{
+    wxLogMessage("Widgets %s focus",
+                 event.GetEventType() == wxEVT_SET_FOCUS ? "got" : "lost");
+
+    event.Skip();
+}
+
 // ----------------------------------------------------------------------------
 // WidgetsPageInfo
 // ----------------------------------------------------------------------------

src/generic/datectlg.cpp

@@ -296,7 +296,6 @@ wxEND_EVENT_TABLE()
 wxBEGIN_EVENT_TABLE(wxDatePickerCtrlGeneric, wxDatePickerCtrlBase)
     EVT_TEXT(wxID_ANY, wxDatePickerCtrlGeneric::OnText)
     EVT_SIZE(wxDatePickerCtrlGeneric::OnSize)
-    EVT_SET_FOCUS(wxDatePickerCtrlGeneric::OnFocus)
 wxEND_EVENT_TABLE()
 
 #ifndef wxHAS_NATIVE_DATEPICKCTRL
@@ -485,12 +484,4 @@ void wxDatePickerCtrlGeneric::OnText(wxCommandEvent &ev)
     m_popup->SendDateEvent(dt);
 }
 
-
-void wxDatePickerCtrlGeneric::OnFocus(wxFocusEvent& WXUNUSED(event))
-{
-    m_combo->SetFocus();
-}
-
-
 #endif // wxUSE_DATEPICKCTRL
-

src/gtk/window.cpp

@@ -308,6 +308,24 @@ static wxPoint gs_lastGesturePoint;
 // the trace mask used for the focus debugging messages
 #define TRACE_FOCUS wxT("focus")
 
+// Function used to dump a brief description of a window.
+static
+wxString wxDumpWindow(wxWindow* win)
+{
+    if ( !win )
+        return "(no window)";
+
+    wxString s = wxString::Format("%s(%p",
+                                  win->GetClassInfo()->GetClassName(), win);
+
+    wxString label = win->GetLabel();
+    if ( !label.empty() )
+        s += wxString::Format(", \"%s\"", label);
+    s += ")";
+
+    return s;
+}
+
 // A handy function to run from under gdb to show information about the given
 // GtkWidget. Right now it only shows its type, we could enhance it to show
 // more information later but this is already pretty useful.
@@ -4335,8 +4353,8 @@ bool wxWindowGTK::GTKHandleFocusIn()
             // GTK+ focus changed from this wxWindow back to itself, so don't
             // emit any events at all
             wxLogTrace(TRACE_FOCUS,
-                       "filtered out spurious focus change within %s(%p, %s)",
-                       GetClassInfo()->GetClassName(), this, GetLabel());
+                       "filtered out spurious focus change within %s",
+                       wxDumpWindow(this));
             gs_deferredFocusOut = NULL;
             return retval;
         }
@@ -4349,14 +4367,20 @@ bool wxWindowGTK::GTKHandleFocusIn()
 
 
     wxLogTrace(TRACE_FOCUS,
-               "handling focus_in event for %s(%p, %s)",
-               GetClassInfo()->GetClassName(), this, GetLabel());
+               "handling focus_in event for %s",
+               wxDumpWindow(this));
 
     if (m_imContext)
         gtk_im_context_focus_in(m_imContext);
 
     gs_currentFocus = this;
-    gs_pendingFocus = NULL;
+
+    if ( gs_pendingFocus )
+    {
+        wxLogTrace(TRACE_FOCUS, "Resetting pending focus %s on focus set",
+                   wxDumpWindow(gs_pendingFocus));
+        gs_pendingFocus = NULL;
+    }
 
 #if wxUSE_CARET
     // caret needs to be informed about focus change
@@ -4388,6 +4412,15 @@ bool wxWindowGTK::GTKHandleFocusOut()
     // handler issues a repaint
     const bool retval = m_wxwindow ? true : false;
 
+    // If this window is still the pending focus one, reset that pointer as
+    // we're not going to have focus any longer and DoFindFocus() must not
+    // return this window.
+    if ( gs_pendingFocus == this )
+    {
+        wxLogTrace(TRACE_FOCUS, "Resetting pending focus %s on focus loss",
+                   wxDumpWindow(this));
+        gs_pendingFocus = NULL;
+    }
 
     // NB: If a control is composed of several GtkWidgets and when focus
     //     changes from one of them to another within the same wxWindow, we get
@@ -4401,8 +4434,8 @@ bool wxWindowGTK::GTKHandleFocusOut()
         wxASSERT_MSG( gs_deferredFocusOut == NULL,
                       "deferred focus out event already pending" );
         wxLogTrace(TRACE_FOCUS,
-                   "deferring focus_out event for %s(%p, %s)",
-                   GetClassInfo()->GetClassName(), this, GetLabel());
+                   "deferring focus_out event for %s",
+                   wxDumpWindow(this));
         gs_deferredFocusOut = this;
         return retval;
     }
@@ -4415,8 +4448,8 @@ bool wxWindowGTK::GTKHandleFocusOut()
 void wxWindowGTK::GTKHandleFocusOutNoDeferring()
 {
     wxLogTrace(TRACE_FOCUS,
-               "handling focus_out event for %s(%p, %s)",
-               GetClassInfo()->GetClassName(), this, GetLabel());
+               "handling focus_out event for %s",
+               wxDumpWindow(this));
 
     gs_lastFocus = this;
 
@@ -4435,8 +4468,8 @@ void wxWindowGTK::GTKHandleFocusOutNoDeferring()
         // * or it goes to another control, in which case focus-in event will
         //   follow immediately and it will set gs_currentFocus to the right
         //   value
-        wxLogDebug("window %s(%p, %s) lost focus even though it didn't have it",
-                   GetClassInfo()->GetClassName(), this, GetLabel());
+        wxLogDebug("window %s lost focus even though it didn't have it",
+                   wxDumpWindow(this));
     }
     gs_currentFocus = NULL;
 
@@ -4463,8 +4496,8 @@ void wxWindowGTK::GTKHandleDeferredFocusOut()
     gs_deferredFocusOut = NULL;
 
     wxLogTrace(TRACE_FOCUS,
-               "processing deferred focus_out event for %s(%p, %s)",
-               GetClassInfo()->GetClassName(), this, GetLabel());
+               "processing deferred focus_out event for %s",
+               wxDumpWindow(this));
 
     GTKHandleFocusOutNoDeferring();
 }
@@ -4493,15 +4526,15 @@ void wxWindowGTK::SetFocus()
          !gtk_widget_get_can_focus(widget) )
     {
         wxLogTrace(TRACE_FOCUS,
-                   wxT("Setting focus to a child of %s(%p, %s)"),
-                   GetClassInfo()->GetClassName(), this, GetLabel().c_str());
+                   wxT("Setting focus to a child of %s"),
+                   wxDumpWindow(this));
         gtk_widget_child_focus(widget, GTK_DIR_TAB_FORWARD);
     }
     else
     {
         wxLogTrace(TRACE_FOCUS,
-                   wxT("Setting focus to %s(%p, %s)"),
-                   GetClassInfo()->GetClassName(), this, GetLabel().c_str());
+                   wxT("Setting focus to %s"),
+                   wxDumpWindow(this));
         gtk_widget_grab_focus(widget);
     }
 }

tests/controls/datepickerctrltest.cpp

@@ -16,9 +16,11 @@
 
 #ifndef WX_PRECOMP
     #include "wx/app.h"
+    #include "wx/button.h"
 #endif // WX_PRECOMP
 
 #include "wx/datectrl.h"
+#include "wx/uiaction.h"
 
 #include "testableframe.h"
 #include "testdate.h"
@@ -35,12 +37,15 @@ private:
     CPPUNIT_TEST_SUITE( DatePickerCtrlTestCase );
         CPPUNIT_TEST( Value );
         CPPUNIT_TEST( Range );
+        WXUISIM_TEST( Focus );
     CPPUNIT_TEST_SUITE_END();
 
     void Value();
     void Range();
+    void Focus();
 
     wxDatePickerCtrl* m_datepicker;
+    wxButton* m_button;
 
     wxDECLARE_NO_COPY_CLASS(DatePickerCtrlTestCase);
 };
@@ -54,10 +59,12 @@ CPPUNIT_TEST_SUITE_NAMED_REGISTRATION( DatePickerCtrlTestCase, "DatePickerCtrlTe
 void DatePickerCtrlTestCase::setUp()
 {
     m_datepicker = new wxDatePickerCtrl(wxTheApp->GetTopWindow(), wxID_ANY);
+    m_button = NULL;
 }
 
 void DatePickerCtrlTestCase::tearDown()
 {
+    delete m_button;
     delete m_datepicker;
 }
 
@@ -115,4 +122,45 @@ void DatePickerCtrlTestCase::Range()
     CPPUNIT_ASSERT_EQUAL( dtBeforeEnd, m_datepicker->GetValue() );
 }
 
+#if wxUSE_UIACTIONSIMULATOR
+
+static wxPoint GetRectCenter(const wxRect& r)
+{
+    return (r.GetTopRight() + r.GetBottomLeft()) / 2;
+}
+
+void DatePickerCtrlTestCase::Focus()
+{
+    // Create another control just to give focus to it initially.
+    m_button = new wxButton(wxTheApp->GetTopWindow(), wxID_OK);
+    m_button->Move(0, m_datepicker->GetSize().y * 3);
+    m_button->SetFocus();
+    wxYield();
+
+    CHECK( !m_datepicker->HasFocus() );
+
+    EventCounter setFocus(m_datepicker, wxEVT_SET_FOCUS);
+    EventCounter killFocus(m_datepicker, wxEVT_KILL_FOCUS);
+
+    wxUIActionSimulator sim;
+
+    sim.MouseMove(GetRectCenter(m_datepicker->GetScreenRect()));
+    sim.MouseClick();
+    wxYield();
+
+    REQUIRE( m_datepicker->HasFocus() );
+    CHECK( setFocus.GetCount() == 1 );
+    CHECK( killFocus.GetCount() == 0 );
+
+    sim.MouseMove(GetRectCenter(m_button->GetScreenRect()));
+    sim.MouseClick();
+    wxYield();
+
+    CHECK( !m_datepicker->HasFocus() );
+    CHECK( setFocus.GetCount() == 1 );
+    CHECK( killFocus.GetCount() == 1 );
+}
+
+#endif // wxUSE_UIACTIONSIMULATOR
+
 #endif // wxUSE_DATEPICKCTRL