Merge branch 'msw-popup'

Reimplement wxPopupWindow using WS_POPUP instead of WS_CHILD window in
wxMSW as the new approach allows using the controls inside the popup
normally, unlike the old one.

See https://github.com/wxWidgets/wxWidgets/pull/986

Closes #18243.

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__)
 
 // ----------------------------------------------------------------------------

interface/wx/popupwin.h

@@ -84,10 +84,10 @@ public:
     /**
         Popup the window (this will show it too).
 
-        If @a winFocus is non-@NULL, it will be kept focused while this window
+        If @a focus is non-@NULL, it will be kept focused while this window
-        is shown, otherwise this window itself will receive focus. In any case,
+        is shown if supported by the current platform, otherwise the popup
-        the popup will disappear automatically if it loses focus because of a
+        itself will receive focus. In any case, the popup will disappear
-        user action.
+        automatically if it loses focus because of a user action.
 
         @see Dismiss()
     */

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
 // ============================================================================
@@ -133,6 +135,12 @@ wxPopupWindowBase::~wxPopupWindowBase()
 
 bool wxPopupWindowBase::Create(wxWindow* WXUNUSED(parent), int WXUNUSED(flags))
 {
+    // By default, block event propagation at this window as it usually
+    // doesn't make sense. This notably prevents wxScrolledWindow from trying
+    // to scroll popup contents into view if a popup is shown from it but
+    // extends beyond its window boundaries.
+    SetExtraStyle(GetExtraStyle() | wxWS_EX_BLOCK_EVENTS);
+
     return true;
 }
 
@@ -201,6 +209,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 +321,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 +375,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 +435,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 +446,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 +490,7 @@ void wxPopupTransientWindow::OnIdle(wxIdleEvent& event)
 }
 #endif // wxOSX/Carbon
 
+#endif // !__WXMSW__
 
 #if wxUSE_COMBOBOX && defined(__WXUNIVERSAL__)
 
@@ -555,6 +546,8 @@ void wxPopupComboWindow::OnKeyDown(wxKeyEvent& event)
 
 #endif // wxUSE_COMBOBOX && defined(__WXUNIVERSAL__)
 
+#ifndef __WXMSW__
+
 // ----------------------------------------------------------------------------
 // wxPopupWindowHandler
 // ----------------------------------------------------------------------------
@@ -696,4 +689,6 @@ void wxPopupFocusHandler::OnChar(wxKeyEvent& event)
     }
 }
 
+#endif // !__WXMSW__
+
 #endif // wxUSE_POPUPWIN

src/msw/button.cpp

@@ -252,28 +252,14 @@ wxWindow *wxButton::SetDefault()
 // return NULL
 static wxTopLevelWindow *GetTLWParentIfNotBeingDeleted(wxWindow *win)
 {
-    for ( ;; )
+    wxWindow* const parent = wxGetTopLevelParent(win);
-    {
+    wxASSERT_MSG( parent, wxT("button without top level parent?") );
-        // IsTopLevel() will return false for a wxTLW being deleted, so we also
-        // need the parent test for this case
-        wxWindow * const parent = win->GetParent();
-        if ( !parent || win->IsTopLevel() )
-        {
-            if ( win->IsBeingDeleted() )
-                return NULL;
 
-            break;
+    if ( parent->IsBeingDeleted() )
-        }
+        return NULL;
 
-        win = parent;
+    // Note that this may still return null for a button inside wxPopupWindow.
-    }
+    return wxDynamicCast(parent, wxTopLevelWindow);
-
-    wxASSERT_MSG( win, wxT("button without top level parent?") );
-
-    wxTopLevelWindow * const tlw = wxDynamicCast(win, wxTopLevelWindow);
-    wxASSERT_MSG( tlw, wxT("logic error in GetTLWParentIfNotBeingDeleted()") );
-
-    return tlw;
 }
 
 // set this button as being currently default

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,68 @@ 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();
+
+    // We can only set focus to one of our children as setting it to another
+    // window would result in an immediate loss of activation and popup
+    // disappearance.
+    if ( focus && IsDescendant(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: