Merge lp:~attente/compiz/plugin-actions into lp:compiz/0.9.11

[William Hua]

Add an interface for plugins to provide non-option key actions that can be triggered.

Add an interface for plugins to provide non-option key actions that can be triggered.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:3819
http://jenkins.qa.ubuntu.com/job/compiz-0.9.11-ci/15/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/compiz-0.9.11-trusty-amd64-ci/15/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/compiz-0.9.11-trusty-armhf-ci/15/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/compiz-0.9.11-trusty-i386-ci/15/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/compiz-0.9.11-ci/15/rebuild

review: Needs Fixing (continuous-integration)

[Christopher Townsend (townsend)]

Gmock already has a class named CompAction that is interfering with your newly created CompAction class. This is causing build failures when building the compiz_plugin_test.

Please rename your new class to something like CompizAction to avoid this.

Thanks!

review: Needs Fixing

[Christopher Townsend (townsend)]

Also, I don't quite understand what this change is for and how to test it. What am I expecting it to do?

[William Hua (attente)]

Thanks, Christopher, I'll fix the CompAction problem, as well as another bug I'm currently encountering with the branch.

Right now, (afaict) the only way for a key CompAction to be triggered is if it's set as the value of a CompOption returned by a Compiz plugin's getOptions () method. So this branch should allow plugins to return a list of key actions which are not necessarily plugin options, and those actions will be appropriately triggered by the event handling code.

I needed this because I've been trying to implement the GNOME key grabber in Unity, which will allow us to stop using passive key grabs in gnome-settings-daemon which conflict with Compiz. I have an initial implementation (there's still a couple of issues I'm trying to fix): https://launchpad.net/~attente/+archive/gnome-key-grabber.

[Christopher Townsend (townsend)]

Ok, thanks for the explanation. I have heard here and there about these changes but I'm not well versed on it. So it seems it really can't be tested until all the pieces are in place.

Once you update the branch, we'll review the code and make sure it builds, runs, etc.

[lp:~attente/compiz/plugin-actions]

3820. By William Hua on 2014-01-13

Fix test errors.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:3820
http://jenkins.qa.ubuntu.com/job/compiz-0.9.11-ci/17/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/compiz-0.9.11-trusty-amd64-ci/17/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/compiz-0.9.11-trusty-armhf-ci/17/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/compiz-0.9.11-trusty-i386-ci/17/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/compiz-0.9.11-ci/17/rebuild

review: Needs Fixing (continuous-integration)

[lp:~attente/compiz/plugin-actions]

3821. By William Hua on 2014-01-16

Merge trunk.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3821
http://jenkins.qa.ubuntu.com/job/compiz-0.9.11-ci/22/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-0.9.11-trusty-amd64-ci/22
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-0.9.11-trusty-armhf-ci/22
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-0.9.11-trusty-i386-ci/22

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/compiz-0.9.11-ci/22/rebuild

review: Approve (continuous-integration)

[Marco Trevisan (Treviño) (3v1n0)]

40 + class Class {
41 + public:
42 + virtual ~Class() {}
43 + virtual Vector & getActions () = 0;
44 + };

No better name? Also, wouldn't make sense to return a const& instead?

[William Hua (attente)]

I just realized, adding those const qualifiers to the CompAction public API may break Compiz consumers... is this something we're ok with doing? Anyone who uses those CompAction member functions will be safe, however we would break anyone who tried to re-implement those functions in a subclass...

[lp:~attente/compiz/plugin-actions]

3822. By William Hua on 2014-01-21

Rename CompAction::Class -> CompAction::Container

[William Hua (attente)]

Hi Marco, I tried to const-correct that, but it seems not possible to make getActions return a const Vector &. The reason is because the initiate and terminate callbacks of CompAction require a non-const pointer to the CompAction object, and there are places in Unity which call non-const member functions on that pointer.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3822
http://jenkins.qa.ubuntu.com/job/compiz-0.9.11-ci/25/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-0.9.11-trusty-amd64-ci/25
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-0.9.11-trusty-armhf-ci/25
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-0.9.11-trusty-i386-ci/25

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/compiz-0.9.11-ci/25/rebuild

