From f3833aa067931c847f0c8417dc9f2c4c9f0743f4 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 3 Jan 2015 01:07:53 +0000 Subject: [PATCH] Don't change MDI children order after showing a file dialog in wxMSW. Don't use the generic focus saving/restoring code for wxMDIParentFrame in wxMSW as it already saves and restores the active MDI child on its own and we should let it do it, as our code could change the active child when restoring focus if it hadn't been saved correctly previously. The fact that it is isn't saved is another bug, but even if it is fixed, we should let MSW MDI implementation handle activation as we can't do it any better -- but can do worse, as the bug described in #16635 shows. Closes #16635. git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@78341 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775 --- docs/changes.txt | 1 + include/wx/msw/mdi.h | 5 +++++ src/msw/mdi.cpp | 37 +++++++++++++++++++++++++++++++++++-- 3 files changed, 41 insertions(+), 2 deletions(-) diff --git a/docs/changes.txt b/docs/changes.txt index 3ea00ee7e9..499cd9c0bb 100644 --- a/docs/changes.txt +++ b/docs/changes.txt @@ -127,6 +127,7 @@ wxMSW: - Fix updating wxSpinCtrlDouble tooltip text (Laurent Poujoulat). - Fix appearance of checked disabled wxToolBar tools with custom images. - Fix reading of not NUL-terminated strings using wxRegKey (Steffen Olszewski). +- Fix unexpected change in MDI children order after showing a file dialog. wxOSX/Cocoa: diff --git a/include/wx/msw/mdi.h b/include/wx/msw/mdi.h index 457414c949..e20a57db5a 100644 --- a/include/wx/msw/mdi.h +++ b/include/wx/msw/mdi.h @@ -91,6 +91,7 @@ public: // Responds to colour changes void OnSysColourChanged(wxSysColourChangedEvent& event); + void OnActivate(wxActivateEvent& event); void OnSize(wxSizeEvent& event); void OnIconized(wxIconizeEvent& event); @@ -145,6 +146,10 @@ private: // return the number of child frames we currently have (maybe 0) int GetChildFramesCount() const; + // if true, indicates whether the event wasn't really processed even though + // it was "handled", see OnActivate() and HandleActivate() + bool m_activationNotHandled; + friend class WXDLLIMPEXP_FWD_CORE wxMDIChildFrame; diff --git a/src/msw/mdi.cpp b/src/msw/mdi.cpp index b54b58580a..7d26bdce8c 100644 --- a/src/msw/mdi.cpp +++ b/src/msw/mdi.cpp @@ -121,6 +121,7 @@ IMPLEMENT_DYNAMIC_CLASS(wxMDIChildFrame, wxFrame) IMPLEMENT_DYNAMIC_CLASS(wxMDIClientWindow, wxWindow) BEGIN_EVENT_TABLE(wxMDIParentFrame, wxFrame) + EVT_ACTIVATE(wxMDIParentFrame::OnActivate) EVT_SIZE(wxMDIParentFrame::OnSize) EVT_ICONIZE(wxMDIParentFrame::OnIconized) EVT_SYS_COLOUR_CHANGED(wxMDIParentFrame::OnSysColourChanged) @@ -152,6 +153,8 @@ void wxMDIParentFrame::Init() // the default menu doesn't have any accelerators (even if we have it) m_accelWindowMenu = NULL; #endif // wxUSE_MENUS && wxUSE_ACCEL + + m_activationNotHandled = false; } bool wxMDIParentFrame::Create(wxWindow *parent, @@ -626,14 +629,44 @@ WXLRESULT wxMDIParentFrame::MSWWindowProc(WXUINT message, return rc; } +void wxMDIParentFrame::OnActivate(wxActivateEvent& WXUNUSED(event)) +{ + // The base class version saves the current focus when we are being + // deactivated and restores it when the window is activated again, but this + // is not necessary here as DefWindowProc() for MDI parent frame already + // takes care of re-activating the MDI child that had been active the last + // time, and MDI children remember their own last focused child already, + // being subclasses of wxTLW. + // + // Moreover, in addition to being unnecessary, this can be actively harmful + // if we somehow don't have the focus any more at the moment of activation + // loss as happens when showing a standard file dialog under Windows 7, see + // #16635: in this case the base class just gives the focus to its first + // child, meaning that we can switch to a different MDI child, which is + // worse than losing focus inside the current child. + // + // So we don't let the base class have this event to prevent this from + // happening. But the event is not really processed, so we set a flag here + // which is used in HandleActivate() below to check if the event was really + // processed (and not skipped) in the user code or just reached this dummy + // handler. + m_activationNotHandled = true; +} + bool wxMDIParentFrame::HandleActivate(int state, bool minimized, WXHWND activate) { bool processed = false; + // Set the flag before testing it to ensure the only way for it to be true + // is to be set in our OnActivate() -- and not just remain set from the + // last time. + m_activationNotHandled = false; + if ( wxWindow::HandleActivate(state, minimized, activate) ) { - // already processed - processed = true; + // already processed, unless we artificially marked the event as + // handled in our own handler without really processing it + processed = !m_activationNotHandled; } // If this window is an MDI parent, we must also send an OnActivate message