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

@@ -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_
-

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;
+// Define the public API of wxPopupTransientWindow:
-class WXDLLIMPEXP_FWD_CORE wxPopupFocusHandler;
+class WXDLLIMPEXP_CORE wxPopupTransientWindowBase : public wxPopupWindow
-
-class WXDLLIMPEXP_CORE wxPopupTransientWindow : 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);
+    virtual bool ProcessLeftDown(wxMouseEvent& WXUNUSED(event))
-
+        { return false; }
-    // Overridden to grab the input on some plaforms
-    virtual bool Show( bool show = true ) wxOVERRIDE;
 
     // 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__)
 
 // ----------------------------------------------------------------------------

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 )

src/common/popupcmn.cpp

@@ -46,8 +46,6 @@
 #elif defined(__WXGTK__)
     #include <gtk/gtk.h>
     #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__)
+    m_focus = winFocus ? winFocus : this;
-    // Focusing on child of popup window does not work on MSW unless WS_POPUP
+    m_focus->SetFocus();
-    // 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();
-    }
 
-#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()
+#if defined(__WXMAC__) && wxOSX_USE_COCOA_OR_CARBON
-{
-    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)
 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

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) )
+    // It's important to update wxCurrentPopupWindow before showing the window,
-        return false;
+    // 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
+        case WM_NCACTIVATE:
-        if (!::SetWindowPos(GetHwnd(), HWND_TOP, 0, 0, 0, 0, SWP_NOMOVE | SWP_NOSIZE))
+            if ( !wParam )
-        {
+            {
-            wxLogLastError(wxT("SetWindowPos"));
+                // Hide the window automatically when it loses activation.
-        }
+                Dismiss();
 
-        // and set it as the foreground window so the mouse can be captured
+                // Activation might have gone to a different window or maybe
-        ::SetForegroundWindow(GetHwnd());
+                // 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

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: