After the recent changes to the event processing logic, forwarding an event
from one event handler to another one stopped working correctly because the
per-event "process here only" flag prevented it from following the event
handler chain after forwarding. This notably broke keyboard navigation in
wxComboCtrl under MSW in wx itself and probably quite a few of other things in
user code.
Fix this by replacing the boolean flag with a pointer to the handler to which
the processing of this event should be restricted. This allows the full
processing to still take place if an event is forwarded to another handler.
So wxEvent::ShouldProcessHereOnly() is now called ShouldProcessOnlyIn() and
takes a wxEvtHandler parameter.
This made appear a problem in wxScrollHelperEvtHandler code that was hidden by
the bug above: the events were still processed multiple times in it. To fix
this, also add wxEvent::DidntHonourProcessOnlyIn() and take it into account in
the base class code. Did I mention that wxScrollHelperEvtHandler must die?
Add another unit test checking that forwarding works correctly.
git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@64464
c3d73ce0-8a6f-49c7-b76d-
6d57e0e08775
// 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;
// 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)
// 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=()
// 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)
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
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)
, 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)
{
}
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;
}
// 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);
// 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;
}
// (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
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;
}
CPPUNIT_TEST( TwoHandlers );
CPPUNIT_TEST( WindowWithoutHandler );
CPPUNIT_TEST( WindowWithHandler );
+ CPPUNIT_TEST( ForwardEvent );
CPPUNIT_TEST( ScrollWindowWithoutHandler );
CPPUNIT_TEST( ScrollWindowWithHandler );
CPPUNIT_TEST_SUITE_END();
void TwoHandlers();
void WindowWithoutHandler();
void WindowWithHandler();
+ void ForwardEvent();
void ScrollWindowWithoutHandler();
void ScrollWindowWithHandler();
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');
projects / wxWidgets.git / commitdiff
