From 214381c0cd8c17e2119fecd65d4da1a73d427eff Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 19 Aug 2021 18:10:36 +0100 Subject: [PATCH] Fix bug with having multiple default buttons in wxMSW dialogs We need to reset the default button using DM_SETDEFID too, otherwise calling DM_SETDEFID later, when setting the new default button, seems to restore BS_DEFPUSHBUTTON on the previous default button (but only if its ID is positive, which probably explains why this bug went unnoticed for so long), resulting in having 2 buttons with BS_DEFPUSHBUTTON in the dialog. Closes #19245. This commit is best viewed ignoring whitespace-only changes. --- src/msw/button.cpp | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/src/msw/button.cpp b/src/msw/button.cpp index 42408a4a32..b8bb295d6e 100644 --- a/src/msw/button.cpp +++ b/src/msw/button.cpp @@ -222,8 +222,8 @@ wxSize wxButtonBase::GetDefaultSize(wxWindow* win) and the current control doesn't process Enter itself somehow. This is handled by ::DefWindowProc() (or maybe ::DefDialogProc()) using DM_SETDEFID Another aspect of "defaultness" is that the default button has different - appearance: this is due to BS_DEFPUSHBUTTON style which is completely - separate from DM_SETDEFID stuff (!). Also note that BS_DEFPUSHBUTTON should + appearance: this is due to BS_DEFPUSHBUTTON style which is only partially + handled by using DM_SETDEFID. Also note that BS_DEFPUSHBUTTON should be unset if our parent window is not active so it should be unset whenever we lose activation and set back when we regain it. @@ -330,22 +330,21 @@ wxButton::SetDefaultStyle(wxButton *btn, bool on) if ( !btn ) return; + // we shouldn't set BS_DEFPUSHBUTTON for any button if we don't have + // focus at all any more + if ( on && !wxTheApp->IsActive() ) + return; + // first, let DefDlgProc() know about the new default button - if ( on ) - { - // we shouldn't set BS_DEFPUSHBUTTON for any button if we don't have - // focus at all any more - if ( !wxTheApp->IsActive() ) - return; + wxWindow * const tlw = wxGetTopLevelParent(btn); + wxCHECK_RET( tlw, wxT("button without top level window?") ); - wxWindow * const tlw = wxGetTopLevelParent(btn); - wxCHECK_RET( tlw, wxT("button without top level window?") ); - - ::SendMessage(GetHwndOf(tlw), DM_SETDEFID, btn->GetId(), 0L); - - // sending DM_SETDEFID also changes the button style to - // BS_DEFPUSHBUTTON so there is nothing more to do - } + // passing -1 to indicate absence of the default button is not documented + // as being supported, but we need to pass something to DM_SETDEFID when + // resetting the default button it in order to prevent DefDlgProc() from + // restoring BS_DEFPUSHBUTTON on it later, see #19245, and -1 shouldn't + // conflict with anything, as it can never be a valid ID + ::SendMessage(GetHwndOf(tlw), DM_SETDEFID, on ? btn->GetId() : -1, 0L); // then also change the style as needed long style = ::GetWindowLong(GetHwndOf(btn), GWL_STYLE);