From a7c0de8a98250e908329ece9e14742b9d688d1e4 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 4 May 2013 23:59:56 +0000 Subject: [PATCH] Fix event handling order in doc/view framework. Ensure that the events are always (provided there is an open document) processed in the following order: 1. wxDocument 2. wxView 3. wxDocManager 4. wxDocChildFrame 5. wxDocParentFrame 6. wxApp Do this by forwarding the events from wxDocParentFrame to wxDocChildFrame first and forward them from there to wxDocManager which -- and this part remains unchanged -- in turn forwards them to the active wxView which finally forwards them to wxDocument. This requires another condition in the event handling code as we still must forward from wxDocParentFrame to wxDocManager itself if there are no active children at all, but this is the only way to have the same event order in all cases, whether the event is originally received by wxDocChildFrame or wxDocParentFrame. Document this and add a unit test verifying that things indeed work like this. See #14314. git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@73928 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775 --- docs/doxygen/overviews/docview.h | 24 +++++++ include/wx/docview.h | 26 ++++--- src/common/docview.cpp | 49 +++++++++++++ tests/events/propagation.cpp | 115 +++++++++++++++++++++++++++++++ 4 files changed, 204 insertions(+), 10 deletions(-) diff --git a/docs/doxygen/overviews/docview.h b/docs/doxygen/overviews/docview.h index 49d2689a55..2c5af959d2 100644 --- a/docs/doxygen/overviews/docview.h +++ b/docs/doxygen/overviews/docview.h @@ -214,6 +214,30 @@ There may be multiple wxDocManager instances in an application. See the example application in @c samples/docview. +@section overview_docview_events Event Propagation in Document/View framework + +While wxDocument, wxDocManager and wxView are abstract objects, with which the +user can't interact directly, all of them derive from wxEvtHandler class and +can handle events arising in the windows showing the document with which the +user does interact. This is implemented by adding additional steps to the event +handling process described in @ref overview_events_processing, so the full list +of the handlers searched for an event occurring directly in wxDocChildFrame is: +
    +
  1. wxDocument opened in this frame.
    2. +
  2. wxView shown in this frame.
    3. +
  3. wxDocManager associated with the parent wxDocParentFrame.
    4. +
  4. wxDocChildFrame itself.
    5. +
  5. wxDocParentFrame, as per the usual event bubbling up to parent rules.
    6. +
  6. wxApp, again as the usual fallback for all events.
    7. +
+ +This is mostly useful to define handlers for some menu commands directly in +wxDocument or wxView and is also used by the framework itself to define the +handlers for several standard commands, such as wxID_NEW or wxID_SAVE, in +wxDocManager itself. Notice that due to the order of the event handler search +detailed above, the handling of these commands can @e not be overridden at +wxDocParentFrame level but must be done at the level of wxDocManager itself. + @section overview_docview_wxcommand wxCommand Overview diff --git a/include/wx/docview.h b/include/wx/docview.h index 1c789626c7..c8b0347215 100644 --- a/include/wx/docview.h +++ b/include/wx/docview.h @@ -643,10 +643,7 @@ protected: // we're not a wxEvtHandler but we provide this wxEvtHandler-like function // which is called from TryBefore() of the derived classes to give our view // a chance to process the message before the frame event handlers are used - bool TryProcessEvent(wxEvent& event) - { - return m_childView && m_childView->ProcessEventLocally(event); - } + bool TryProcessEvent(wxEvent& event); // called from EVT_CLOSE handler in the frame: check if we can close and do // cleanup if so; veto the event otherwise @@ -830,11 +827,22 @@ private: class WXDLLIMPEXP_CORE wxDocParentFrameAnyBase { public: - wxDocParentFrameAnyBase() { m_docManager = NULL; } + wxDocParentFrameAnyBase(wxWindow* frame) + : m_frame(frame) + { + m_docManager = NULL; + } wxDocManager *GetDocumentManager() const { return m_docManager; } protected: + // This is similar to wxDocChildFrameAnyBase method with the same name: + // while we're not an event handler ourselves and so can't override + // TryBefore(), we provide a helper that the derived template class can use + // from its TryBefore() implementation. + bool TryProcessEvent(wxEvent& event); + + wxWindow* const m_frame; wxDocManager *m_docManager; wxDECLARE_NO_COPY_CLASS(wxDocParentFrameAnyBase); @@ -847,7 +855,7 @@ class WXDLLIMPEXP_CORE wxDocParentFrameAny : public BaseFrame, public wxDocParentFrameAnyBase { public: - wxDocParentFrameAny() { } + wxDocParentFrameAny() : wxDocParentFrameAnyBase(this) { } wxDocParentFrameAny(wxDocManager *manager, wxFrame *frame, wxWindowID id, @@ -856,6 +864,7 @@ public: const wxSize& size = wxDefaultSize, long style = wxDEFAULT_FRAME_STYLE, const wxString& name = wxFrameNameStr) + : wxDocParentFrameAnyBase(this) { Create(manager, frame, id, title, pos, size, style, name); } @@ -886,10 +895,7 @@ protected: // hook the document manager into event handling chain here virtual bool TryBefore(wxEvent& event) { - if ( m_docManager && m_docManager->ProcessEventLocally(event) ) - return true; - - return BaseFrame::TryBefore(event); + return TryProcessEvent(event) || BaseFrame::TryBefore(event); } private: diff --git a/src/common/docview.cpp b/src/common/docview.cpp index 468016ec2d..246d247254 100644 --- a/src/common/docview.cpp +++ b/src/common/docview.cpp @@ -2014,6 +2014,26 @@ void wxDocManager::ActivateView(wxView *view, bool activate) // wxDocChildFrameAnyBase // ---------------------------------------------------------------------------- +bool wxDocChildFrameAnyBase::TryProcessEvent(wxEvent& event) +{ + if ( !m_childView ) + { + // We must be being destroyed, don't forward events anywhere as + // m_childDocument could be invalid by now. + return false; + } + + // Forward the event to the document manager which will, in turn, forward + // it to its active view which must be our m_childView. + // + // Notice that we do things in this roundabout way to guarantee the correct + // event handlers call order: first the document, then the new and then the + // document manager itself. And if we forwarded the event directly to the + // view, then the document manager would do it once again when we forwarded + // it to it. + return m_childDocument->GetDocumentManager()->ProcessEventLocally(event); +} + bool wxDocChildFrameAnyBase::CloseView(wxCloseEvent& event) { if ( m_childView ) @@ -2046,6 +2066,35 @@ bool wxDocChildFrameAnyBase::CloseView(wxCloseEvent& event) // wxDocParentFrameAnyBase // ---------------------------------------------------------------------------- +bool wxDocParentFrameAnyBase::TryProcessEvent(wxEvent& event) +{ + if ( !m_docManager ) + return false; + + // If we have an active view, its associated child frame may have + // already forwarded the event to wxDocManager, check for this: + if ( wxView* const view = m_docManager->GetAnyUsableView() ) + { + // Notice that we intentionally don't use wxGetTopLevelParent() here + // because we want to check both for the case of a child "frame" (e.g. + // MDI child frame or notebook page) inside this TLW and a separate + // child TLW frame (as used in the SDI mode) here. + for ( wxWindow* win = view->GetFrame(); win; win = win->GetParent() ) + { + if ( win == m_frame ) + return false; + } + } + + // But forward the event to wxDocManager ourselves if there are no views at + // all or if we are the frame's view ourselves. + return m_docManager->ProcessEventLocally(event); +} + +// ---------------------------------------------------------------------------- +// Printing support +// ---------------------------------------------------------------------------- + #if wxUSE_PRINTING_ARCHITECTURE namespace diff --git a/tests/events/propagation.cpp b/tests/events/propagation.cpp index 0a9222f7e0..5bd36a3bd2 100644 --- a/tests/events/propagation.cpp +++ b/tests/events/propagation.cpp @@ -24,6 +24,7 @@ #include "wx/window.h" #endif // WX_PRECOMP +#include "wx/docmdi.h" #include "wx/frame.h" #include "wx/menu.h" #include "wx/scopedptr.h" @@ -100,6 +101,26 @@ struct TestPaintEvtHandler : TestEvtHandlerBase } }; +// Another custom event handler, suitable for use with Connect(). +struct TestEvtSink : wxEvtHandler +{ + TestEvtSink(char tag) + : m_tag(tag) + { + } + + void Handle(wxEvent& event) + { + g_str += m_tag; + + event.Skip(); + } + + const char m_tag; + + wxDECLARE_NO_COPY_CLASS(TestEvtSink); +}; + // a window handling the test event class TestWindow : public wxWindow { @@ -195,6 +216,7 @@ private: CPPUNIT_TEST( ScrollWindowWithoutHandler ); CPPUNIT_TEST( ScrollWindowWithHandler ); CPPUNIT_TEST( MenuEvent ); + CPPUNIT_TEST( DocView ); CPPUNIT_TEST_SUITE_END(); void OneHandler(); @@ -205,6 +227,7 @@ private: void ScrollWindowWithoutHandler(); void ScrollWindowWithHandler(); void MenuEvent(); + void DocView(); DECLARE_NO_COPY_CLASS(EventPropagationTestCase) }; @@ -434,3 +457,95 @@ void EventPropagationTestCase::MenuEvent() ASSERT_MENU_EVENT_RESULT( menu, "aomobowA" ); } + +// Minimal viable implementations of wxDocument and wxView. +class EventTestDocument : public wxDocument +{ +public: + EventTestDocument() { } + + wxDECLARE_DYNAMIC_CLASS(EventTestDocument); +}; + +class EventTestView : public wxView +{ +public: + EventTestView() { } + + virtual void OnDraw(wxDC*) { } + + wxDECLARE_DYNAMIC_CLASS(EventTestView); +}; + +wxIMPLEMENT_DYNAMIC_CLASS(EventTestDocument, wxDocument); +wxIMPLEMENT_DYNAMIC_CLASS(EventTestView, wxView); + +void EventPropagationTestCase::DocView() +{ + // Set up the parent frame and its menu bar. + wxDocManager docManager; + + wxScopedPtr + parent(new wxDocMDIParentFrame(&docManager, NULL, wxID_ANY, "Parent")); + + wxMenu* const menu = CreateTestMenu(parent.get()); + + + // Set up the event handlers. + TestEvtSink sinkDM('m'); + docManager.Connect(wxEVT_MENU, + wxEventHandler(TestEvtSink::Handle), NULL, &sinkDM); + + TestEvtSink sinkParent('p'); + parent->Connect(wxEVT_MENU, + wxEventHandler(TestEvtSink::Handle), NULL, &sinkParent); + + + // Check that wxDocManager and wxFrame get the event in order. + ASSERT_MENU_EVENT_RESULT( menu, "ampA" ); + + + // Now check what happens if we have an active document. + wxDocTemplate docTemplate(&docManager, "Test", "", "", "", + "Test Document", "Test View", + wxCLASSINFO(EventTestDocument), + wxCLASSINFO(EventTestView)); + wxDocument* const doc = docTemplate.CreateDocument(""); + wxView* const view = doc->GetFirstView(); + + wxScopedPtr + child(new wxDocMDIChildFrame(doc, view, parent.get(), wxID_ANY, "Child")); + + wxMenu* const menuChild = CreateTestMenu(child.get()); + +#ifdef __WXGTK__ + // There are a lot of hacks related to child frame menu bar handling in + // wxGTK and, in particular, the code in src/gtk/mdi.cpp relies on getting + // idle events to really put everything in place. Moreover, as wxGTK uses + // GtkNotebook as its MDI pages container, the frame must be shown for all + // this to work as gtk_notebook_set_current_page() doesn't do anything if + // called for a hidden window (this incredible fact cost me quite some time + // to find empirically -- only to notice its confirmation in GTK+ + // documentation immediately afterwards). So just do whatever it takes to + // make things work "as usual". + child->Show(); + parent->Show(); + wxYield(); +#endif // __WXGTK__ + + TestEvtSink sinkDoc('d'); + doc->Connect(wxEVT_MENU, + wxEventHandler(TestEvtSink::Handle), NULL, &sinkDoc); + + TestEvtSink sinkView('v'); + view->Connect(wxEVT_MENU, + wxEventHandler(TestEvtSink::Handle), NULL, &sinkView); + + TestEvtSink sinkChild('c'); + child->Connect(wxEVT_MENU, + wxEventHandler(TestEvtSink::Handle), NULL, &sinkChild); + + // Check that wxDocument, wxView, wxDocManager, child frame and the parent + // get the event in order. + ASSERT_MENU_EVENT_RESULT( menuChild, "advmcpA" ); +} -- 2.47.2