Fix bug with wxRadioButton state changing unexpectedly in wxMSW

In wxMSW, a focused wxRadioButton is always checked, which meant that
checking a wxRadioButton while focus was not in the window containing it
and later giving the focus to that window could uncheck it by giving
focus to another wxRadioButton that had had it previously.

Fix this by adding WXSetPendingFocus() to wxMSW wxWindow and calling it
from wxRadioButton::SetValue() to ensure that when the focus is
regained, it goes to the newly checked radio button and not some other
one.

This replaces the previously used, for the same purpose, wxMSW-specific
wxTopLevelWindow::SetLastFocus(), so while this solution is not exactly
pretty, it's not worse than we had before, while being more generic.

Also add a unit test checking that things work correctly in the scenario
described above.

Closes https://github.com/wxWidgets/wxWidgets/pull/1257

Closes #18341.

include/wx/containr.h

@@ -266,6 +266,12 @@ public:
     {
         return m_container.HasTransparentBackground();
     }
+
+    WXDLLIMPEXP_INLINE_CORE
+    virtual void WXDoUpdatePendingFocus(wxWindow* win) wxOVERRIDE
+    {
+        return m_container.SetLastFocus(win);
+    }
 #endif // __WXMSW__
 
 protected:

include/wx/msw/toplevel.h

@@ -100,9 +100,12 @@ public:
     // event handlers
     void OnActivate(wxActivateEvent& event);
 
-    // called by wxWindow whenever it gets focus
+    // called from wxWidgets code itself only when the pending focus, i.e. the
-    void SetLastFocus(wxWindow *win) { m_winLastFocused = win; }
+    // element which should get focus when this TLW is activated again, changes
-    wxWindow *GetLastFocus() const { return m_winLastFocused; }
+    virtual void WXDoUpdatePendingFocus(wxWindow* win) wxOVERRIDE
+    {
+        m_winLastFocused = win;
+    }
 
     // translate wxWidgets flags to Windows ones
     virtual WXDWORD MSWGetStyle(long flags, WXDWORD *exstyle) const wxOVERRIDE;

include/wx/msw/window.h

@@ -576,6 +576,14 @@ public:
     // Return true if the button was clicked, false otherwise.
     static bool MSWClickButtonIfPossible(wxButton* btn);
 
+    // This method is used for handling wxRadioButton-related complications,
+    // see wxRadioButton::SetValue(). It calls WXDoUpdatePendingFocus() for
+    // this window and all its parents up to the enclosing TLW, recursively.
+    void WXSetPendingFocus(wxWindow* win);
+
+    // Should be overridden by all classes storing the "last focused" window.
+    virtual void WXDoUpdatePendingFocus(wxWindow* WXUNUSED(win)) {}
+
 protected:
     // this allows you to implement standard control borders without
     // repeating the code in different classes that are not derived from

src/msw/radiobut.cpp

@@ -103,24 +103,15 @@ void wxRadioButton::SetValue(bool value)
     // reselected automatically, if a parent window loses the focus and regains
     // it.
     wxWindow * const focus = FindFocus();
-    wxTopLevelWindow * const
-        tlw = wxDynamicCast(wxGetTopLevelParent(this), wxTopLevelWindow);
-    wxCHECK_RET( tlw, wxT("radio button outside of TLW?") );
-    wxWindow * const focusInTLW = tlw->GetLastFocus();
 
     const wxWindowList& siblings = GetParent()->GetChildren();
     wxWindowList::compatibility_iterator nodeThis = siblings.Find(this);
     wxCHECK_RET( nodeThis, wxT("radio button not a child of its parent?") );
 
-    // this will be set to true in the code below if the focus is in our TLW
+    // this will be set to true in the code below if the focus belongs to one
-    // and belongs to one of the other buttons in the same group
+    // of the other buttons in the same group
     bool shouldSetFocus = false;
 
-    // this will be set to true if the focus is outside of our TLW currently
-    // but the remembered focus of this TLW is one of the other buttons in the
-    // same group
-    bool shouldSetTLWFocus = false;
-
     // if it's not the first item of the group ...
     if ( !HasFlag(wxRB_GROUP) )
     {
@@ -146,8 +137,6 @@ void wxRadioButton::SetValue(bool value)
 
             if ( btn == focus )
                 shouldSetFocus = true;
-            else if ( btn == focusInTLW )
-                shouldSetTLWFocus = true;
 
             btn->SetValue(false);
 
@@ -179,16 +168,25 @@ void wxRadioButton::SetValue(bool value)
 
         if ( btn == focus )
             shouldSetFocus = true;
-        else if ( btn == focusInTLW )
-            shouldSetTLWFocus = true;
 
         btn->SetValue(false);
     }
 
     if ( shouldSetFocus )