review: Approve (continuous-integration)

[Marco Trevisan (Treviño) (3v1n0)]

Ok, that's fine then.

review: Approve

[Christopher Townsend (townsend)]

Hi William,

This is indeed an ABI break. We'll need to update the ABI version in Compiz to reflect this. To do this:

1. Open include/core/abiversion.h
2. Replace the value of CORE_ABIVERSION to today's date in form of yyyymmdd.

Other than that, this looks good. Once you get the ABI version change in, we'll top approve.

Thanks!

[lp:~attente/compiz/plugin-actions]

3823. By William Hua on 2014-01-23

Bump ABI version.

[William Hua (attente)]

Great, thanks guys! If we need any last minute changes, just let me know!

[Christopher Townsend (townsend)]

Ok, looks good to me as well.

review: Approve

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3823
http://jenkins.qa.ubuntu.com/job/compiz-0.9.11-ci/27/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-0.9.11-trusty-amd64-ci/27
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-0.9.11-trusty-armhf-ci/27
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-0.9.11-trusty-i386-ci/27

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/compiz-0.9.11-ci/27/rebuild

review: Approve (continuous-integration)

=== modified file 'include/core/abiversion.h'
--- include/core/abiversion.h	2013-11-05 18:51:36 +0000
+++ include/core/abiversion.h	2014-01-23 16:45:20 +0000
@@ -5,6 +5,6 @@
 #  error Conflicting definitions of CORE_ABIVERSION
 #endif
 
-#define CORE_ABIVERSION 20131105
+#define CORE_ABIVERSION 20140123
 
 #endif // COMPIZ_ABIVERSION_H

=== modified file 'include/core/action.h'
--- include/core/action.h	2012-10-11 10:12:05 +0000
+++ include/core/action.h	2014-01-23 16:45:20 +0000
@@ -32,8 +32,6 @@
 
 #include <boost/function.hpp>
 
-#include <X11/Xlib-xcb.h>
-
 namespace compiz
 {
 namespace actions
@@ -109,6 +107,9 @@
 		bool fromString (const CompString &str);
 		CompString toString () const;
 
+		bool operator== (const KeyBinding &k) const;
+		bool operator!= (const KeyBinding &k) const;
+
 	    private:
 		unsigned int mModifiers;
 		int          mKeycode;
@@ -126,6 +127,9 @@
 		bool fromString (const CompString &str);
 		CompString toString () const;
 
+		bool operator== (const ButtonBinding &b) const;
+		bool operator!= (const ButtonBinding &b) const;
+
 	    private:
 		unsigned int mModifiers;
 		int          mButton;
@@ -133,32 +137,41 @@
 
 	typedef unsigned int State;
 	typedef unsigned int BindingType;
+	typedef std::vector<CompAction> Vector;
 	typedef boost::function <bool (CompAction *, State, CompOption::Vector &)> CallBack;
 
+	class Container {
+	    public:
+		virtual ~Container() {}
+		virtual Vector & getActions () = 0;
+	};
+
     public:
 	CompAction ();
 	CompAction (const CompAction &);
 	~CompAction ();
 
-	CallBack initiate ();
-	CallBack terminate ();
+	CallBack initiate () const;
+	CallBack terminate () const;
 
 	void setInitiate (const CallBack &initiate);
 	void setTerminate (const CallBack &terminate);
 
-	State state ();
-	BindingType type ();
+	State state () const;
+	BindingType type () const;
 
 	KeyBinding & key ();
+	const KeyBinding & key () const;
 	void setKey (const KeyBinding &key);
 
 	ButtonBinding & button ();
+	const ButtonBinding & button () const;
 	void setButton (const ButtonBinding &button);
 
-	unsigned int edgeMask ();
+	unsigned int edgeMask () const;
 	void setEdgeMask (unsigned int edge);
 
-	bool bell ();
+	bool bell () const;
 	void setBell (bool bell);
 
 	void setState (State state);
@@ -172,13 +185,13 @@
 	bool buttonFromString (const CompString &str);
 	bool edgeMaskFromString (const CompString &str);
 
-	CompString keyToString ();
-	CompString buttonToString ();
-	CompString edgeMaskToString ();
+	CompString keyToString () const;
+	CompString buttonToString () const;
+	CompString edgeMaskToString () const;
 
 	static CompString edgeToString (unsigned int edge);
 
-	bool active ();
+	bool active () const;
 
 	/* CompAction should be a pure virtual class so
 	 * that we can pass the interface required to for setActionActiveState
@@ -191,4 +204,6 @@
 	PrivateAction *priv;
 };
 
+CompAction::Vector & noActions ();
+
 #endif

=== modified file 'include/core/plugin.h'
--- include/core/plugin.h	2013-04-17 06:58:36 +0000
+++ include/core/plugin.h	2014-01-23 16:45:20 +0000
@@ -27,6 +27,7 @@
 #define _COMPIZ_PLUGIN_H
 
 #include <core/string.h>
+#include <core/action.h>
 #include <core/option.h>
 #include <core/privateunion.h>
 #include <core/pluginclasshandler.h>
@@ -146,6 +147,8 @@
 
 		virtual bool setOption (const CompString  &name,
 					CompOption::Value &value);
+
+		virtual CompAction::Vector & getActions ();
 	    private:
 		CompString   mName;
 		VTable       **mSelf;
@@ -167,6 +170,7 @@
 		CompOption::Vector & getOptions ();
 		bool setOption (const CompString &name,
 				CompOption::Value &value);
+		CompAction::Vector & getActions ();
 
 	    private:
 
@@ -190,6 +194,7 @@
 		void finiScreen (CompScreen *s);
 		CompOption::Vector & getOptions ();
 		bool setOption (const CompString &name, CompOption::Value &value);
+		CompAction::Vector & getActions ();
 
 	    private:
 
@@ -364,6 +369,15 @@
     return oc->setOption (name, value);
 }
 
+template <typename T, typename T2, int ABI>
+CompAction::Vector & CompPlugin::VTableForScreenAndWindow<T, T2, ABI>::getActions ()
+{
+    CompAction::Container *ac = dynamic_cast<CompAction::Container *> (T::get (screen));
+    if (!ac)
+	return noActions ();
+    return ac->getActions ();
+}
+
 /**
  * Mark the plugin class handlers as ready to be initialized
  */
@@ -435,6 +449,16 @@
     return oc->setOption (name, value);
 }
 
+template <typename T, int ABI>
+CompAction::Vector &
+CompPlugin::VTableForScreen<T, ABI>::getActions ()
+{
+    CompAction::Container *ac = dynamic_cast<CompAction::Container *> (T::get (screen));
+    if (!ac)
+	return noActions ();
+    return ac->getActions ();
+}
+
 typedef CompPlugin::VTable *(*PluginGetInfoProc) (void);
 
 #endif

=== modified file 'src/action.cpp'
--- src/action.cpp	2013-11-25 20:54:05 +0000
+++ src/action.cpp	2014-01-23 16:45:20 +0000
@@ -242,6 +242,18 @@
     return binding;
 }
 
+bool
+CompAction::KeyBinding::operator== (const KeyBinding &k) const
+{
+    return mKeycode == k.mKeycode && mModifiers == k.mModifiers;
+}
+
+bool
+CompAction::KeyBinding::operator!= (const KeyBinding &k) const
+{
+    return !(*this == k);
+}
+
 CompAction::ButtonBinding::ButtonBinding () :
     mModifiers (0),
     mButton (0)
@@ -314,6 +326,18 @@
     return binding;
 }
 
