Merge lp:~compiz-team/compiz/compiz.fix_1053280 into lp:compiz/0.9.8

[Sam Spilsbury]

Don't trigger actions unless they are actually marked active. It is possible that we may have a passive button grab for other reasons than actions being bound, so actions which haven't been added should be ignored.

Tests added for refactored code:

 Start 107: privatescreen_ButtonPressEdgeEventManagementTest.IgnoreWhenEventAndRootWindowMismatch
107/548 Test #107: privatescreen_ButtonPressEdgeEventManagementTest.IgnoreWhenEventAndRootWindowMismatch ....................................................................... Passed 0.01 sec
        Start 108: privatescreen_ButtonPressEdgeEventManagementTest.IgnoreWhenEventMismatchAndNoGrabs
108/548 Test #108: privatescreen_ButtonPressEdgeEventManagementTest.IgnoreWhenEventMismatchAndNoGrabs .......................................................................... Passed 0.01 sec
        Start 109: privatescreen_ButtonPressEdgeEventManagementTest.AllowWhenEventButNotRootWindowMismatchWhileGrabbed
109/548 Test #109: privatescreen_ButtonPressEdgeEventManagementTest.AllowWhenEventButNotRootWindowMismatchWhileGrabbed ......................................................... Passed 0.01 sec
        Start 110: privatescreen_ButtonPressEdgeEventManagementTest.NoTriggerOnInvalidEdge
110/548 Test #110: privatescreen_ButtonPressEdgeEventManagementTest.NoTriggerOnInvalidEdge ..................................................................................... Passed 0.01 sec
        Start 111: privatescreen_ButtonPressEdgeEventManagementTest.NoTriggerOnUnboundAction
111/548 Test #111: privatescreen_ButtonPressEdgeEventManagementTest.NoTriggerOnUnboundAction ................................................................................... Passed 0.01 sec
        Start 112: privatescreen_ButtonPressEdgeEventManagementTest.NoTriggerOnMismatchedEdgeMask
112/548 Test #112: privatescreen_ButtonPressEdgeEventManagementTest.NoTriggerOnMismatchedEdgeMask .............................................................................. Passed 0.01 sec
        Start 113: privatescreen_ButtonPressEdgeEventManagementTest.NoTriggerOnMismatchedButtonNumber
113/548 Test #113: privatescreen_ButtonPressEdgeEventManagementTest.NoTriggerOnMismatchedButtonNumber .......................................................................... Passed 0.01 sec
        Start 114: privatescreen_ButtonPressEdgeEventManagementTest.NoTriggerOnMismatchedButtonState
114/548 Test #114: privatescreen_ButtonPressEdgeEventManagementTest.NoTriggerOnMismatchedButtonState ........................................................................... Passed 0.01 sec
        Start 115: privatescreen_ButtonPressEdgeEventManagementTest.TriggerWhenStateButtonAndEdgeMaskMatch
115/548 Test #115: privatescreen_ButtonPressEdgeEventManagementTest.TriggerWhenStateButtonAndEdgeMaskMatch ..................................................................... Passed 0.01 sec
        Start 116: privatescreen_ButtonPressEventManagementTest.SetEventWindowArgument
116/548 Test #116: privatescreen_ButtonPressEventManagementTest.SetEventWindowArgument ......................................................................................... Passed 0.01 sec
        Start 117: privatescreen_ButtonPressEventManagementTest.NoTriggerOnUnboundAction
117/548 Test #117: privatescreen_ButtonPressEventManagementTest.NoTriggerOnUnboundAction ....................................................................................... Passed 0.01 sec
        Start 118: privatescreen_ButtonPressEventManagementTest.NoTriggerOnUnboundInactiveAction
118/548 Test #118: privatescreen_ButtonPressEventManagementTest.NoTriggerOnUnboundInactiveAction ............................................................................... Passed 0.01 sec
        Start 119: privatescreen_ButtonPressEventManagementTest.NoTriggerOnMismatchedButtonNumber
119/548 Test #119: privatescreen_ButtonPressEventManagementTest.NoTriggerOnMismatchedButtonNumber .............................................................................. Passed 0.01 sec
        Start 120: privatescreen_ButtonPressEventManagementTest.NoTriggerOnMismatchedButtonState
120/548 Test #120: privatescreen_ButtonPressEventManagementTest.NoTriggerOnMismatchedButtonState ............................................................................... Passed 0.01 sec
        Start 121: privatescreen_ButtonPressEventManagementTest.TriggerWhenStateAndButtonMatch
121/548 Test #121: privatescreen_ButtonPressEventManagementTest.TriggerWhenStateAndButtonMatch ................................................................................. Passed 0.01 sec

[jenkins (martin-mrazik+qa)]

FAILED: Continuous integration, rev:3390
http://jenkins.qa.ubuntu.com/job/compiz-ci/95/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/compiz-ci/./build=pbuilder,distribution=quantal,flavor=amd64/95/console

review: Needs Fixing (continuous-integration)

[Martin Mrazik (mrazik)]