+    {
+        // Change focus immediately, we can't do anything else in this case as
+        // leaving it to the other radio button would put it in an impossible
+        // state: a radio button can't have focus and be unchecked.
         SetFocus();
-    else if ( shouldSetTLWFocus )
+    }
-        tlw->SetLastFocus(this);
+    else
+    {
+        // Don't change focus right now, this could be unexpected, but do
+        // ensure that when our parent regains focus, it goes to this button
+        // and not another one, which would result in this one losing its
+        // checked status.
+        GetParent()->WXSetPendingFocus(this);
+    }
 }
 
 bool wxRadioButton::GetValue() const

src/msw/window.cpp

@@ -1026,6 +1026,17 @@ void wxWindowMSW::MSWUpdateUIState(int action, int state)
     ::SendMessage(GetHwnd(), WM_CHANGEUISTATE, MAKEWPARAM(action, state), 0);
 }
 
+void wxWindowMSW::WXSetPendingFocus(wxWindow* win)
+{
+    for ( wxWindow* parent = this; parent; parent = parent->GetParent() )
+    {
+        parent->WXDoUpdatePendingFocus(win);
+
+        if ( parent->IsTopLevel() )
+            break;
+    }
+}
+
 // ---------------------------------------------------------------------------
 // scrolling stuff
 // ---------------------------------------------------------------------------

tests/controls/radiobuttontest.cpp

@@ -16,7 +16,10 @@
 
 #ifndef WX_PRECOMP
     #include "wx/app.h"
+    #include "wx/button.h"
+    #include "wx/panel.h"
     #include "wx/radiobut.h"
+    #include "wx/sizer.h"
     #include "wx/stattext.h"
 #endif // WX_PRECOMP
 
@@ -195,4 +198,58 @@ void RadioButtonTestCase::Single()
     CHECK(ngradio->GetValue());
 }
 
+TEST_CASE("wxRadioButton::Focus", "[radiobutton][focus]")
+{
+    // Create a container panel just to be able to destroy all the windows
+    // created here at once by simply destroying it.
+    wxWindow* const tlw = wxTheApp->GetTopWindow();
+    wxScopedPtr<wxPanel> parentPanel(new wxPanel(tlw));
+
+    // Create a panel containing 2 radio buttons and another control outside
+    // this panel, so that we could give focus to something different and then
+    // return it back to the panel.
+    wxPanel* const radioPanel = new wxPanel(parentPanel.get());
+    wxRadioButton* const radio1 = new wxRadioButton(radioPanel, wxID_ANY, "1");
+    wxRadioButton* const radio2 = new wxRadioButton(radioPanel, wxID_ANY, "2");
+    wxSizer* const radioSizer = new wxBoxSizer(wxHORIZONTAL);
+    radioSizer->Add(radio1);
+    radioSizer->Add(radio2);
+    radioPanel->SetSizer(radioSizer);
+
+    wxButton* const dummyButton = new wxButton(parentPanel.get(), wxID_OK);
+
+    wxSizer* const sizer = new wxBoxSizer(wxVERTICAL);
+    sizer->Add(radioPanel, wxSizerFlags(1).Expand());
+    sizer->Add(dummyButton, wxSizerFlags().Expand());
+    parentPanel->SetSizer(sizer);
+
+    parentPanel->SetSize(tlw->GetClientSize());
+    parentPanel->Layout();
+
+    // Initially the first radio button should be checked.
+    radio1->SetFocus();
+    CHECK(radio1->GetValue());
+    CHECK(wxWindow::FindFocus() == radio1);
+
+    // Switching focus from it shouldn't change this.
+    dummyButton->SetFocus();
+    CHECK(radio1->GetValue());
+
+    // Checking another radio button should make it checked and uncheck the
+    // first one.
+    radio2->SetValue(true);
+    CHECK(!radio1->GetValue());
+    CHECK(radio2->GetValue());
+
+    // While not changing focus.
+    CHECK(wxWindow::FindFocus() == dummyButton);
+
+    // And giving the focus to the panel shouldn't change radio button
+    // selection.
+    radioPanel->SetFocus();
+    CHECK(wxWindow::FindFocus() == radio2);
+    CHECK(!radio1->GetValue());
+    CHECK(radio2->GetValue());
+}
+
 #endif //wxUSE_RADIOBTN