+bool
+CompAction::ButtonBinding::operator== (const ButtonBinding &b) const
+{
+    return mButton == b.mButton && mModifiers == b.mModifiers;
+}
+
+bool
+CompAction::ButtonBinding::operator!= (const ButtonBinding &b) const
+{
+    return !(*this == b);
+}
+
 CompAction::CompAction () :
     priv (new PrivateAction ())
 {
@@ -330,13 +354,13 @@
 }
 
 CompAction::CallBack
-CompAction::initiate ()
+CompAction::initiate () const
 {
     return priv->initiate;
 }
 
 CompAction::CallBack
-CompAction::terminate ()
+CompAction::terminate () const
 {
     return priv->terminate;
 }
@@ -354,13 +378,13 @@
 }
 
 CompAction::State
-CompAction::state ()
+CompAction::state () const
 {
     return priv->state;
 }
 
 CompAction::BindingType
-CompAction::type ()
+CompAction::type () const
 {
     return priv->type;
 }
@@ -371,6 +395,12 @@
     return priv->key;
 }
 
+const CompAction::KeyBinding &
+CompAction::key () const
+{
+    return priv->key;
+}
+
 void
 CompAction::setKey (const CompAction::KeyBinding &key)
 {
@@ -388,6 +418,12 @@
     return priv->button;
 }
 
+const CompAction::ButtonBinding &
+CompAction::button () const
+{
+    return priv->button;
+}
+
 void
 CompAction::setButton (const CompAction::ButtonBinding &button)
 {
@@ -405,7 +441,7 @@
 }
 
 unsigned int
-CompAction::edgeMask ()
+CompAction::edgeMask () const
 {
     return priv->edgeMask;
 }
@@ -426,7 +462,7 @@
 }
 
 bool
-CompAction::bell ()
+CompAction::bell () const
 {
     return priv->bell;
 }
@@ -558,7 +594,7 @@
 }
 
 CompString
-CompAction::keyToString ()
+CompAction::keyToString () const
 {
     CompString binding = priv->key.toString ();
 
@@ -569,7 +605,7 @@
 }
 
 CompString
-CompAction::buttonToString ()
+CompAction::buttonToString () const
 {
     CompString edge;
 
@@ -584,7 +620,7 @@
 }
 
 CompString
-CompAction::edgeMaskToString ()
+CompAction::edgeMaskToString () const
 {
     CompString edge;
 
@@ -609,7 +645,7 @@
 }
 
 bool
-CompAction::active ()
+CompAction::active () const
 {
     return priv->active;
 }
@@ -654,3 +690,10 @@
 {
     memcpy (&priv, &a.priv, sizeof (CompPrivate));
 }
+
+CompAction::Vector &
+noActions ()
+{
+    static CompAction::Vector v;
+    return v;
+}

=== modified file 'src/event.cpp'
--- src/event.cpp	2013-11-22 15:11:24 +0000
+++ src/event.cpp	2014-01-23 16:45:20 +0000
@@ -98,6 +98,23 @@
 		       Mod3Mask | Mod4Mask | Mod5Mask | CompNoMask)
 
 static bool
+isCallBackBinding (CompAction	           &action,
+		   CompAction::BindingType type,
+		   CompAction::State       state)
+{
+    if (!(action.type () & type))
+	return false;
+
+    if (!(action.state () & state))
+	return false;
+
+    if (!action.active ())
+	return false;
+
+    return true;
+}
+
+static bool
 isCallBackBinding (CompOption	           &option,
 		   CompAction::BindingType type,
 		   CompAction::State       state)
@@ -105,16 +122,7 @@
     if (!option.isAction ())
 	return false;
 
-    if (!(option.value ().action ().type () & type))
-	return false;
-
-    if (!(option.value ().action ().state () & state))
-	return false;
-
-    if (!option.value ().action ().active ())
-	return false;
-
-    return true;
+    return isCallBackBinding (option.value ().action (), type, state);
 }
 
 static bool
@@ -135,6 +143,14 @@
 }
 
 static bool
