Merge lp:~vanvugt/compiz-core/fix-953089.2 into lp:compiz-core

[Daniel van Vugt]

Don't rely on the value of "grabbed" to tell you if the root window is
presently accepting key events (meaning there are no external active grabs).
There are some edge cases where grabbed is false but root is still accepting
key events, so when that happened compiz would ignore its own key shortcuts
(LP: #953089)

This was a regression caused by the fix for LP: #806255. The fix for that
bug has now been redesigned to avoid causing LP: #953089.

[Sam Spilsbury (smspillaz)]

review: Approve

=== modified file 'src/event.cpp'
--- src/event.cpp	2012-03-21 03:54:56 +0000
+++ src/event.cpp	2012-03-22 12:48:23 +0000
@@ -143,24 +143,7 @@
 
     if (state == CompAction::StateInitKey && grabsEmpty ())
     {
-        if (grabbed)
-        {
-            possibleTap = action;
-        }
-        else
-        {
-            /*
-             * If we received this keypress event and weren't grabbed then
-             * the event doesn't belong to us. More likely belongs to
-             * a different client's grab so ignore it. (LP: #806255)
-             * The reason why we might receive such keypress events while
-             * other clients have grabs is because of the XKB extension that
-             * we're using. It sends you events even if they're not yours...
-             * http://www.x.org/releases/current/doc/libX11/specs/XKB/xkblib.html
-             */
-            possibleTap = NULL;
-            return false;
-        }
+        possibleTap = action;
     }
 
     if (!action->initiate ().empty ())
@@ -680,6 +663,8 @@
 
     switch (event->type) {
     case ButtonPress:
+	lastKeyCode = 0;
+
 	/* We need to determine if we clicked on a parent frame
 	 * window, if so, pass the appropriate child window as
 	 * "window" and the frame as "event_window"
@@ -715,6 +700,8 @@
 	}
 	break;
     case ButtonRelease:
+	lastKeyCode = 0;
+
 	o[0].value ().set ((int) event->xbutton.window);
 	o[1].value ().set ((int) event->xbutton.window);
 	o[2].value ().set ((int) event->xbutton.state);
@@ -736,6 +723,8 @@
 	}
 	break;
     case KeyPress:
+	lastKeyCode = event->xkey.keycode;
+
 	o[0].value ().set ((int) event->xkey.window);
 	o[1].value ().set ((int) activeWindow);
 	o[2].value ().set ((int) event->xkey.state);
@@ -758,6 +747,8 @@
 	}
 	break;
     case KeyRelease:
+	lastKeyCode = event->xkey.keycode;
+
 	o[0].value ().set ((int) event->xkey.window);
 	o[1].value ().set ((int) activeWindow);
 	o[2].value ().set ((int) event->xkey.state);
@@ -959,6 +950,28 @@
 	    {
 		XkbStateNotifyEvent *stateEvent = (XkbStateNotifyEvent *) event;
 
+		/*
+		 * You will reach this point when modifier keys or mouse
+		 * buttons change state (are pressed or released). However the
+		 * XKB extension does not honour active grabs held by other
+		 * clients and will send us these XkbStateNotify events even
+		 * when we shouldn't receive them at all (LP: #806255).
+		 * [http://www.x.org/releases/current/doc/libX11/specs/XKB/xkblib.html]
+		 *
+		 * So we need a sanity check. The sanity check is to only
+		 * accept XkbStateNotify's which were preceded by a matching
+		 * KeyPress or KeyRelease event, or which occurred during a
+		 * compiz active grab. Then we can be sure the keyboard is not
+		 * actively grabbed by another client and that compiz can
+		 * safely respond to this event.
+		 *
+		 * The only cleaner fix for LP: #806255 I can think of would
+		 * be to remove XkbStateNotify support completely from compiz.
+		 * But that would be a huge change, in core and some plugins.
+		 */
+		if (grabsEmpty() && stateEvent->keycode != lastKeyCode)
+		    break;
+
 		o[0].value ().set ((int) activeWindow);
 		o[1].value ().set ((int) activeWindow);
 		o[2].value ().set ((int) stateEvent->mods);

=== modified file 'src/privatescreen.h'
--- src/privatescreen.h	2012-03-12 14:30:28 +0000
+++ src/privatescreen.h	2012-03-22 12:48:23 +0000
@@ -989,6 +989,8 @@
 	CompTimer               edgeDelayTimer;
 	CompDelayedEdgeSettings edgeDelaySettings;
 	Window	xdndWindow;
+
+	int lastKeyCode;
 };
 
 class CompManager

=== modified file 'src/screen.cpp'
--- src/screen.cpp	2012-03-15 06:01:46 +0000
+++ src/screen.cpp	2012-03-22 12:48:23 +0000
@@ -5124,7 +5124,8 @@
     initialized (false),
     edgeWindow (None),
     edgeDelayTimer (),
-    xdndWindow (None)
+    xdndWindow (None),
+    lastKeyCode (0)
 {
     pingTimer.setCallback (
 	boost::bind (&PrivateScreen::handlePingTimeout, this));