From 58d4b0e209dad0e6ced56585e74102ab09c930df Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 30 Mar 2019 04:11:46 +0100 Subject: [PATCH] Fix for showing multiple popups in a row in wxMSW There were at least 2 problems when showing a transient popup while another one was already shown in wxMSW, as could be seen by showing several wxRichToolTips in a row from a timer event handler, for example: First problem was that wxCurrentPopupWindow was incorrectly reset in this case by the old popup window when it was hidden, even though it should have been remaining set to the new popup window. Fix this by checking that we only reset wxCurrentPopupWindow when hiding the popup if it still points to this popup, but not if it has been changed to point to another one in the meanwhile. Second problem was more mysterious and resulted in simply not receiving the activation events for the new popup when showing it resulted in hiding the previous one. The working hypothesis is that hiding a window, which changes activation on its own, from WM_NCACTIVATE handler in our code confused ShowWindow(), which handles switching activation, which didn't expect this to happen, so the fix is to avoid doing anything immediately from this handler and wait until the next idle event to do it instead. These fixes ensure that showing several popups in a row works correctly, i.e. hides the previous popup when a new one is shown and also keeps the parent window appearing active during all the time and deactivates it if the focus switches to another top level window at the end. --- include/wx/popupwin.h | 2 ++ src/msw/popupwin.cpp | 70 ++++++++++++++++++++++++++++++++----------- 2 files changed, 54 insertions(+), 18 deletions(-) diff --git a/include/wx/popupwin.h b/include/wx/popupwin.h index e8c0dd9242..6735d2593c 100644 --- a/include/wx/popupwin.h +++ b/include/wx/popupwin.h @@ -152,6 +152,8 @@ public: WXLPARAM lParam) wxOVERRIDE; private: + void DismissOnDeactivate(WXHWND hwndActive); + wxDECLARE_DYNAMIC_CLASS(wxPopupTransientWindow); wxDECLARE_NO_COPY_CLASS(wxPopupTransientWindow); }; diff --git a/src/msw/popupwin.cpp b/src/msw/popupwin.cpp index 525fd29469..ab6cdf6b9c 100644 --- a/src/msw/popupwin.cpp +++ b/src/msw/popupwin.cpp @@ -119,10 +119,32 @@ void wxPopupWindow::SetFocus() bool wxPopupWindow::Show(bool show) { + // Note that we're called from the ctor before the window is actually + // created to hide the popup initially. This call doesn't really hide the + // window, so don't do anything in this case, in particular don't change + // wxCurrentPopupWindow value. + if ( !GetHwnd() ) + return wxPopupWindowBase::Show(show); + // It's important to update wxCurrentPopupWindow before showing the window, - // to ensure that it already set by the time the owner gets WM_NCACTIVATE - // from inside Show() so that it knows to remain [appearing] active. - wxCurrentPopupWindow = show ? this : NULL; + // to ensure that it's already set by the time the owner gets WM_NCACTIVATE + // from inside Show() so that it knows to remain [appearing] active, see + // the WM_NCACTIVATE handler in wxWindow::MSWHandleMessage(). + if ( show ) + { + // There could have been a previous popup window which hasn't been + // hidden yet. This will happen now, when we show this one, as it will + // result in activation loss for the other one, so it's ok to overwrite + // the old pointer, even if it's non-NULL. + wxCurrentPopupWindow = this; + } + else + { + // Only reset the pointer if it points to this window, otherwise we + // would lose the correct value in the situation described above. + if ( wxCurrentPopupWindow == this ) + wxCurrentPopupWindow = NULL; + } if ( HasFlag(wxPU_CONTAINS_CONTROLS) ) { @@ -170,6 +192,24 @@ void wxPopupTransientWindow::Dismiss() Hide(); } +void wxPopupTransientWindow::DismissOnDeactivate(WXHWND hwndActive) +{ + // Hide the window automatically when it loses activation. + Dismiss(); + + // Activation might have gone to a different window or maybe + // even a different application, don't let our owner continue + // to appear active in this case. + wxWindow* const owner = MSWGetOwner(); + if ( owner ) + { + if ( hwndActive != GetHwndOf(owner) ) + { + ::SendMessage(GetHwndOf(owner), WM_NCACTIVATE, FALSE, 0); + } + } +} + bool wxPopupTransientWindow::MSWHandleMessage(WXLRESULT *result, WXUINT message, @@ -181,21 +221,15 @@ wxPopupTransientWindow::MSWHandleMessage(WXLRESULT *result, case WM_NCACTIVATE: if ( !wParam ) { - // Hide the window automatically when it loses activation. - Dismiss(); - - // Activation might have gone to a different window or maybe - // even a different application, don't let our owner continue - // to appear active in this case. - wxWindow* const owner = MSWGetOwner(); - if ( owner ) - { - const HWND hwndActive = ::GetActiveWindow(); - if ( hwndActive != GetHwndOf(owner) ) - { - ::SendMessage(GetHwndOf(owner), WM_NCACTIVATE, FALSE, 0); - } - } + // We need to dismiss this window, however doing it directly + // from here seems to confuse ::ShowWindow(), which ends up + // calling this handler, and may result in losing activation + // entirely, so postpone it slightly. + // + // Note that we do use the currently active window, just in + // case it changes between now and the next idle event. + CallAfter(&wxPopupTransientWindow::DismissOnDeactivate, + ::GetActiveWindow()); } break; }