+isBound (CompAction             &action,
+         CompAction::BindingType type,
+         CompAction::State       state)
+{
+    return isCallBackBinding (action, type, state);
+}
+
+static bool
 isBound (CompOption             &option,
          CompAction::BindingType type,
          CompAction::State       state,
@@ -379,19 +395,53 @@
 }
 
 bool
+PrivateScreen::shouldTriggerKeyPressAction (CompAction *action,
+					    XKeyEvent  *event)
+{
+    unsigned int modMask = REAL_MOD_MASK & ~modHandler->ignoredModMask ();
+    unsigned int bindMods = modHandler->virtualToRealModMask (action->key ().modifiers ());
+    int          bindCode = action->key ().keycode ();
+    unsigned int bindCodeMod = bindCode ? modHandler->keycodeToModifiers (bindCode) : 0;
+    unsigned int eventMods = event->state & modMask;
+
+    if (bindCode == (int) event->keycode)
+	return (bindMods & modMask) == eventMods;
+    else if (!xkbEvent.get() && bindCode == 0)
+	return bindMods == eventMods;
+    else if (bindCodeMod != 0)
+    {
+	for (int mod = 0; mod < 8; mod++)
+	{
+	    int mask = 1 << mod;
+
+	    if ((bindMods & mask) == 0)
+		continue;
+
+	    for (int k = mod * modHandler->modMap ()->max_keypermod;
+		 k < (mod + 1) * modHandler->modMap ()->max_keypermod;
+		 k++)
+	    {
+		if (modHandler->modMap ()->modifiermap[k] == event->keycode)
+		{
+		    if ((((bindMods & ~mask) | bindCodeMod) & modMask) == eventMods)
+			return true;
+
+		    break;
+		}
+	    }
+	}
+    }
+
+    return false;
+}
+
+bool
 PrivateScreen::triggerKeyPressBindings (CompOption::Vector &options,
+					CompAction::Vector &actions,
 					XKeyEvent          *event,
 					CompOption::Vector &arguments)
 {
     CompAction::State state = 0;
-    CompAction	      *action;
-    unsigned int      modMask = REAL_MOD_MASK & ~modHandler->ignoredModMask ();
-    unsigned int      modifierForBindCode;
-    unsigned int      bindMods;
-    int               bindCode;
-    int               mask;
-    int               mod;
-    int               k;
 
     if (event->keycode == escapeKeyCode)
 	state = CompAction::StateCancel;
@@ -410,6 +460,12 @@
 	    }
 	}
 
+	foreach (CompAction &a, actions)
+	{
+	    if (!a.terminate ().empty ())
+		a.terminate () (&a, state, noOptions ());
+	}
+
 	if (state == CompAction::StateCancel)
 	    return false;
     }
@@ -417,103 +473,114 @@
     state = CompAction::StateInitKey;
     foreach (CompOption &option, options)
     {
-	if (isBound (option, CompAction::BindingTypeKey, state, &action))
-	{
-	    bindCode = action->key ().keycode ();
-	    bindMods = modHandler->virtualToRealModMask (action->key ().modifiers ());
-
-	    if (bindCode == 0)
-		modifierForBindCode = 0;
-	    else
-		modifierForBindCode = modHandler->keycodeToModifiers (bindCode);
-
-	    bool match = false;
-	    if (bindCode == (int) event->keycode)
-		match = ((bindMods & modMask) == (event->state & modMask));
-	    else if (!xkbEvent.get() && bindCode == 0)
-		match = (bindMods == (event->state & modMask));
-	    else if (modifierForBindCode != 0)
-	    {
-		for (mod = 0; mod < 8; mod++)
-		{
-		    mask = 1 << mod;
-
-		    if ((bindMods & mask) == 0)
-			continue;
-
-		    for (k = mod * modHandler->modMap ()->max_keypermod;
-			 k < (mod + 1) * modHandler->modMap ()->max_keypermod;
-			 k++)
-		    {
-			if (modHandler->modMap ()->modifiermap[k] == event->keycode)
-			{
-			    match = ((((bindMods & ~mask) | modifierForBindCode) & modMask) == (event->state & modMask));
-			    break;
-			}
-		    }
-
-		    if (match)
-			break;
-		}
-	    }
-
-	    if (match && eventManager.triggerPress (action, state, arguments))
-		return true;
-	}
+	CompAction *action;
+
+	if (isBound (option, CompAction::BindingTypeKey, state, &action) &&
+	    shouldTriggerKeyPressAction (action, event) &&
+	    eventManager.triggerPress (action, state, arguments))
+		return true;
+    }
+
+    foreach (CompAction &action, actions)
+    {
+	if (isBound (action, CompAction::BindingTypeKey, state) &&
+	    shouldTriggerKeyPressAction (&action, event) &&
+	    eventManager.triggerPress (&action, state, arguments))
+		return true;
     }
 
     return false;
 }
 
 bool
