From 56c419116838045f23f046112960d4e42f98f80d Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 20 Oct 2018 18:44:18 +0200 Subject: [PATCH] Reimplement wxPopupWindow as a WS_POPUP window under MSW Don't use the child window of the desktop window for popup windows under MSW, while this worked in simplest cases, it didn't allow having functional controls inside a wxPopupWindow as e.g. wxTextCtrl didn't accept input it at all if created as a child of such window. Instead, switch to using a top-level window, with WS_POPUP style, and fix the problem with the loss of activation by explicitly pretending to still be active in the owner window when losing activation to our own popup (thanks to Barmak Shemirani for providing this solution). Also use an MSW-specific and much simpler implementation of detecting when the popup should be dismissed in wxPopupTransientWindow: instead of capturing mouse or tracking focus, just react to activation loss directly. Add a wxTextCtrl to the popup in samples/popup to show that editing it works now. --- include/wx/msw/popupwin.h | 15 +++--- include/wx/popupwin.h | 88 ++++++++++++++++++++++++-------- samples/popup/popup.cpp | 2 + src/common/popupcmn.cpp | 85 ++++++++++++++----------------- src/msw/popupwin.cpp | 103 ++++++++++++++++++++++++++------------ src/msw/window.cpp | 22 ++++++++ 6 files changed, 206 insertions(+), 109 deletions(-) diff --git a/include/wx/msw/popupwin.h b/include/wx/msw/popupwin.h index 5ac4a09c03..d41dda58e5 100644 --- a/include/wx/msw/popupwin.h +++ b/include/wx/msw/popupwin.h @@ -18,25 +18,28 @@ class WXDLLIMPEXP_CORE wxPopupWindow : public wxPopupWindowBase { public: - wxPopupWindow() { } + wxPopupWindow() { m_owner = NULL; } wxPopupWindow(wxWindow *parent, int flags = wxBORDER_NONE) { (void)Create(parent, flags); } bool Create(wxWindow *parent, int flags = wxBORDER_NONE); - virtual void SetFocus() wxOVERRIDE; virtual bool Show(bool show = true) wxOVERRIDE; // return the style to be used for the popup windows virtual WXDWORD MSWGetStyle(long flags, WXDWORD *exstyle) const wxOVERRIDE; - // get the HWND to be used as parent of this window with CreateWindow() - virtual WXHWND MSWGetParent() const wxOVERRIDE; -protected: + // Implementation only from now on. + + // Return the top level window parent of this popup or null. + wxWindow* MSWGetOwner() const { return m_owner; } + +private: + wxWindow* m_owner; + wxDECLARE_DYNAMIC_CLASS_NO_COPY(wxPopupWindow); }; #endif // _WX_MSW_POPUPWIN_H_ - diff --git a/include/wx/popupwin.h b/include/wx/popupwin.h index ea6a4427a4..3d20bf4a7b 100644 --- a/include/wx/popupwin.h +++ b/include/wx/popupwin.h @@ -76,24 +76,16 @@ public: // when the user clicks mouse outside it or if it loses focus in any other way // ---------------------------------------------------------------------------- -class WXDLLIMPEXP_FWD_CORE wxPopupWindowHandler; -class WXDLLIMPEXP_FWD_CORE wxPopupFocusHandler; - -class WXDLLIMPEXP_CORE wxPopupTransientWindow : public wxPopupWindow +// Define the public API of wxPopupTransientWindow: +class WXDLLIMPEXP_CORE wxPopupTransientWindowBase : public wxPopupWindow { public: - // ctors - wxPopupTransientWindow() { Init(); } - wxPopupTransientWindow(wxWindow *parent, int style = wxBORDER_NONE); - - virtual ~wxPopupTransientWindow(); - // popup the window (this will show it too) and keep focus at winFocus // (or itself if it's NULL), dismiss the popup if we lose focus - virtual void Popup(wxWindow *focus = NULL); + virtual void Popup(wxWindow *focus = NULL) = 0; // hide the window - virtual void Dismiss(); + virtual void Dismiss() = 0; // can the window be dismissed now? // @@ -104,24 +96,74 @@ public: // called when a mouse is pressed while the popup is shown: return true // from here to prevent its normal processing by the popup (which consists // in dismissing it if the mouse is clicked outside it) - virtual bool ProcessLeftDown(wxMouseEvent& event); - - // Overridden to grab the input on some plaforms - virtual bool Show( bool show = true ) wxOVERRIDE; + virtual bool ProcessLeftDown(wxMouseEvent& WXUNUSED(event)) + { return false; } // Override to implement delayed destruction of this window. virtual bool Destroy() wxOVERRIDE; protected: - // common part of all ctors - void Init(); - // this is called when the popup is disappeared because of anything // else but direct call to Dismiss() - virtual void OnDismiss(); + virtual void OnDismiss() { } // dismiss and notify the derived class - void DismissAndNotify(); + void DismissAndNotify() + { + Dismiss(); + OnDismiss(); + } +}; + +#ifdef __WXMSW__ + +class WXDLLIMPEXP_CORE wxPopupTransientWindow : public wxPopupTransientWindowBase +{ +public: + // ctors + wxPopupTransientWindow() { } + wxPopupTransientWindow(wxWindow *parent, int style = wxBORDER_NONE) + { Create(parent, style); } + + // Implement base class pure virtuals. + virtual void Popup(wxWindow *focus = NULL) wxOVERRIDE; + virtual void Dismiss() wxOVERRIDE; + + // Override to handle WM_NCACTIVATE. + virtual bool MSWHandleMessage(WXLRESULT *result, + WXUINT message, + WXWPARAM wParam, + WXLPARAM lParam) wxOVERRIDE; + +private: + wxDECLARE_DYNAMIC_CLASS(wxPopupTransientWindow); + wxDECLARE_NO_COPY_CLASS(wxPopupTransientWindow); +}; + +#else // !__WXMSW__ + +class WXDLLIMPEXP_FWD_CORE wxPopupWindowHandler; +class WXDLLIMPEXP_FWD_CORE wxPopupFocusHandler; + +class WXDLLIMPEXP_CORE wxPopupTransientWindow : public wxPopupTransientWindowBase +{ +public: + // ctors + wxPopupTransientWindow() { Init(); } + wxPopupTransientWindow(wxWindow *parent, int style = wxBORDER_NONE); + + virtual ~wxPopupTransientWindow(); + + // Implement base class pure virtuals. + virtual void Popup(wxWindow *focus = NULL) wxOVERRIDE; + virtual void Dismiss() wxOVERRIDE; + + // Overridden to grab the input on some plaforms + virtual bool Show( bool show = true ) wxOVERRIDE; + +protected: + // common part of all ctors + void Init(); // remove our event handlers void PopHandlers(); @@ -129,7 +171,7 @@ protected: // get alerted when child gets deleted from under us void OnDestroy(wxWindowDestroyEvent& event); -#if defined(__WXMSW__) ||(defined(__WXMAC__) && wxOSX_USE_COCOA_OR_CARBON) +#if defined(__WXMAC__) && wxOSX_USE_COCOA_OR_CARBON // Check if the mouse needs to be captured or released: we must release // when it's inside our window if we want the embedded controls to work. void OnIdle(wxIdleEvent& event); @@ -154,6 +196,8 @@ protected: wxDECLARE_NO_COPY_CLASS(wxPopupTransientWindow); }; +#endif // __WXMSW__/!__WXMSW__ + #if wxUSE_COMBOBOX && defined(__WXUNIVERSAL__) // ---------------------------------------------------------------------------- diff --git a/samples/popup/popup.cpp b/samples/popup/popup.cpp index 0d1bc729e2..1e0cf61912 100644 --- a/samples/popup/popup.cpp +++ b/samples/popup/popup.cpp @@ -135,6 +135,8 @@ SimpleTransientPopup::SimpleTransientPopup( wxWindow *parent, bool scrolled ) topSizer->Add( text, 0, wxALL, 5 ); topSizer->Add( m_button, 0, wxALL, 5 ); topSizer->Add( m_spinCtrl, 0, wxALL, 5 ); + topSizer->Add( new wxTextCtrl(m_panel, wxID_ANY, "Try to type here"), + 0, wxEXPAND|wxALL, 5 ); topSizer->Add( m_mouseText, 0, wxCENTRE|wxALL, 5 ); if ( scrolled ) diff --git a/src/common/popupcmn.cpp b/src/common/popupcmn.cpp index e69ad0d2ad..19c16e4fb5 100644 --- a/src/common/popupcmn.cpp +++ b/src/common/popupcmn.cpp @@ -46,8 +46,6 @@ #elif defined(__WXGTK__) #include #define gtk_widget_get_window(x) x->window -#elif defined(__WXMSW__) - #include "wx/msw/private.h" #elif defined(__WXX11__) #include "wx/x11/private.h" #endif @@ -59,6 +57,8 @@ wxIMPLEMENT_DYNAMIC_CLASS(wxPopupTransientWindow, wxPopupWindow); wxIMPLEMENT_DYNAMIC_CLASS(wxPopupComboWindow, wxPopupTransientWindow); #endif +#ifndef __WXMSW__ + // ---------------------------------------------------------------------------- // private classes // ---------------------------------------------------------------------------- @@ -113,11 +113,13 @@ wxBEGIN_EVENT_TABLE(wxPopupFocusHandler, wxEvtHandler) wxEND_EVENT_TABLE() wxBEGIN_EVENT_TABLE(wxPopupTransientWindow, wxPopupWindow) -#if defined(__WXMSW__) || (defined(__WXMAC__) && wxOSX_USE_COCOA_OR_CARBON) +#if defined(__WXMAC__) && wxOSX_USE_COCOA_OR_CARBON EVT_IDLE(wxPopupTransientWindow::OnIdle) #endif wxEND_EVENT_TABLE() +#endif // !__WXMSW__ + // ============================================================================ // implementation // ============================================================================ @@ -201,6 +203,27 @@ void wxPopupWindowBase::Position(const wxPoint& ptOrigin, Move(x, y, wxSIZE_NO_ADJUSTMENTS); } +// ---------------------------------------------------------------------------- +// wxPopupTransientWindowBase +// ---------------------------------------------------------------------------- + +bool wxPopupTransientWindowBase::Destroy() +{ + // The popup window can be deleted at any moment, even while some events + // are still being processed for it, so delay its real destruction until + // the next idle time when we're sure that it's safe to really destroy it. + + wxCHECK_MSG( !wxPendingDelete.Member(this), false, + wxS("Shouldn't destroy the popup twice.") ); + + wxPendingDelete.Append(this); + + return true; +} + +// MSW implementation is in platform-specific src/msw/popupwin.cpp. +#ifndef __WXMSW__ + // ---------------------------------------------------------------------------- // wxPopupTransientWindow // ---------------------------------------------------------------------------- @@ -292,18 +315,10 @@ void wxPopupTransientWindow::Popup(wxWindow *winFocus) m_child->PushEventHandler(m_handlerPopup); -#if defined(__WXMSW__) - // Focusing on child of popup window does not work on MSW unless WS_POPUP - // style is set. We do not even want to try to set the focus, as it may - // provoke errors on some Windows versions (Vista and later). - if ( ::GetWindowLong(GetHwnd(), GWL_STYLE) & WS_POPUP ) -#endif - { - m_focus = winFocus ? winFocus : this; - m_focus->SetFocus(); - } + m_focus = winFocus ? winFocus : this; + m_focus->SetFocus(); -#if defined( __WXMSW__ ) || (defined( __WXMAC__) && wxOSX_USE_COCOA_OR_CARBON) +#if defined( __WXMAC__) && wxOSX_USE_COCOA_OR_CARBON // MSW doesn't allow to set focus to the popup window, but we need to // subclass the window which has the focus, and not winFocus passed in or // otherwise everything else breaks down @@ -354,7 +369,7 @@ bool wxPopupTransientWindow::Show( bool show ) } #endif -#if defined( __WXMSW__ ) || defined( __WXMAC__) +#if defined( __WXMAC__) if (!show && m_child && m_child->HasCapture()) { m_child->ReleaseMouse(); @@ -414,7 +429,7 @@ bool wxPopupTransientWindow::Show( bool show ) } #endif -#if defined( __WXMSW__ ) || defined( __WXMAC__) +#if defined( __WXMAC__) if (show && m_child) { // Assume that the mouse is outside the popup to begin with @@ -425,44 +440,13 @@ bool wxPopupTransientWindow::Show( bool show ) return ret; } -bool wxPopupTransientWindow::Destroy() -{ - // The popup window can be deleted at any moment, even while some events - // are still being processed for it, so delay its real destruction until - // the next idle time when we're sure that it's safe to really destroy it. - - wxCHECK_MSG( !wxPendingDelete.Member(this), false, - wxS("Shouldn't destroy the popup twice.") ); - - wxPendingDelete.Append(this); - - return true; -} - void wxPopupTransientWindow::Dismiss() { Hide(); PopHandlers(); } -void wxPopupTransientWindow::DismissAndNotify() -{ - Dismiss(); - OnDismiss(); -} - -void wxPopupTransientWindow::OnDismiss() -{ - // nothing to do here - but it may be interesting for derived class -} - -bool wxPopupTransientWindow::ProcessLeftDown(wxMouseEvent& WXUNUSED(event)) -{ - // no special processing here - return false; -} - -#if defined(__WXMSW__) ||(defined(__WXMAC__) && wxOSX_USE_COCOA_OR_CARBON) +#if defined(__WXMAC__) && wxOSX_USE_COCOA_OR_CARBON void wxPopupTransientWindow::OnIdle(wxIdleEvent& event) { event.Skip(); @@ -500,6 +484,7 @@ void wxPopupTransientWindow::OnIdle(wxIdleEvent& event) } #endif // wxOSX/Carbon +#endif // !__WXMSW__ #if wxUSE_COMBOBOX && defined(__WXUNIVERSAL__) @@ -555,6 +540,8 @@ void wxPopupComboWindow::OnKeyDown(wxKeyEvent& event) #endif // wxUSE_COMBOBOX && defined(__WXUNIVERSAL__) +#ifndef __WXMSW__ + // ---------------------------------------------------------------------------- // wxPopupWindowHandler // ---------------------------------------------------------------------------- @@ -696,4 +683,6 @@ void wxPopupFocusHandler::OnChar(wxKeyEvent& event) } } +#endif // !__WXMSW__ + #endif // wxUSE_POPUPWIN diff --git a/src/msw/popupwin.cpp b/src/msw/popupwin.cpp index f1ed6258ba..eea9c444bc 100644 --- a/src/msw/popupwin.cpp +++ b/src/msw/popupwin.cpp @@ -32,19 +32,28 @@ #include "wx/msw/private.h" // for GetDesktopWindow() +// Set to the popup window currently being shown, if any. +extern wxPopupWindow* wxCurrentPopupWindow = NULL; + // ============================================================================ // implementation // ============================================================================ +// ---------------------------------------------------------------------------- +// wxPopupWindow +// ---------------------------------------------------------------------------- + bool wxPopupWindow::Create(wxWindow *parent, int flags) { // popup windows are created hidden by default Hide(); + m_owner = wxGetTopLevelParent(parent); + return wxPopupWindowBase::Create(parent) && wxWindow::Create(parent, wxID_ANY, wxDefaultPosition, wxDefaultSize, - flags | wxPOPUP_WINDOW); + flags); } WXDWORD wxPopupWindow::MSWGetStyle(long flags, WXDWORD *exstyle) const @@ -52,6 +61,14 @@ WXDWORD wxPopupWindow::MSWGetStyle(long flags, WXDWORD *exstyle) const // we only honour the border flags, the others don't make sense for us WXDWORD style = wxWindow::MSWGetStyle(flags & wxBORDER_MASK, exstyle); + // We need to be a popup (i.e. not a child) window in order to not be + // confined to the parent window area, as is required for a drop down, for + // example. Old implementation used WS_CHILD and made this window a child + // of the desktop window, but this resulted in problems with handling input + // in the popup children, and so was changed to the current version. + style &= ~WS_CHILD; + style |= WS_POPUP; + if ( exstyle ) { // a popup window floats on top of everything @@ -61,45 +78,65 @@ WXDWORD wxPopupWindow::MSWGetStyle(long flags, WXDWORD *exstyle) const return style; } -WXHWND wxPopupWindow::MSWGetParent() const -{ - // we must be a child of the desktop to be able to extend beyond the parent - // window client area (like the comboboxes drop downs do) - // - // NB: alternative implementation would be to use WS_POPUP instead of - // WS_CHILD but then showing a popup would deactivate the parent which - // is ugly and working around this, although possible, is even more - // ugly - return (WXHWND)::GetDesktopWindow(); -} - -void wxPopupWindow::SetFocus() -{ - // Focusing on a popup window does not work on MSW unless WS_POPUP style is - // set (which is never the case currently, see the note in MSWGetParent()). - // We do not even want to try to set the focus, as it returns an error from - // SetFocus() on recent Windows versions (since Vista) and the resulting - // debug message is annoying. -} - bool wxPopupWindow::Show(bool show) { - if ( !wxWindowMSW::Show(show) ) - return false; + // 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; - if ( show ) + return wxPopupWindowBase::Show(show); +} + +// ---------------------------------------------------------------------------- +// wxPopupTransientWindow +// ---------------------------------------------------------------------------- + +void wxPopupTransientWindow::Popup(wxWindow* focus) +{ + Show(); + + if ( focus ) + focus->SetFocus(); +} + +void wxPopupTransientWindow::Dismiss() +{ + Hide(); +} + +bool +wxPopupTransientWindow::MSWHandleMessage(WXLRESULT *result, + WXUINT message, + WXWPARAM wParam, + WXLPARAM lParam) +{ + switch ( message ) { - // raise to top of z order - if (!::SetWindowPos(GetHwnd(), HWND_TOP, 0, 0, 0, 0, SWP_NOMOVE | SWP_NOSIZE)) - { - wxLogLastError(wxT("SetWindowPos")); - } + case WM_NCACTIVATE: + if ( !wParam ) + { + // Hide the window automatically when it loses activation. + Dismiss(); - // and set it as the foreground window so the mouse can be captured - ::SetForegroundWindow(GetHwnd()); + // 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); + } + } + } + break; } - return true; + return wxPopupTransientWindowBase::MSWHandleMessage(result, message, + wParam, lParam); } #endif // #if wxUSE_POPUPWIN diff --git a/src/msw/window.cpp b/src/msw/window.cpp index 83add1d1d8..d17cf3f39e 100644 --- a/src/msw/window.cpp +++ b/src/msw/window.cpp @@ -60,6 +60,7 @@ #include "wx/hashmap.h" #include "wx/evtloop.h" +#include "wx/popupwin.h" #include "wx/power.h" #include "wx/scopeguard.h" #include "wx/sysopt.h" @@ -134,6 +135,10 @@ extern wxMenu *wxCurrentPopupMenu; #endif +#if wxUSE_POPUPWIN +extern wxPopupWindow* wxCurrentPopupWindow; +#endif // wxUSE_POPUPWIN + #if wxUSE_UXTHEME // This is a hack used by the owner-drawn wxButton implementation to ensure // that the brush used for erasing its background is correctly aligned with the @@ -3677,6 +3682,23 @@ wxWindowMSW::MSWHandleMessage(WXLRESULT *result, } break; + case WM_NCACTIVATE: + // When we're losing activation to our own popup window, we want to + // retain the "active" appearance of the title bar, as dropping + // down a combobox popup shouldn't deactivate the window containing + // the combobox, for example. Explicitly calling DefWindowProc() to + // draw the window as active seems to be the only way of achieving + // this (thanks to Barmak Shemirani for suggesting it at + // https://stackoverflow.com/a/52808753/15275). + if ( !wParam && + wxCurrentPopupWindow && + wxCurrentPopupWindow->MSWGetOwner() == this ) + { + rc.result = MSWDefWindowProc(message, TRUE, lParam); + processed = true; + } + break; + #if wxUSE_UXTHEME // If we want the default themed border then we need to draw it ourselves case WM_NCCALCSIZE: