diff --git a/include/wx/event.h b/include/wx/event.h index e7536eb9ce..f989454ce6 100644 --- a/include/wx/event.h +++ b/include/wx/event.h @@ -999,7 +999,21 @@ public: // This is also used only internally by ProcessEvent() to check if it // should process the event normally or only restrict the search for the // event handler to this object itself. - bool ShouldProcessHereOnly() const { return m_processHereOnly; } + bool ShouldProcessOnlyIn(wxEvtHandler *h) const + { + return h == m_handlerToProcessOnlyIn; + } + + // Called to indicate that the result of ShouldProcessOnlyIn() wasn't taken + // into account. The existence of this function may seem counterintuitive + // but unfortunately it's needed by wxScrollHelperEvtHandler, see comments + // there. Don't even think of using this in your own code, this is a gross + // hack and is only needed because of wx complicated history and should + // never be used anywhere else. + void DidntHonourProcessOnlyIn() + { + m_handlerToProcessOnlyIn = NULL; + } protected: wxObject* m_eventObject; @@ -1011,6 +1025,10 @@ public: // m_callbackUserData is for internal usage only wxObject* m_callbackUserData; +private: + // If this handler + wxEvtHandler *m_handlerToProcessOnlyIn; + protected: // the propagation level: while it is positive, we propagate the event to // the parent window (if any) @@ -1025,11 +1043,6 @@ protected: // once for this event bool m_wasProcessed; - // this flag is used by ProcessEventLocally() to prevent ProcessEvent() - // from doing its usual stuff and force it to just call TryHere() instead, - // see the comment there explaining why is this needed - bool m_processHereOnly; - protected: wxEvent(const wxEvent&); // for implementing Clone() wxEvent& operator=(const wxEvent&); // for derived classes operator=() @@ -1038,8 +1051,8 @@ private: // it needs to access our m_propagationLevel friend class WXDLLIMPEXP_FWD_BASE wxPropagateOnce; - // and this one needs to access our m_processHereOnly - friend class WXDLLIMPEXP_FWD_BASE wxEventProcessHereOnly; + // and this one needs to access our m_handlerToProcessOnlyIn + friend class WXDLLIMPEXP_FWD_BASE wxEventProcessInHandlerOnly; DECLARE_ABSTRACT_CLASS(wxEvent) @@ -1093,31 +1106,28 @@ private: wxDECLARE_NO_COPY_CLASS(wxPropagateOnce); }; -// A helper used by ProcessEventLocally() to restrict the event processing -// to this handler only. -class WXDLLIMPEXP_BASE wxEventProcessHereOnly +// A helper object used to temporarily make wxEvent::ShouldProcessOnlyIn() +// return true for the handler passed to its ctor. +class wxEventProcessInHandlerOnly { public: - wxEventProcessHereOnly(wxEvent& event) : m_event(event) + wxEventProcessInHandlerOnly(wxEvent& event, wxEvtHandler *handler) + : m_event(event), + m_handlerToProcessOnlyInOld(event.m_handlerToProcessOnlyIn) { - // This would be unexpected and would also restore the wrong value in - // this class dtor so if even does happen legitimately we'd need to - // store the value in ctor and restore it in dtor. - wxASSERT_MSG( !m_event.m_processHereOnly, - "shouldn't be used twice for the same event" ); - - m_event.m_processHereOnly = true; + m_event.m_handlerToProcessOnlyIn = handler; } - ~wxEventProcessHereOnly() + ~wxEventProcessInHandlerOnly() { - m_event.m_processHereOnly = false; + m_event.m_handlerToProcessOnlyIn = m_handlerToProcessOnlyInOld; } private: wxEvent& m_event; + wxEvtHandler * const m_handlerToProcessOnlyInOld; - wxDECLARE_NO_COPY_CLASS(wxEventProcessHereOnly); + wxDECLARE_NO_COPY_CLASS(wxEventProcessInHandlerOnly); }; #if wxUSE_GUI diff --git a/src/common/event.cpp b/src/common/event.cpp index b6c0144752..c4f1ae8164 100644 --- a/src/common/event.cpp +++ b/src/common/event.cpp @@ -363,10 +363,10 @@ wxEvent::wxEvent(int theId, wxEventType commandType) m_id = theId; m_skipped = false; m_callbackUserData = NULL; + m_handlerToProcessOnlyIn = NULL; m_isCommandEvent = false; m_propagationLevel = wxEVENT_PROPAGATE_NONE; m_wasProcessed = false; - m_processHereOnly = false; } wxEvent::wxEvent(const wxEvent& src) @@ -376,11 +376,11 @@ wxEvent::wxEvent(const wxEvent& src) , m_timeStamp(src.m_timeStamp) , m_id(src.m_id) , m_callbackUserData(src.m_callbackUserData) + , m_handlerToProcessOnlyIn(NULL) , m_propagationLevel(src.m_propagationLevel) , m_skipped(src.m_skipped) , m_isCommandEvent(src.m_isCommandEvent) , m_wasProcessed(false) - , m_processHereOnly(false) { } @@ -393,11 +393,12 @@ wxEvent& wxEvent::operator=(const wxEvent& src) m_timeStamp = src.m_timeStamp; m_id = src.m_id; m_callbackUserData = src.m_callbackUserData; + m_handlerToProcessOnlyIn = NULL; m_propagationLevel = src.m_propagationLevel; m_skipped = src.m_skipped; m_isCommandEvent = src.m_isCommandEvent; - // don't change m_wasProcessed nor m_processHereOnly + // don't change m_wasProcessed return *this; } @@ -1378,7 +1379,7 @@ bool wxEvtHandler::ProcessEvent(wxEvent& event) // Short circuit the event processing logic if we're requested to process // this event in this handler only, see DoTryChain() for more details. - if ( event.ShouldProcessHereOnly() ) + if ( event.ShouldProcessOnlyIn(this) ) return TryHere(event); @@ -1432,12 +1433,21 @@ bool wxEvtHandler::DoTryChain(wxEvent& event) // window to be called. // // So we must call ProcessEvent() but it must not do what it usually - // does. To resolve this paradox we pass a special "process here only" - // flag to ProcessEvent() via the event object itself. This ensures - // that if our own, base class, version is called, it will just call - // TryHere() and won't do anything else, just as we want it to. - wxEventProcessHereOnly processHereOnly(event); - if ( h->ProcessEvent(event) ) + // does. To resolve this paradox we set up a special flag inside the + // object itself to let ProcessEvent() know that it shouldn't do any + // pre/post-processing for this event if it gets it. Note that this + // only applies to this handler, if the event is passed to another one + // by explicitly calling its ProcessEvent(), pre/post-processing should + // be done as usual. + // + // Final complication is that if the implementation of ProcessEvent() + // called wxEvent::DidntHonourProcessOnlyIn() (as the gross hack that + // is wxScrollHelperEvtHandler::ProcessEvent() does) and ignored our + // request to process event in this handler only, we have to compensate + // for it by not processing the event further because this was already + // done by that rogue event handler. + wxEventProcessInHandlerOnly processInHandlerOnly(event, h); + if ( h->ProcessEvent(event) || !event.ShouldProcessOnlyIn(h) ) return true; } diff --git a/src/generic/scrlwing.cpp b/src/generic/scrlwing.cpp index 0f1a4f57ef..6244d494a0 100644 --- a/src/generic/scrlwing.cpp +++ b/src/generic/scrlwing.cpp @@ -209,11 +209,6 @@ bool wxScrollHelperEvtHandler::ProcessEvent(wxEvent& event) // (as indicated by "process here only" flag being set) and we do want to // execute the handler defined in the window we're associated with right // now, without waiting until TryAfter() is called from wxEvtHandler. - // - // Note that this means that the handler in the window will be called twice - // if there is a preceding event handler in the chain because we do it from - // here now and the base class DoTryChain() will also call it itself when - // we return. But this unfortunately seems unavoidable. bool processed = m_nextHandler->ProcessEvent(event); // always process the size events ourselves, even if the user code handles @@ -310,6 +305,17 @@ bool wxScrollHelperEvtHandler::ProcessEvent(wxEvent& event) event.Skip(wasSkipped); + // We called ProcessEvent() on the next handler, meaning that we explicitly + // worked around the request to process the event in this handler only. As + // explained above, this is unfortunately really necessary but the trouble + // is that the event will continue to be post-processed by the previous + // handler resulting in duplicate calls to event handlers. Call the special + // function below to prevent this from happening, base class DoTryChain() + // will check for it and behave accordingly. + // + // And if we're not called from DoTryChain(), this won't do anything anyhow. + event.DidntHonourProcessOnlyIn(); + return processed; } diff --git a/tests/events/propagation.cpp b/tests/events/propagation.cpp index 7ec1e50dd3..49d6fac0a2 100644 --- a/tests/events/propagation.cpp +++ b/tests/events/propagation.cpp @@ -178,6 +178,7 @@ private: CPPUNIT_TEST( TwoHandlers ); CPPUNIT_TEST( WindowWithoutHandler ); CPPUNIT_TEST( WindowWithHandler ); + CPPUNIT_TEST( ForwardEvent ); CPPUNIT_TEST( ScrollWindowWithoutHandler ); CPPUNIT_TEST( ScrollWindowWithHandler ); CPPUNIT_TEST_SUITE_END(); @@ -186,6 +187,7 @@ private: void TwoHandlers(); void WindowWithoutHandler(); void WindowWithHandler(); + void ForwardEvent(); void ScrollWindowWithoutHandler(); void ScrollWindowWithHandler(); @@ -262,6 +264,48 @@ void EventPropagationTestCase::WindowWithHandler() CPPUNIT_ASSERT_EQUAL( "oa2o1cpA", g_str ); } +void EventPropagationTestCase::ForwardEvent() +{ + // The idea of this test is to check that the events explicitly forwarded + // to another event handler still get pre/post-processed as usual as this + // used to be broken by the fixes trying to avoid duplicate processing. + TestWindow * const win = new TestWindow(wxTheApp->GetTopWindow(), 'w'); + wxON_BLOCK_EXIT_OBJ0( *win, wxWindow::Destroy ); + + TestEvtHandler h1('1'); + win->PushEventHandler(&h1); + wxON_BLOCK_EXIT_OBJ1( *win, wxWindow::PopEventHandler, false ); + + class ForwardEvtHandler : public wxEvtHandler + { + public: + ForwardEvtHandler(wxEvtHandler& h) : m_h(&h) { } + + virtual bool ProcessEvent(wxEvent& event) + { + g_str += 'f'; + + return m_h->ProcessEvent(event); + } + + private: + wxEvtHandler *m_h; + } f(h1); + + // First send the event directly to f. + wxCommandEvent event1(TEST_EVT); + f.ProcessEvent(event1); + CPPUNIT_ASSERT_EQUAL( "foa1wA", g_str ); + g_str.clear(); + + // And then also test sending it to f indirectly. + wxCommandEvent event2(TEST_EVT); + TestEvtHandler h2('2'); + h2.SetNextHandler(&f); + h2.ProcessEvent(event2); + CPPUNIT_ASSERT_EQUAL( "oa2fo1wAA", g_str ); +} + void EventPropagationTestCase::ScrollWindowWithoutHandler() { TestWindow * const parent = new TestWindow(wxTheApp->GetTopWindow(), 'p');