+PrivateScreen::shouldTriggerKeyReleaseAction (CompAction *action,
+					      XKeyEvent  *event)
+{
+    unsigned int modMask = REAL_MOD_MASK & ~modHandler->ignoredModMask ();
+    unsigned int bindMods = modHandler->virtualToRealModMask (action->key ().modifiers ());
+
+    if ((bindMods & modMask) == 0)
+	return (unsigned int) action->key ().keycode () == event->keycode;
+
+    unsigned int mods = modHandler->keycodeToModifiers (event->keycode);
+    return !xkbEvent.get() && ((mods & modMask & bindMods) != bindMods);
+}
+
+bool
 PrivateScreen::triggerKeyReleaseBindings (CompOption::Vector &options,
+					  CompAction::Vector &actions,
 					  XKeyEvent          *event,
 					  CompOption::Vector &arguments)
 {
     CompAction::State state = CompAction::StateTermKey;
-    CompAction        *action;
-    unsigned int      ignored = modHandler->ignoredModMask ();
-    unsigned int      modMask = REAL_MOD_MASK & ~ignored;
-    unsigned int      bindMods;
-    unsigned int      mods;
 
-    mods = modHandler->keycodeToModifiers (event->keycode);
-    if (!xkbEvent.get() && !mods)
+    if (!xkbEvent.get() && !modHandler->keycodeToModifiers (event->keycode))
 	return false;
 
     bool handled = false;
 
     foreach (CompOption &option, options)
     {
-	if (isBound (option, CompAction::BindingTypeKey, state, &action))
-	{
-	    bindMods = modHandler->virtualToRealModMask (action->key ().modifiers ());
-
-	    bool match = false;
-	    if ((bindMods & modMask) == 0)
-		match = ((unsigned int) action->key ().keycode () ==
-		         (unsigned int) event->keycode);
-	    else if (!xkbEvent.get() && ((mods & modMask & bindMods) != bindMods))
-	        match = true;
-
-	    handled |= match && eventManager.triggerRelease (action, state, arguments);
-	}
+	CompAction *action;
+
+	if (isBound (option, CompAction::BindingTypeKey, state, &action) &&
+	    shouldTriggerKeyReleaseAction (action, event))
+		handled |= eventManager.triggerRelease (action, state, arguments);
+    }
+
+    foreach (CompAction &action, actions)
+    {
+	if (isBound (action, CompAction::BindingTypeKey, state) &&
+	    shouldTriggerKeyReleaseAction (&action, event))
+		handled |= eventManager.triggerRelease (&action, state, arguments);
     }
 
     return handled;
 }
 
 bool
