Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Fix button key listeners.

Only one listener can be registered for each key. Subsequent attempts
are ignored. A frame count is needed for skip back, because new
DisplayObjects are constructed before the old ones are destroyed when
jumping back, and this means that their attempt to register a key
listener is blocked by the not-yet-destroyed Button.
  • Loading branch information...
commit 3db73486d8b99425a707e6ab04acf37942303eb8 1 parent 5d18407
@bwy bwy authored
View
51 libcore/Button.cpp
@@ -222,6 +222,24 @@ class ButtonActionPusher {
DisplayObject* _tp;
};
+class ButtonKeyRegisterer {
+public:
+ ButtonKeyRegisterer(movie_root& mr, Button* this_ptr)
+ :
+ _mr(mr),
+ _tp(this_ptr)
+ {}
+
+ void operator()(const SWF::ButtonAction& b) const
+ {
+ _mr.registerButtonKey(b.getKeyCode(), _tp);
+ }
+
+private:
+ movie_root& _mr;
+ Button* _tp;
+};
+
}
namespace {
@@ -294,17 +312,10 @@ Button::Button(as_object* object, const SWF::DefineButtonTag* def,
_def(def)
{
assert(object);
-
- // check up presence Key events
- if (_def->hasKeyPressHandler()) {
- stage().add_key_listener(this);
- }
-
}
Button::~Button()
{
- stage().remove_key_listener(this);
}
bool
@@ -343,23 +354,21 @@ void
Button::notifyEvent(const event_id& id)
{
if (unloaded()) {
- // We dont' respond to events while unloaded
+ // We don't respond to events while unloaded
// See bug #22982
return;
}
- // We only respond keypress events
- if ( id.id() != event_id::KEY_PRESS ) return;
-
- // We only respond to valid key code (should we assert here?)
- if ( id.keyCode() == key::INVALID ) return;
+ assert(id.id() == event_id::KEY_PRESS);
+ assert(id.keyCode() != key::INVALID);
ButtonActionPusher xec(stage(), this);
_def->forEachTrigger(id, xec);
}
bool
-Button::handleFocus() {
+Button::handleFocus()
+{
/// Nothing to do, but can receive focus.
return false;
}
@@ -454,8 +463,7 @@ Button::topmostMouseEntity(boost::int32_t x, boost::int32_t y)
void
Button::mouseEvent(const event_id& event)
{
- if ( unloaded() )
- {
+ if (unloaded()) {
// We don't respond to events while unloaded. See bug #22982.
log_debug("Button %s received %s button event while unloaded: ignored",
getTarget(), event);
@@ -843,6 +851,13 @@ Button::construct(as_object* initObj)
// There is no INITIALIZE/CONSTRUCT/LOAD/ENTERFRAME/UNLOAD event
// for Buttons
+
+ // Register key events.
+ if (_def->hasKeyPressHandler()) {
+ ButtonKeyRegisterer r(stage(), this);
+ _def->forEachAction(r);
+ }
+
}
void
@@ -866,6 +881,7 @@ Button::markOwnResources() const
bool
Button::unloadChildren()
{
+ GNASH_REPORT_FUNCTION;
bool childsHaveUnload = false;
@@ -893,6 +909,9 @@ Button::unloadChildren()
void
Button::destroy()
{
+ GNASH_REPORT_FUNCTION;
+
+ stage().removeButtonKey(this);
for (DisplayObjects::iterator i = _stateCharacters.begin(),
e=_stateCharacters.end(); i != e; ++i) {
View
3  libcore/Button.h
@@ -120,8 +120,7 @@ class Button : public InteractiveObject
/// Do ActionScript construction of the Button.
//
- /// (1) Register this button instance as a live DisplayObject
- /// (2) Construct all button state DisplayObjects.
+ /// Construct all button state DisplayObjects.
//
/// @param init An init object, which can be passed when constructing
/// Buttons with attachMovie, but is never used.
View
67 libcore/movie_root.cpp
@@ -518,8 +518,8 @@ movie_root::reset()
// remove all loadMovie requests
_movieLoader.clear();
- // remove key listeners
- _keyListeners.clear();
+ // Remove button key events.
+ _buttonKeys.clear();
// Cleanup the stack.
_vm.getStack().clear();
@@ -600,10 +600,12 @@ movie_root::keyEvent(key::code k, bool down)
// A stack limit like that is hardly of any use, but could be used
// maliciously to crash Gnash.
if (down) {
- callMethod(key, getURI(_vm, NSV::PROP_BROADCAST_MESSAGE), "onKeyDown");
+ callMethod(key, getURI(_vm, NSV::PROP_BROADCAST_MESSAGE),
+ "onKeyDown");
}
else {
- callMethod(key, getURI(_vm,NSV::PROP_BROADCAST_MESSAGE), "onKeyUp");
+ callMethod(key, getURI(_vm,NSV::PROP_BROADCAST_MESSAGE),
+ "onKeyUp");
}
}
catch (const ActionLimitException &e) {
@@ -613,21 +615,17 @@ movie_root::keyEvent(key::code k, bool down)
}
}
- // Then any button keys are notified.
- Listeners lcopy = _keyListeners;
- for (Listeners::iterator iter = lcopy.begin(), itEnd = lcopy.end();
- iter != itEnd; ++iter) {
-
- // sprite, button & input_edit_text DisplayObjects
- Button* const ch = *iter;
- if (ch->unloaded()) continue;
+ if (down) {
+ ButtonKeys::const_iterator it =
+ _buttonKeys.find(key::codeMap[k][key::SWF]);
- if (down) {
- ch->notifyEvent(event_id(event_id::KEY_PRESS, k));
+ if (it != _buttonKeys.end()) {
+ if (!it->second.first->unloaded()) {
+ it->second.first->notifyEvent(event_id(event_id::KEY_PRESS, k));
+ }
}
}
-
// If we're focused on an editable text field, finally the text is updated
if (down) {
TextField* tf = dynamic_cast<TextField*>(_currentFocus);
@@ -1728,17 +1726,6 @@ movie_root::markReachableResources() const
}
#endif
-#if ( GNASH_PARANOIA_LEVEL > 1 ) || defined(ALLOW_GC_RUN_DURING_ACTIONS_EXECUTION)
- for (Listeners::const_iterator i=_keyListeners.begin(),
- e=_keyListeners.end(); i!=e; ++i) {
-#ifdef ALLOW_GC_RUN_DURING_ACTIONS_EXECUTION
- (*i)->setReachable();
-#else
- assert((*i)->isReachable());
-#endif
- }
-#endif
-
}
InteractiveObject*
@@ -1886,20 +1873,34 @@ movie_root::callExternalCallback(const std::string &name,
}
void
-movie_root::remove_key_listener(Button* listener)
+movie_root::removeButtonKey(Button* listener)
{
- _keyListeners.remove_if(std::bind2nd(std::equal_to<Button*>(), listener));
+ GNASH_REPORT_FUNCTION;
+
+ // Remove the button and the key associated with it from the map.
+ for (ButtonKeys::iterator i = _buttonKeys.begin(), e = _buttonKeys.end();
+ i != e;) {
+ if (i->second.first == listener) {
+ _buttonKeys.erase(i++);
+ }
+ else ++i;
+ }
+
}
void
-movie_root::add_key_listener(Button* listener)
+movie_root::registerButtonKey(int code, Button* listener)
{
- assert(listener);
+ log_debug("Adding listener for code %s", +code);
- if (std::find(_keyListeners.begin(), _keyListeners.end(), listener)
- != _keyListeners.end()) return;
+ const size_t frame = _rootMovie->get_current_frame();
+ ButtonKeys::iterator it = _buttonKeys.find(code);
+
+ if (it != _buttonKeys.end() && it->second.second < frame) {
+ return;
+ }
- _keyListeners.push_front(listener);
+ _buttonKeys[code] = std::make_pair(listener, frame);
}
void
View
23 libcore/movie_root.h
@@ -150,9 +150,6 @@ class DSOEXPORT movie_root : public GcRoot, boost::noncopyable
{
public:
- /// Listeners container
- typedef std::list<Button*> Listeners;
-
class LoadCallback {
public:
LoadCallback(boost::shared_ptr<IOChannel> s, as_object* o)
@@ -394,10 +391,10 @@ class DSOEXPORT movie_root : public GcRoot, boost::noncopyable
}
/// Push a new DisplayObject listener for key events
- void add_key_listener(Button* listener);
+ void registerButtonKey(int c, Button* listener);
/// Remove a DisplayObject listener for key events
- void remove_key_listener(Button* listener);
+ void removeButtonKey(Button* listener);
/// Get the DisplayObject having focus
//
@@ -566,9 +563,7 @@ class DSOEXPORT movie_root : public GcRoot, boost::noncopyable
/// - Mouse entities (m_mouse_button_state)
/// - Timer targets (_intervalTimers)
/// - Resources reachable by ActionQueue code (_actionQueue)
- /// - Key listeners (_keyListeners)
/// - Any DisplayObject being dragged
- ///
void markReachableResources() const;
/// \brief
@@ -927,15 +922,13 @@ class DSOEXPORT movie_root : public GcRoot, boost::noncopyable
void handleActionLimitHit(const std::string& ref);
- /// Buttons listening for key events
- //
- /// Note that Buttons (the only key listeners left) deregister themselves
- /// on destruction. This isn't correct behaviour and also requires that
- /// _keyListeners be alive longer than _gc so that deregistration doesn't
- /// access a destroyed object.
+ /// A map of SWF key code to Buttons.
//
- /// TODO: fix it.
- Listeners _keyListeners;
+ /// The Buttons are removed on destruction, so there is no need to
+ /// mark them reachable.
+ typedef std::pair<Button*, size_t> ButtonFrame;
+ typedef std::map<int, ButtonFrame> ButtonKeys;
+ ButtonKeys _buttonKeys;
GC _gc;
View
5 libcore/swf/DefineButtonTag.cpp
@@ -297,8 +297,7 @@ ButtonAction::ButtonAction(SWFStream& in, TagType t, unsigned long endPos,
bool
ButtonAction::triggeredBy(const event_id& ev) const
{
- switch ( ev.id() )
- {
+ switch (ev.id()) {
case event_id::ROLL_OVER: return _conditions & IDLE_TO_OVER_UP;
case event_id::ROLL_OUT: return _conditions & OVER_UP_TO_IDLE;
case event_id::PRESS: return _conditions & OVER_UP_TO_OVER_DOWN;
@@ -309,7 +308,7 @@ ButtonAction::triggeredBy(const event_id& ev) const
case event_id::KEY_PRESS:
{
int keycode = getKeyCode();
- if (! keycode) return false; // not a keypress event
+ if (!keycode) return false; // not a keypress event
return key::codeMap[ev.keyCode()][key::SWF] == keycode;
}
default: return false;
View
12 libcore/swf/DefineButtonTag.h
@@ -158,8 +158,6 @@ class ButtonAction
return (_conditions & KEYPRESS);
}
-private:
-
/// Return the keycode triggering this action
//
/// Return 0 if no key is supposed to trigger us
@@ -167,6 +165,8 @@ class ButtonAction
return (_conditions & KEYPRESS) >> 9;
}
+private:
+
enum condition
{
IDLE_TO_OVER_UP = 1 << 0,
@@ -250,6 +250,14 @@ class DefineButtonTag : public DefinitionTag
if (ba.triggeredBy(ev)) f(ba._actions);
}
}
+
+ template<class E>
+ void forEachAction(E& f) const {
+ for (size_t i = 0, e = _buttonActions.size(); i < e; ++i) {
+ const ButtonAction& ba = _buttonActions[i];
+ f(ba);
+ }
+ }
private:
Please sign in to comment.
Something went wrong with that request. Please try again.