This is the jenkins error:
[ 14%] Building CXX object src/CMakeFiles/compiz_core.dir/event.cpp.o
cd /tmp/buildd/compiz-0.9.8.2+bzr3377/obj-x86_64-linux-gnu/src && /usr/lib/ccache/c++ -Dcompiz_core_EXPORTS -DHAVE_SCANDIR_POSIX -DHAVE_SCANDIR_POSIX -DHAVE_SCANDIR_POSIX -DHAVE_CONFIG_H -g -O2 -fstack-protector --param=ssp-buffer-size=4 -Wformat -Werror=format-security -D_FORTIFY_SOURCE=2 -fPIC -Wall -Wl,-zdefs -Wno-deprecated-declarations -Werror -fPIC -Wall -Wl,-zdefs -Wno-deprecated-declarations -Werror -fPIC -Wall -Wl,-zdefs -Wno-deprecated-declarations -Werror -O2 -g -fPIC -I/tmp/buildd/compiz-0.9.8.2+bzr3377/include -I/tmp/buildd/compiz-0.9.8.2+bzr3377/obj-x86_64-linux-gnu -I/tmp/buildd/compiz-0.9.8.2+bzr3377/obj-x86_64-linux-gnu/generated -I/usr/include/libxml2 -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -I/usr/include/glibmm-2.4 -I/usr/lib/x86_64-linux-gnu/glibmm-2.4/include -I/usr/include/sigc++-2.0 -I/usr/lib/x86_64-linux-gnu/sigc++-2.0/include -I/usr/include/startup-notification-1.0 -I/include -I/tmp/buildd/compiz-0.9.8.2+bzr3377/src -I/tmp/buildd/compiz-0.9.8.2+bzr3377/src/string/include -I/tmp/buildd/compiz-0.9.8.2+bzr3377/src/string/src -I/tmp/buildd/compiz-0.9.8.2+bzr3377/src/logmessage/include -I/tmp/buildd/compiz-0.9.8.2+bzr3377/src/logmessage/src -I/tmp/buildd/compiz-0.9.8.2+bzr3377/src/timer/include -I/tmp/buildd/compiz-0.9.8.2+bzr3377/src/timer/src -I/tmp/buildd/compiz-0.9.8.2+bzr3377/src/pluginclasshandler/include -I/tmp/buildd/compiz-0.9.8.2+bzr3377/src/pluginclasshandler/src -I/tmp/buildd/compiz-0.9.8.2+bzr3377/src/point/include -I/tmp/buildd/compiz-0.9.8.2+bzr3377/src/point/src -I/tmp/buildd/compiz-0.9.8.2+bzr3377/src/rect/include -I/tmp/buildd/compiz-0.9.8.2+bzr3377/src/rect/src -I/tmp/buildd/compiz-0.9.8.2+bzr3377/src/servergrab/include -I/tmp/buildd/compiz-0.9.8.2+bzr3377/src/servergrab/src -I/tmp/buildd/compiz-0.9.8.2+bzr3377/src/region/include -I/tmp/buildd/compiz-0.9.8.2+bzr3377/src/region/src -I/tmp/buildd/compiz-0.9.8.2+bzr3377/src/window/geometry/include -I/tmp/buildd/compiz-0.9.8.2+bzr3377/src/window/geometry/src -I/tmp/buildd/compiz-0.9.8.2+bzr3377/src/window/geometry-saver/include -I/tmp/buildd/compiz-0.9.8.2+bzr3377/src/window/geometry-saver/src -I/tmp/buildd/compiz-0.9.8.2+bzr3377/src/window/extents/include -I/tmp/buildd/compiz-0.9.8.2+bzr3377/src/window/extents/src -I/tmp/buildd/compiz-0.9.8.2+bzr3377/src/window/constrainment/include -I/tmp/buildd/compiz-0.9.8.2+bzr3377/src/window/constrainment/src -DPLUGINDIR=\"/usr/lib/compiz\" -DSHAREDIR=\"/usr/share/compiz/\" -DMETADATADIR=\"/usr/share/compiz\" -o CMakeFiles/compiz_core.dir/event.cpp.o -c /tmp/buildd/compiz-0.9.8.2+bzr3377/src/event.cpp
/tmp/buildd/compiz-0.9.8.2+bzr3377/src/event.cpp:44:29: fatal error: eventmanagement.h: No such file or directory
compilation terminated.
make[3]: *** [src/CMakeFiles/compiz_core.dir/event.cpp.o] Error 1
make[3]: Leaving directory `/tmp/buildd/compiz-0.9.8.2+bzr3377/obj-x86_64-linux-gnu'

[Daniel van Vugt (vanvugt)]

What's the real bug ID? I don't think it's bug 1053820.

review: Needs Fixing

[Daniel van Vugt (vanvugt)]

And after that, please set to Resubmit. I think this is too large and dangerous to squeeze in 0.9.8.

[Daniel van Vugt (vanvugt)]

Resubmit per the prereq branch.

review: Needs Resubmitting

[Daniel van Vugt (vanvugt)]

Sorry, that comment doesn't belong here. Please fix as requested above.

review: Needs Fixing

[Daniel van Vugt (vanvugt)]

Confirmed the same build failure as jenkins:
/home/dan/bzr/compiz/tmp.142/src/event.cpp:44:29: fatal error: eventmanagement.h: No such file or directory

Please also correct the bug links and branch name. Then leave it in progress or Resubmit state.

review: Needs Fixing

[Sam Spilsbury (smspillaz)]

Resubmitted with typo fixed and file added (sorry about that). Marking resubmit for now as requested, as this bug is no longer high priority.

review: Needs Resubmitting

[jenkins (martin-mrazik+qa)]

PASSED: Continuous integration, rev:3393
http://jenkins.qa.ubuntu.com/job/compiz-ci/100/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-ci/./build=pbuilder,distribution=quantal,flavor=amd64/100/console

review: Approve (continuous-integration)

[Daniel van Vugt (vanvugt)]

Resubmit... just so I don't keep forgetting and looking at this again.

review: Needs Resubmitting

[lp:~compiz-team/compiz/compiz.fix_1053280]

3394. By Sam Spilsbury on 2012-10-02

Merge lp:compiz and handle some BCI stuff

3395. By Sam Spilsbury on 2012-10-02

Constify the API a bit

3396. By Sam Spilsbury on 2012-10-02

Cleanup.

Unindent namespaces, simplify interface, add namespace alias where
appropriate.

3397. By Sam Spilsbury on 2012-10-11

Forward declare correctly

3398. By Sam Spilsbury on 2012-10-11

Merge lp:compiz

3399. By Daniel van Vugt on 2012-10-15

Fix build error/warning: Remove unused function.

Unmerged revisions

3399. By Daniel van Vugt on 2012-10-15

Fix build error/warning: Remove unused function.

3398. By Sam Spilsbury on 2012-10-11

Merge lp:compiz

3397. By Sam Spilsbury on 2012-10-11

Forward declare correctly

3396. By Sam Spilsbury on 2012-10-02

Cleanup.

Unindent namespaces, simplify interface, add namespace alias where
appropriate.

3395. By Sam Spilsbury on 2012-10-02

Constify the API a bit

3394. By Sam Spilsbury on 2012-10-02

Merge lp:compiz and handle some BCI stuff

3393. By Sam Spilsbury on 2012-09-24

Move check to the right place

3392. By Sam Spilsbury on 2012-09-24

Merge lp:compiz

3391. By Sam Spilsbury on 2012-09-24

Add missing file

3390. By Sam Spilsbury on 2012-09-21

Don't trigger actions unless they're actually added

=== modified file 'include/core/action.h'
--- include/core/action.h	2012-09-07 22:37:20 +0000
+++ include/core/action.h	2012-09-24 11:41:41 +0000
@@ -34,6 +34,14 @@
 
 #include <X11/Xlib-xcb.h>
 
+namespace compiz
+{
+    namespace actions
+    {
+	void setActionActiveState (const CompAction  &action,
+				   bool              active);
+    }
+}
 
 class PrivateAction;
 
@@ -171,7 +179,10 @@
 
 	bool active ();
 
-	friend class CompScreenImpl;
+	/* BCI: Not optimal, CompAction should implement
+	 * BoundAction instead or be a PV class */
+	friend void compiz::actions::setActionActiveState (const CompAction  &action,
+							   bool              active);
 
     private:
 	PrivateAction *priv;
 
=== modified file 'include/core/option.h'
--- include/core/option.h	2012-05-30 09:24:15 +0000
+++ include/core/option.h	2012-09-24 11:41:41 +0000
@@ -244,8 +244,10 @@
 
 	CompString name ();
 
-	Type type ();
-	Value & value ();
+	Type type () const;
+	Type type (); // BCI: drop this in 0.9.9
+	Value & value () const;
+	Value & value (); // BCI: drop this in 0.9.9
 	Restriction & rest ();
 
 	bool set (Value &val);
 
=== modified file 'src/action.cpp'
--- src/action.cpp	2012-05-10 12:58:01 +0000
+++ src/action.cpp	2012-09-24 11:41:41 +0000
@@ -33,6 +33,8 @@
 #include <core/screen.h>
 #include "privateaction.h"
 
+namespace ca = compiz::actions;
+
 struct _Modifier {
     const char *name;
     int        modifier;
@@ -626,6 +628,28 @@
     return priv->active;
 }
 
+void
+PrivateAction::makeActive ()
+{
+    active = true;
+}
+
+void
+PrivateAction::makeInactive ()
+{
+    active = false;
+}
+
+void
+ca::setActionActiveState (const CompAction &action,
+			  bool                  active)
+{
+    if (active)
+	action.priv->makeActive ();
+    else
+	action.priv->makeInactive ();
+}
+
 PrivateAction::PrivateAction () :
     initiate (),
     terminate (),
 
=== modified file 'src/event.cpp'
--- src/event.cpp	2012-09-05 16:42:03 +0000
+++ src/event.cpp	2012-09-24 11:41:41 +0000
@@ -41,8 +41,10 @@
 #include "privatescreen.h"
 #include "privatewindow.h"
 #include "privatestackdebugger.h"
+#include "eventmanagement.h"
 
 namespace cps = compiz::private_screen;
+namespace ce = compiz::events;
 
 namespace
 {
@@ -109,6 +111,9 @@
     if (!(option.value ().action ().state () & state))
 	return false;
 
+    if (!option.value ().action ().active ())
+	return false;
+
     return true;
 }
 
@@ -189,78 +194,160 @@
     return false;
 }
 
+int
+ce::processButtonPressOnEdgeWindow (Window               edgeWindow,
+				    Window               root,
+				    Window               eventWindow,
+				    Window               eventRoot,
+				    cps::GrabList        &grabList,
+				    const CompScreenEdge *screenEdge)
+{
+    int edge = -1;
+
+    if (eventRoot != root)
+	return edge;
+
+    if (eventWindow != edgeWindow)
+    {
+	if (grabList.grabsEmpty () ||
+	    eventRoot != root)
+	    return edge;
+    }
+
+    for (unsigned int i = 0; i < SCREEN_EDGE_NUM; i++)
+    {
+	if (edgeWindow == screenEdge[i].id)
+	{
+	    edge = 1 << i;
+	    break;
+	}
+    }
+
+    return edge;
+}
+
+void
+ce::setEventWindowInButtonPressArguments (ce::EventArguments &arguments,
+					  Window             eventWindow)
+{
+    arguments[1].value ().set ((int) eventWindow);
+}
+
+namespace
+{
+    bool buttonActionModifiersMatchEventState (unsigned int actionModifiers,
+					       unsigned int eventState)
+    {
+	const unsigned int ignored = modHandler->ignoredModMask ();
+	const unsigned int modMask = REAL_MOD_MASK & ~ignored;
+	const unsigned int bindMods = modHandler->virtualToRealModMask (actionModifiers);
+
+	return (bindMods & modMask) == (eventState & modMask);
+    }
+}
+
+bool
+ce::activateButtonPressOnWindowBindingOption (CompOption                            &option,
+					      unsigned int                          eventButton,
+					      unsigned int                          eventState,
+					      cps::EventManager                     &eventManager,
+					      const ActionModsMatchesEventStateFunc &matchEventState,
+					      ce::EventArguments                    &arguments)
+{
+    CompAction              *action;
+    const CompAction::State state = CompAction::StateInitButton;
+
+    if (isBound (option, CompAction::BindingTypeButton, state, &action))
+    {
+	if (action->button ().button () == (int) eventButton)
+	{
+	    if (matchEventState (action->button ().modifiers (),
+				 eventState))
+	    {
+		if (eventManager.triggerPress (action, state, arguments))
+		    return true;
+	    }
+	}
+    }
+
+    return false;
+}
+
+bool
+ce::activateButtonPressOnEdgeBindingOption (CompOption                            &option,
+					    unsigned int                          eventButton,
+					    unsigned int                          eventState,
+					    int                                   edge,
+					    cps::EventManager                     &eventManager,
+					    const ActionModsMatchesEventStateFunc &matchEventState,
+					    ce::EventArguments                    &arguments)
+{
+    CompAction              *action;
+    const CompAction::State state = CompAction::StateInitButton |
+				    CompAction::StateInitEdge;
+
+    if (edge != -1)
+    {
+	if (isInitiateBinding (option, CompAction::BindingTypeEdgeButton,
+			       state, &action))
+	{
+	    if ((action->button ().button () == (int) eventButton) &&
+		(action->edgeMask () & edge))
+	    {
+		if (matchEventState (action->button ().modifiers (),
+				     eventState))
+		    if (action->initiate () (action, state,
+					     arguments))
+			return true;
+	    }
+	}
+    }
+
+    return false;
+}
+
 bool
 PrivateScreen::triggerButtonPressBindings (CompOption::Vector &options,
 					   XButtonEvent       *event,
 					   CompOption::Vector &arguments)
 {
-    CompAction::State state = CompAction::StateInitButton;
-    CompAction        *action;
-    unsigned int      ignored = modHandler->ignoredModMask ();
-    unsigned int      modMask = REAL_MOD_MASK & ~ignored;
-    unsigned int      bindMods;
-    unsigned int      edge = 0;
+    int               edge = -1;
+
+    static const ce::ActionModsMatchesEventStateFunc matchEventState (
+		boost::bind (buttonActionModifiersMatchEventState,
+			     _1, _2));
 
     if (edgeWindow)
-    {
-	unsigned int i;
-
-	if (event->root != screen->root())
-	    return false;
-
-	if (event->window != edgeWindow)
-	{
-	    if (eventManager.grabsEmpty () || event->window != screen->root())
-		return false;
-	}
-
-	for (i = 0; i < SCREEN_EDGE_NUM; i++)
-	{
-	    if (edgeWindow == screenEdge[i].id)
-	    {
-		edge = 1 << i;
-		arguments[1].value ().set ((int) orphanData.activeWindow);
-		break;
-	    }
-	}
-    }
+	edge = ce::processButtonPressOnEdgeWindow (edgeWindow,
+						   screen->root (),
+						   event->window,
+						   event->root,
+						   eventManager,
+						   screenEdge);
+
+    if (edge != -1)
+	ce::setEventWindowInButtonPressArguments (arguments,
+						  orphanData.activeWindow);
 
     foreach (CompOption &option, options)
     {
-	if (isBound (option, CompAction::BindingTypeButton, state, &action))
-	{
-	    if (action->button ().button () == (int) event->button)
-	    {
-		bindMods = modHandler->virtualToRealModMask (
-		    action->button ().modifiers ());
-
-		if ((bindMods & modMask) == (event->state & modMask))
-		{
-		    if (eventManager.triggerPress (action, state, arguments))
-			return true;
-		}
-	    }
-	}
-
-	if (edge)
-	{
-	    if (isInitiateBinding (option, CompAction::BindingTypeEdgeButton,
-				   state | CompAction::StateInitEdge, &action))
-	    {
-		if ((action->button ().button () == (int) event->button) &&
-		    (action->edgeMask () & edge))
-		{
-		    bindMods = modHandler->virtualToRealModMask (
-			action->button ().modifiers ());
-
-		    if ((bindMods & modMask) == (event->state & modMask))
-			if (action->initiate () (action, state |
-						 CompAction::StateInitEdge,
-						 arguments))
-			    return true;
-		}
-	    }
-	}
+	if (ce::activateButtonPressOnWindowBindingOption (option,
+							  event->button,
+							  event->state,
+							  eventManager,
+							  matchEventState,
+							  arguments))
+	    return true;
+
+	if (ce::activateButtonPressOnEdgeBindingOption (option,
+							event->button,
+							event->state,
+							edge,
+							eventManager,
+							matchEventState,
+							arguments))
+	    return true;
+
     }
 
     return false;
 
=== added file 'src/eventmanagement.h'
--- src/eventmanagement.h	1970-01-01 00:00:00 +0000
+++ src/eventmanagement.h	2012-09-24 11:41:41 +0000
@@ -0,0 +1,80 @@
+/*
+ * Copyright © 2012 Canonical, Ltd.
+ *
+ * Permission to use, copy, modify, distribute, and sell this software
+ * and its documentation for any purpose is hereby granted without
+ * fee, provided that the above copyright notice appear in all copies
+ * and that both that copyright notice and this permission notice
+ * appear in supporting documentation, and that the name of
+ * Canonical, Ltd. not be used in advertising or publicity pertaining to
+ * distribution of the software without specific, written prior permission.
+ * Canonical, Ltd. makes no representations about the suitability of this
+ * software for any purpose. It is provided "as is" without express or
+ * implied warranty.
+ *
+ * CANONICAL LTD. DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
+ * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN
+ * NO EVENT SHALL Canonical, Ltd. BE LIABLE FOR ANY SPECIAL, INDIRECT OR
+ * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS
+ * OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
+ * NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION
+ * WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ *
+ * Author: Sam Spilsbury <[e-mail]>
+ */
+#ifndef _COMPIZ_EVENT_MANAGEMENT_H
+#define _COMPIZ_EVENT_MANAGEMENT_H
+
+#include <boost/function.hpp>
+
+class CompScreenEdge;
+class CompOption;
+
+namespace compiz
+{
+    namespace private_screen
+    {
+	struct GrabList;
+	class EventManager;
+    }
+
+    namespace events
+    {
+	namespace cps = compiz::private_screen;
+	typedef std::vector <CompOption> EventArguments;
+	typedef boost::function <bool (unsigned int, unsigned int)> ActionModsMatchesEventStateFunc;
+
+	int
+	processButtonPressOnEdgeWindow (Window               edgeWindow,
+				        Window               root,
+				        Window               eventWindow,
+				        Window               eventRoot,
+				        cps::GrabList        &grabList,
+				        const CompScreenEdge *screenEdge);
+
+	void
+	setEventWindowInButtonPressArguments (EventArguments &arguments,
+					      Window         eventWindow);
+
+	bool
+	activateButtonPressOnWindowBindingOption (CompOption                            &option,
+						  unsigned int                          eventButton,
+						  unsigned int                          eventState,
+						  cps::EventManager                     &eventManager,
+						  const ActionModsMatchesEventStateFunc &matchEventState,
+						  EventArguments                        &arguments);
+
+	bool
+	activateButtonPressOnEdgeBindingOption (CompOption                            &option,
+					        unsigned int                          eventButton,
+					        unsigned int                          eventState,
+					        int                                   edge,
+					        cps::EventManager                     &eventManager,
+					        const ActionModsMatchesEventStateFunc &matchEventState,
+					        EventArguments                        &arguments);
+    }
+}
+
+	
+
+#endif
 
=== modified file 'src/option.cpp'
--- src/option.cpp	2012-06-24 09:00:27 +0000
+++ src/option.cpp	2012-09-24 11:41:41 +0000
@@ -462,12 +462,24 @@
     return priv->type;
 }
 
+CompOption::Type
+CompOption::type () const
+{
+    return priv->type;
+}
+
 CompOption::Value &
 CompOption::value ()
 {
     return priv->value;
 }
 
+CompOption::Value &
+CompOption::value () const
+{
+    return priv->value;
+}
+
 CompOption::Restriction &
 CompOption::rest ()
 {
@@ -477,6 +489,12 @@
 bool
 CompOption::set (CompOption::Value &val)
 {
+    /* XXX: It is uncertain as to why this is done. The only
+     * logical reason would be that actions are stateful and
+     * we don't want to care about the old state from the
+     * action that we're setting this value to, so we're just
+     * clearing that state and starting over, however copyState
+     * does a lot more than that */
     if (isAction () && priv->type != CompOption::TypeAction)
 	val.action ().copyState (priv->value.action ());
 
 
=== modified file 'src/option/tests/option.cpp'
--- src/option/tests/option.cpp	2012-05-27 03:27:57 +0000
+++ src/option/tests/option.cpp	2012-09-24 11:41:41 +0000
@@ -206,3 +206,34 @@
 	ASSERT_EQ(expectTwo, as_const.f());
     }
 }
+
+TEST (CompOption, AssignDefaultActionValueToUnsetTypeClearsOldStateKeepsInfo)
+{
+    /* Value is unset at this point */
+    CompOption        option ("testing", CompOption::TypeKey);
+    CompAction        action;
+
+    /* We need to set up the state here as
+     * the CompOption::Value constructor makes
+     * a copy of the action */
+    action.setState (CompAction::StateInitKey);
+    action.setButton (CompAction::ButtonBinding (1,
+						 1 << 1));
+
+    CompOption::Value value (action);
+
+    ASSERT_EQ (value.action ().state (), CompAction::StateInitKey);
+
+    /* Actually set the action value, this will
+     * overwrite the internal value */
+    option.set (value);
+
+    /* We don't care about the old action's state, so get
+     * rid of it */
+    ASSERT_EQ (option.value ().action ().state (), 0);
+
+    /* We do want to keep the non-stateful data which is
+     * pure info */
+    ASSERT_EQ (option.value ().action ().button ().button (), 1);
+    ASSERT_EQ (option.value ().action ().button ().modifiers (), 1 << 1);
+}
 
=== modified file 'src/privateaction.h'
--- src/privateaction.h	2012-02-14 11:39:59 +0000
+++ src/privateaction.h	2012-09-24 11:41:41 +0000
@@ -28,11 +28,35 @@
 #ifndef _PRIVATEACTION_H
 #define _PRIVATEACTION_H
 
-class PrivateAction {
+namespace compiz
+{
+    namespace actions
+    {
+	class BoundAction
+	{
+	    public:
+
+		virtual ~BoundAction () {}
+
+		virtual void makeActive () = 0;
+		virtual void makeInactive () = 0;
+	};
+
+	void setActionActiveState (const CompAction  &action,
+				   bool              active);
+    }
+}
+
+class PrivateAction :
+    public compiz::actions::BoundAction
+{
     public:
 	PrivateAction ();
 	PrivateAction (const PrivateAction&);
 
+	void makeActive ();
+	void makeInactive ();
+
 	CompAction::CallBack initiate;
 	CompAction::CallBack terminate;
 
 
=== modified file 'src/privatescreen/tests/test-privatescreen.cpp'
--- src/privatescreen/tests/test-privatescreen.cpp	2012-06-24 09:00:27 +0000
+++ src/privatescreen/tests/test-privatescreen.cpp	2012-09-24 11:41:41 +0000
@@ -1,5 +1,6 @@
 #include "privatescreen.h"
-
+#include "privateaction.h"
+#include "eventmanagement.h"
 
 // Get rid of stupid macro from X.h
 // Why, oh why, are we including X.h?
@@ -16,6 +17,8 @@
 
 namespace {
 
+const unsigned long None = 0;
+
 class MockCompScreen : public CompScreen
 {
 public:
@@ -355,6 +358,7 @@
 } // (abstract) namespace
 
 namespace cps = compiz::private_screen;
+namespace ce = compiz::events;
 
 TEST(privatescreen_PluginManagerTest, create_and_destroy)
 {
@@ -909,3 +913,511 @@
 
     EXPECT_EQ (vp, CompPoint (0, 1));
 }
+
+namespace
+{
+    const Window topLeftScreenEdge = 1;
+    const Window topScreenEdge = 2;
+    const Window topRightScreenEdge = 3;
+    const Window rightScreenEdge = 4;
+    const Window bottomRightScreenEdge = 5;
+    const Window bottomScreenEdge = 6;
+    const Window bottomLeftScreenEdge = 7;
+    const Window leftScreenEdge = 8;
+
+    const CompScreenEdge screenEdges[SCREEN_EDGE_NUM] =
+    {
+	{ leftScreenEdge, SCREEN_EDGE_LEFT },
+	{ rightScreenEdge, SCREEN_EDGE_RIGHT },
+	{ topScreenEdge, SCREEN_EDGE_TOP },
+	{ bottomScreenEdge , SCREEN_EDGE_BOTTOM },
+	{ topLeftScreenEdge, SCREEN_EDGE_TOPLEFT },
+	{ topRightScreenEdge, SCREEN_EDGE_TOPRIGHT },
+	{ bottomLeftScreenEdge, SCREEN_EDGE_BOTTOMLEFT },
+	{ bottomRightScreenEdge, SCREEN_EDGE_BOTTOMRIGHT}
+    };
+}
+
+TEST (privatescreen_ButtonPressEdgeEventManagementTest, IgnoreWhenEventAndRootWindowMismatch)
+{
+    const Window rootWindow = 1;
+    const Window edgeWindow = topScreenEdge;
+
+    cps::OrphanData orphanData;
+    cps::GrabList grabList;
+
+    EXPECT_EQ (ce::processButtonPressOnEdgeWindow (edgeWindow,
+						   rootWindow,
+						   0,
+						   0,
+						   grabList,
+						   screenEdges), -1);
+}
+
+TEST (privatescreen_ButtonPressEdgeEventManagementTest, IgnoreWhenEventMismatchAndNoGrabs)
+{
+    const Window rootWindow = 1;
+    const Window edgeWindow = topScreenEdge;
+
+    cps::OrphanData orphanData;
+    cps::GrabList grabList;
+
+    EXPECT_EQ (ce::processButtonPressOnEdgeWindow (edgeWindow,
+						   rootWindow,
+						   0,
+						   rootWindow,
+						   grabList,
+						   screenEdges), -1);
+}
+
+TEST (privatescreen_ButtonPressEdgeEventManagementTest, AllowWhenEventButNotRootWindowMismatchWhileGrabbed)
+{
+    const Window rootWindow = 1;
+    const Window edgeWindow = topScreenEdge;
+    unsigned int topEdgeMask = 1 << SCREEN_EDGE_TOP;
+
+    cps::OrphanData orphanData;
+    cps::GrabList grabList;
+
+    grabList.grabsPush (new cps::Grab (None, "Nil"));
+
+    EXPECT_EQ (ce::processButtonPressOnEdgeWindow (edgeWindow,
+						   rootWindow,
+						   0,
+						   rootWindow,
+						   grabList,
+						   screenEdges), topEdgeMask);
+
+    grabList.grabsRemove (grabList.grabsBack ());
+}
+
+TEST (privatescreen_ButtonPressEventManagementTest, SetEventWindowArgument)
+{
+    const Window activeWindow = 1;
+
+    ce::EventArguments arguments (2);
+    ce::setEventWindowInButtonPressArguments (arguments, activeWindow);
+    EXPECT_EQ (arguments[1].value ().i (), activeWindow);
+}
+
+namespace
+{
+    class MockTriggerableAction
+    {
+	public:
+
+	    MOCK_METHOD2 (matchEventState, bool (unsigned int,
+						 unsigned int));
+	    MOCK_METHOD3 (initiate, bool (CompAction         *,
+					  CompAction::State   ,
+					  CompOption::Vector &));
+	    MOCK_METHOD3 (terminate, bool (CompAction         *,
+					   CompAction::State   ,
+					   CompOption::Vector &));
+    };
+
+    const unsigned int testingButtonNumber = 1;
+    const unsigned int testingButtonState = (1 << 1);
+
+    ce::ActionModsMatchesEventStateFunc
+    GetMatchEventStateFuncForMock (MockTriggerableAction &triggerableAction)
+    {
+	return boost::bind (&MockTriggerableAction::matchEventState,
+			    &triggerableAction,
+			    _1, _2);
+    }
+
+    CompAction::CallBack
+    GetInitiateForMock (MockTriggerableAction &triggerableAction)
+    {
+	return boost::bind (&MockTriggerableAction::initiate,
+			    &triggerableAction,
+			    _1, _2, _3);
+    }
+
+    CompAction::CallBack
+    GetTerminateForMock (MockTriggerableAction &triggerableAction)
+    {
+	return boost::bind (&MockTriggerableAction::terminate,
+			    &triggerableAction,
+			    _1, _2, _3);
+    }
+}
+
+bool operator== (const CompOption &lhs,
+		 const CompOption &rhs)
+{
+    if (lhs.type () != rhs.type ())
+	return false;
+
+    return lhs.value () == rhs.value ();
+}
+
+TEST (privatescreen_ButtonPressEventManagementTest, NoTriggerOnUnboundAction)
+{
+    CompAction            action;
+    CompOption            option ("button", CompOption::TypeButton);
+    CompOption::Value     value (action);
+    cps::EventManager     eventManager;
+    MockTriggerableAction triggerableAction;
+    ce::EventArguments    arguments;
+
+    option.set (value);
+
+    const ce::ActionModsMatchesEventStateFunc &matchEventState =
+	    GetMatchEventStateFuncForMock (triggerableAction);
+
+    EXPECT_FALSE (ce::activateButtonPressOnWindowBindingOption (option,
+								testingButtonNumber,
+								testingButtonState,
+								eventManager,
+								matchEventState,
+								arguments));
+}
+
+TEST (privatescreen_ButtonPressEventManagementTest, NoTriggerOnUnboundInactiveAction)
+{
+    CompAction            action;
+    MockTriggerableAction triggerableAction;
+    int                   edgeMask = 1 << SCREEN_EDGE_TOP;
+
+    const ce::ActionModsMatchesEventStateFunc &matchEventState =
+	    GetMatchEventStateFuncForMock (triggerableAction);
+    const CompAction::CallBack                &initiate =
+	    GetInitiateForMock (triggerableAction);
+
+    action.setButton (CompAction::ButtonBinding (testingButtonNumber,
+						 testingButtonState));
+    action.setEdgeMask (edgeMask);
+
+
+    CompOption            option ("button", CompOption::TypeButton);
+    CompOption::Value     value (action);
+    cps::EventManager     eventManager;
+    ce::EventArguments    arguments;
+
+    option.set (value);
+    option.value ().action ().setInitiate (initiate);
+    option.value ().action ().setState (CompAction::StateInitButton);
+    compiz::actions::setActionActiveState (option.value ().action (), false);
+
+    EXPECT_FALSE (ce::activateButtonPressOnEdgeBindingOption (option,
+							      testingButtonNumber,
+							      testingButtonState,
+							      edgeMask,
+							      eventManager,
+							      matchEventState,
+							      arguments));
+}
+
+TEST (privatescreen_ButtonPressEventManagementTest, NoTriggerOnMismatchedButtonNumber)
+{
+    CompAction            action;
+    MockTriggerableAction triggerableAction;
+
+    const ce::ActionModsMatchesEventStateFunc &matchEventState =
+	    GetMatchEventStateFuncForMock (triggerableAction);
+    const CompAction::CallBack                &initiate =
+	    GetInitiateForMock (triggerableAction);
+
+    action.setButton (CompAction::ButtonBinding (testingButtonNumber,
+						 testingButtonState));
+
+    CompOption            option ("button", CompOption::TypeButton);
+    CompOption::Value     value (action);
+    cps::EventManager     eventManager;
+    ce::EventArguments    arguments;
+
+    option.set (value);
+    option.value ().action ().setInitiate (initiate);
+    option.value ().action ().setState (CompAction::StateInitButton);
+    compiz::actions::setActionActiveState (option.value ().action (), true);
+
+    EXPECT_CALL (triggerableAction, initiate (_, _, _)).Times (0);
+    EXPECT_FALSE (ce::activateButtonPressOnWindowBindingOption (option,
+								0,
+								testingButtonState,
+								eventManager,
+								matchEventState,
+								arguments));
+}
+
+TEST (privatescreen_ButtonPressEventManagementTest, NoTriggerOnMismatchedButtonState)
+{
+    CompAction            action;
+    MockTriggerableAction triggerableAction;
+
+    const ce::ActionModsMatchesEventStateFunc &matchEventState =
+	    GetMatchEventStateFuncForMock (triggerableAction);
+    const CompAction::CallBack                &initiate =
+	    GetInitiateForMock (triggerableAction);
+
+    action.setButton (CompAction::ButtonBinding (testingButtonNumber,
+						 testingButtonState));
+
+    CompOption            option ("button", CompOption::TypeButton);
+    CompOption::Value     value (action);
+    cps::EventManager     eventManager;
+    ce::EventArguments    arguments;
+
+    option.set (value);
+    option.value ().action ().setInitiate (initiate);
+    option.value ().action ().setState (CompAction::StateInitButton);
+    compiz::actions::setActionActiveState (option.value ().action (), true);
+
+    EXPECT_CALL (triggerableAction, matchEventState (testingButtonState, 0))
+	    .WillOnce (Return (false));
+    EXPECT_CALL (triggerableAction, initiate (_, _, _)).Times (0);
+    EXPECT_FALSE (ce::activateButtonPressOnWindowBindingOption (option,
+								testingButtonNumber,
+								0,
+								eventManager,
+								matchEventState,
+								arguments));
+}
+
+TEST (privatescreen_ButtonPressEventManagementTest, TriggerWhenStateAndButtonMatch)
+{
+    CompAction            action;
+    MockTriggerableAction triggerableAction;
+
+    const ce::ActionModsMatchesEventStateFunc &matchEventState =
+	    GetMatchEventStateFuncForMock (triggerableAction);
+    const CompAction::CallBack                &initiate =
+	    GetInitiateForMock (triggerableAction);
+
+    action.setButton (CompAction::ButtonBinding (testingButtonNumber,
+						 testingButtonState));
+
+    CompOption            option ("button", CompOption::TypeButton);
+    CompOption::Value     value (action);
+    cps::EventManager     eventManager;
+    ce::EventArguments    arguments;
+
+    option.set (value);
+    option.value ().action ().setInitiate (initiate);
+    option.value ().action ().setState (CompAction::StateInitButton);
+    compiz::actions::setActionActiveState (option.value ().action (), true);
+
+    EXPECT_CALL (triggerableAction, matchEventState (testingButtonState, testingButtonState))
+	    .WillOnce (Return (true));
+    EXPECT_CALL (triggerableAction, initiate (&option.value ().action (),
+					      CompAction::StateInitButton,
+					      arguments)).WillOnce (Return (true));
+    EXPECT_TRUE (ce::activateButtonPressOnWindowBindingOption (option,
+							       testingButtonNumber,
+							       testingButtonState,
+							       eventManager,
+							       matchEventState,
+							       arguments));
+}
+
+TEST (privatescreen_ButtonPressEdgeEventManagementTest, NoTriggerOnInvalidEdge)
+{
+    CompAction            action;
+    CompOption            option ("button", CompOption::TypeButton);
+    CompOption::Value     value (action);
+    cps::EventManager     eventManager;
+    MockTriggerableAction triggerableAction;
+    ce::EventArguments    arguments;
+    int                   edgeMask = -1;
+
+    option.set (value);
+
+    const ce::ActionModsMatchesEventStateFunc &matchEventState =
+	    GetMatchEventStateFuncForMock (triggerableAction);
+
+    EXPECT_FALSE (ce::activateButtonPressOnEdgeBindingOption (option,
+							      testingButtonNumber,
+							      testingButtonState,
+							      edgeMask,
+							      eventManager,
+							      matchEventState,
+							      arguments));
+}
+
+TEST (privatescreen_ButtonPressEdgeEventManagementTest, NoTriggerOnUnboundAction)
+{
+    CompAction            action;
+    CompOption            option ("button", CompOption::TypeButton);
+    CompOption::Value     value (action);
+    cps::EventManager     eventManager;
+    MockTriggerableAction triggerableAction;
+    ce::EventArguments    arguments;
+    int                   edgeMask = 1 << SCREEN_EDGE_TOP;
+
+    option.set (value);
+
+    const ce::ActionModsMatchesEventStateFunc &matchEventState =
+	    GetMatchEventStateFuncForMock (triggerableAction);
+
+    EXPECT_FALSE (ce::activateButtonPressOnEdgeBindingOption (option,
+							      testingButtonNumber,
+							      testingButtonState,
+							      edgeMask,
+							      eventManager,
+							      matchEventState,
+							      arguments));
+}
+
+TEST (privatescreen_ButtonPressEdgeEventManagementTest, NoTriggerOnMismatchedEdgeMask)
+{
+    CompAction            action;
+    MockTriggerableAction triggerableAction;
+    int                   edgeMask = 1 << SCREEN_EDGE_TOP;
+
+    const ce::ActionModsMatchesEventStateFunc &matchEventState =
+	    GetMatchEventStateFuncForMock (triggerableAction);
+    const CompAction::CallBack                &initiate =
+	    GetInitiateForMock (triggerableAction);
+
+    action.setButton (CompAction::ButtonBinding (testingButtonNumber,
+						 testingButtonState));
+    action.setEdgeMask (edgeMask);
+
+    CompOption            option ("button", CompOption::TypeButton);
+    CompOption::Value     value (action);
+    cps::EventManager     eventManager;
+    ce::EventArguments    arguments;
+
+
+    option.set (value);
+    option.value ().action ().setInitiate (initiate);
+    option.value ().action ().setState (CompAction::StateInitButton);
+    compiz::actions::setActionActiveState (option.value ().action (), true);
+
+    EXPECT_CALL (triggerableAction, initiate (_, _, _)).Times (0);
+    EXPECT_FALSE (ce::activateButtonPressOnEdgeBindingOption (option,
+							      testingButtonNumber,
+							      testingButtonState,
+							      0,
+							      eventManager,
+							      matchEventState,
+							      arguments));
+}
+
+TEST (privatescreen_ButtonPressEdgeEventManagementTest, NoTriggerOnMismatchedButtonNumber)
+{
+    CompAction            action;
+    MockTriggerableAction triggerableAction;
+    int                   edgeMask = 1 << SCREEN_EDGE_TOP;
+
+    const ce::ActionModsMatchesEventStateFunc &matchEventState =
+	    GetMatchEventStateFuncForMock (triggerableAction);
+    const CompAction::CallBack                &initiate =
+	    GetInitiateForMock (triggerableAction);
+
+    action.setButton (CompAction::ButtonBinding (testingButtonNumber,
+						 testingButtonState));
+    action.setEdgeMask (edgeMask);
+
+    CompOption            option ("button", CompOption::TypeButton);
+    CompOption::Value     value (action);
+    cps::EventManager     eventManager;
+    ce::EventArguments    arguments;
+
+
+    option.set (value);
+    option.value ().action ().setInitiate (initiate);
+    option.value ().action ().setState (CompAction::StateInitButton);
+    compiz::actions::setActionActiveState (option.value ().action (), true);
+
+    EXPECT_CALL (triggerableAction, initiate (_, _, _)).Times (0);
+    EXPECT_FALSE (ce::activateButtonPressOnEdgeBindingOption (option,
+							      0,
+							      testingButtonState,
+							      edgeMask,
+							      eventManager,
+							      matchEventState,
+							      arguments));
+}
+
+TEST (privatescreen_ButtonPressEdgeEventManagementTest, NoTriggerOnMismatchedButtonState)
+{
+    CompAction            action;
+    MockTriggerableAction triggerableAction;
+    int                   edgeMask = 1 << SCREEN_EDGE_TOP;
+
+    const ce::ActionModsMatchesEventStateFunc &matchEventState =
+	    GetMatchEventStateFuncForMock (triggerableAction);
+    const CompAction::CallBack                &initiate =
+	    GetInitiateForMock (triggerableAction);
+
+    action.setButton (CompAction::ButtonBinding (testingButtonNumber,
+						 testingButtonState));
+    action.setEdgeMask (edgeMask);
+
+
+    CompOption            option ("button", CompOption::TypeButton);
+    CompOption::Value     value (action);
+    cps::EventManager     eventManager;
+    ce::EventArguments    arguments;
+
+    option.set (value);
+    option.value ().action ().setInitiate (initiate);
+    option.value ().action ().setState (CompAction::StateInitButton);
+    compiz::actions::setActionActiveState (option.value ().action (), true);
+
+    EXPECT_CALL (triggerableAction, matchEventState (testingButtonState, 0))
+	    .WillOnce (Return (false));
+    EXPECT_CALL (triggerableAction, initiate (_, _, _)).Times (0);
+    EXPECT_FALSE (ce::activateButtonPressOnEdgeBindingOption (option,
+							      testingButtonNumber,
+							      0,
+							      edgeMask,
+							      eventManager,
+							      matchEventState,
+							      arguments));
+}
+
+TEST (privatescreen_ButtonPressEdgeEventManagementTest, TriggerWhenStateButtonAndEdgeMaskMatch)
+{
+    CompAction            action;
+    MockTriggerableAction triggerableAction;
+    int                   edgeMask = 1 << SCREEN_EDGE_TOP;
+
+    const ce::ActionModsMatchesEventStateFunc &matchEventState =
+	    GetMatchEventStateFuncForMock (triggerableAction);
+    const CompAction::CallBack                &initiate =
+	    GetInitiateForMock (triggerableAction);
+
+    action.setButton (CompAction::ButtonBinding (testingButtonNumber,
+						 testingButtonState));
+    action.setEdgeMask (edgeMask);
+
+    CompOption            option ("button", CompOption::TypeButton);
+    CompOption::Value     value (action);
+    cps::EventManager     eventManager;
+    ce::EventArguments    arguments;
+
+    option.set (value);
+    option.value ().action ().setInitiate (initiate);
+    option.value ().action ().setState (CompAction::StateInitButton |
+					CompAction::StateInitEdge);
+    compiz::actions::setActionActiveState (option.value ().action (), true);
+
+    EXPECT_CALL (triggerableAction, matchEventState (testingButtonState, testingButtonState))
+	    .WillOnce (Return (true));
+    EXPECT_CALL (triggerableAction, initiate (&option.value ().action (),
+					      CompAction::StateInitButton |
+					      CompAction::StateInitEdge,
+					      arguments)).WillOnce (Return (true));
+    EXPECT_TRUE (ce::activateButtonPressOnEdgeBindingOption (option,
+							     testingButtonNumber,
+							     testingButtonState,
+							     edgeMask,
+							     eventManager,
+							     matchEventState,
+							     arguments));
+}
+
+TEST (privatescreen_ActionActiveTest, TestMakeActionActive)
+{
+    CompAction action;
+
+    ASSERT_EQ (action.active (), false);
+    compiz::actions::setActionActiveState (action, true);
+    ASSERT_EQ (action.active (), true);
+    compiz::actions::setActionActiveState (action, false);
+    ASSERT_EQ (action.active (), false);
+}
 
=== modified file 'src/screen.cpp'
--- src/screen.cpp	2012-09-18 01:44:23 +0000
+++ src/screen.cpp	2012-09-24 11:41:41 +0000
@@ -89,6 +89,7 @@
 } MwmHints;
 
 namespace cps = compiz::private_screen;
+namespace ca = compiz::actions;
 
 
 
@@ -3565,7 +3566,7 @@
 		privateScreen.enableEdge (i);
     }
 
-    action->priv->active = true;
+    ca::setActionActiveState (*action, true);
 
     return true;
 }
@@ -3594,7 +3595,7 @@
 		privateScreen.disableEdge (i);
     }
 
-    action->priv->active = false;
+    ca::setActionActiveState (*action, false);
 }
 
 CompRect