+PrivateScreen::shouldTriggerModifierPressAction (CompAction          *action,
+						 XkbStateNotifyEvent *event)
+{
+    if (action->key ().keycode ())
+	return false;
+
+    unsigned int modMask = REAL_MOD_MASK & ~modHandler->ignoredModMask ();
+    unsigned int bindMods = modHandler->virtualToRealModMask (action->key ().modifiers ());
+    unsigned int eventMods = event->mods & modMask;
+
+    return eventMods == bindMods;
+}
+
+bool
+PrivateScreen::shouldTriggerModifierReleaseAction (CompAction          *action,
+						   XkbStateNotifyEvent *event)
+{
+    unsigned int modMask = REAL_MOD_MASK & ~modHandler->ignoredModMask ();
+    unsigned int bindMods = modHandler->virtualToRealModMask (action->key ().modifiers ());
+
+    if (event->mods && ((event->mods & modMask) != bindMods))
+	return true;
+
+    unsigned int eventCodeMod = modHandler->keycodeToModifiers (event->keycode);
+
+    if (!event->mods && (eventCodeMod == bindMods))
+	return true;
+
+    int bindCode = action->key ().keycode ();
+    unsigned int bindCodeMod = bindCode ? modHandler->keycodeToModifiers (bindCode) : 0;
+
+    return bindCodeMod && event->keycode == bindCode &&
+	 ((bindMods | bindCodeMod) == ((event->mods & modMask) | eventCodeMod));
+}
+
+bool
 PrivateScreen::triggerStateNotifyBindings (CompOption::Vector  &options,
+					   CompAction::Vector  &actions,
 					   XkbStateNotifyEvent *event,
 					   CompOption::Vector  &arguments)
 {
     CompAction::State state;
-    CompAction        *action;
-    unsigned int      ignored = modHandler->ignoredModMask ();
-    unsigned int      modMask = REAL_MOD_MASK & ~ignored;
-    unsigned int      bindMods;
-    int               bindCode;
 
     if (event->event_type == KeyPress)
     {
@@ -521,20 +588,20 @@
 
 	foreach (CompOption &option, options)
 	{
-	    if (isBound (option, CompAction::BindingTypeKey, state, &action))
-	    {
-		if (action->key ().keycode () == 0)
-		{
-		    bindMods =
-			modHandler->virtualToRealModMask (action->key ().modifiers ());
-
-		    if ((event->mods & modMask) == bindMods)
-		    {
-		        if (eventManager.triggerPress (action, state, arguments))
-			    return true;
-		    }
-		}
-	    }
+	    CompAction *action;
+
+	    if (isBound (option, CompAction::BindingTypeKey, state, &action) &&
+		shouldTriggerModifierPressAction (action, event) &&
+		eventManager.triggerPress (action, state, arguments))
+		    return true;
+	}
+
+	foreach (CompAction &action, actions)
+	{
+	    if (isBound (action, CompAction::BindingTypeKey, state) &&
+		shouldTriggerModifierPressAction (&action, event) &&
+		eventManager.triggerPress (&action, state, arguments))
+		    return true;
 	}
     }
     else if (event->event_type == KeyRelease)
@@ -544,21 +611,18 @@
 
 	foreach (CompOption &option, options)
 	{
-	    if (isBound (option, CompAction::BindingTypeKey, state, &action))
-	    {
-		bindCode = action->key ().keycode ();
-		bindMods = modHandler->virtualToRealModMask (action->key ().modifiers ());
-		unsigned int modifierForBindCode = bindCode ? modHandler->keycodeToModifiers (bindCode) : 0;
-		unsigned int modifierForEventCode = modHandler->keycodeToModifiers (event->keycode);
+	    CompAction *action;
 
-		if ((event->mods && ((event->mods & modMask) != bindMods)) ||
-		    (!event->mods && (modifierForEventCode == bindMods)) ||
-		    (modifierForBindCode && event->keycode == bindCode &&
-		     ((bindMods | modifierForBindCode) == ((event->mods & modMask) | modifierForEventCode))))
-		{
+	    if (isBound (option, CompAction::BindingTypeKey, state, &action) &&
+		shouldTriggerModifierReleaseAction (action, event))
 		    handled |= eventManager.triggerRelease (action, state, arguments);
-		}
-	    }
+	}
+
+	foreach (CompAction &action, actions)
+	{
+	    if (isBound (action, CompAction::BindingTypeKey, state) &&
+		shouldTriggerModifierReleaseAction (&action, event))
+		    handled |= eventManager.triggerRelease (&action, state, arguments);
 	}
 
 	if (handled)
@@ -880,7 +944,8 @@
 	foreach (CompPlugin *p, CompPlugin::getPlugins ())
 	{
 	    CompOption::Vector &options = p->vTable->getOptions ();
-	    if (triggerKeyPressBindings (options, &event->xkey, o))
+	    CompAction::Vector &actions = p->vTable->getActions ();
+	    if (triggerKeyPressBindings (options, actions, &event->xkey, o))
 		return true;
 	}
 	break;
@@ -904,7 +969,8 @@
 	foreach (CompPlugin *p, CompPlugin::getPlugins ())
 	{
 	    CompOption::Vector &options = p->vTable->getOptions ();
-	    handled |= triggerKeyReleaseBindings (options, &event->xkey, o);
+	    CompAction::Vector &actions = p->vTable->getActions ();
+	    handled |= triggerKeyReleaseBindings (options, actions, &event->xkey, o);
 	}
 
         if (handled)
@@ -1110,7 +1176,8 @@
 		foreach (CompPlugin *p, CompPlugin::getPlugins ())
 		{
 		    CompOption::Vector &options = p->vTable->getOptions ();
-		    handled |= triggerStateNotifyBindings (options, stateEvent, arg);
+		    CompAction::Vector &actions = p->vTable->getActions ();
+		    handled |= triggerStateNotifyBindings (options, actions, stateEvent, arg);
 		}
 
 		if (handled)

=== modified file 'src/plugin.cpp'
--- src/plugin.cpp	2013-06-28 01:10:57 +0000
+++ src/plugin.cpp	2014-01-23 16:45:20 +0000
@@ -638,3 +638,9 @@
 {
     return false;
 }
+
+CompAction::Vector &
+CompPlugin::VTable::getActions ()
+{
+    return noActions ();
+}

=== modified file 'src/plugin/tests/test-plugin.cpp'
--- src/plugin/tests/test-plugin.cpp	2013-06-28 01:10:57 +0000
+++ src/plugin/tests/test-plugin.cpp	2014-01-23 16:45:20 +0000
@@ -13,11 +13,6 @@
 #include <gtest_shared_tmpenv.h>
 #include <gtest_shared_autodestroy.h>
 
-/* This is a link-seam so that we don't have to include screen.h */
-class CompAction
-{
-};
-
 class CompMatch
 {
 };

=== modified file 'src/privatescreen.h'
--- src/privatescreen.h	2013-10-31 15:59:14 +0000
+++ src/privatescreen.h	2014-01-23 16:45:20 +0000
@@ -624,15 +624,30 @@
 					   XButtonEvent       *event,
 					   CompOption::Vector &arguments);
 
+	bool shouldTriggerKeyPressAction (CompAction *action,
+					  XKeyEvent  *event);
+
+	bool shouldTriggerKeyReleaseAction (CompAction *action,
+					    XKeyEvent  *event);
+
+	bool shouldTriggerModifierPressAction (CompAction          *action,
+					       XkbStateNotifyEvent *event);
+
+	bool shouldTriggerModifierReleaseAction (CompAction          *action,
+						 XkbStateNotifyEvent *event);
+
 	bool triggerKeyPressBindings (CompOption::Vector &options,
+				      CompAction::Vector &actions,
 				      XKeyEvent          *event,
 				      CompOption::Vector &arguments);
 
 	bool triggerKeyReleaseBindings (CompOption::Vector &options,
+					CompAction::Vector &actions,
 					XKeyEvent          *event,
 					CompOption::Vector &arguments);
 
 	bool triggerStateNotifyBindings (CompOption::Vector  &options,
+					 CompAction::Vector  &actions,
 					 XkbStateNotifyEvent *event,
 					 CompOption::Vector  &arguments);