Merge lp:~vanvugt/compiz-core/fix-alt-bugs-2 into lp:compiz-core

[Daniel van Vugt]

Improved the fix for LP: #943194 so that it now works with GTK-2 menus too.
The first attempt only worked with GTK-3 it seems.

This also required that the fix for LP: #806255 be redesigned to avoid
XGrabKeyboard completely. Because that was part of what broke GTK-2 menus.

[Didier Roche-Tolomelli (didrocks)]

Hey Daniel, thanks for working on that! Are you sure that the fix for LP: #806255 was what made gtk2 menus regressed? (we don't have this commit in the proposed snapshot and it was broken beforehand).

Anyway, I'm trying backporting both commits then :)

[Daniel van Vugt (vanvugt)]

Yes, I definitely had to remove the XGrabKeyboard (fix for LP: #806255). I also had to change the XGrabKey mode to GrabModeSync to ensure the Alt keypress event was replayed correctly to the underlying app by XAllowEvents in alwaysHandleEvent.

I hope ubuntu downstream gets a new snapshot when 0.9.7.2 is done. Or even before then...

[Didier Roche-Tolomelli (didrocks)]

We are almost shipping trunk right now (rev 3039 minus rev 3036)
There is the lim merge we need to discuss as it will require a FFe that we denied (or reverted, or maybe conditionnally built, or having garantees the code isn't executed if the unity side isn't there), but that's a side talk. We tried to qualified rev 3035 at some point (and it's already frozen for 2 weeks) but we saw this alt regression which didn't let us ship it…

[Alan Griffiths (alan-griffiths)]

review: Approve

[Didier Roche-Tolomelli (didrocks)]

Tested and everything works fine (xul/qt apps)! Really nice work Daniel :)

=== modified file 'src/event.cpp'
--- src/event.cpp	2012-03-07 09:31:30 +0000
+++ src/event.cpp	2012-03-08 08:17:17 +0000
@@ -143,30 +143,24 @@
 
     if (state == CompAction::StateInitKey && grabs.empty ())
     {
-        int pressTime = arguments[7].value ().i ();
-        int err = XGrabKeyboard (dpy, grabWindow, True,
-                                 GrabModeAsync, GrabModeAsync, pressTime);
-        /*
-         * We don't actually need this active grab for anything other than
-         * to test if it fails (AlreadyGrabbed). So we know if some other
-         * app has an active grab like a virtual machine or lock screen.
-         * (LP: #806255)
-         */
-        if (err == GrabSuccess)
+        if (grabbed)
         {
-            XUngrabKeyboard (dpy, pressTime);
             possibleTap = action;
-            tapStart = pressTime;
+            tapStart = arguments[7].value ().i ();
         }
         else
         {
-            possibleTap = NULL;
             /*
-             * Don't trigger any compiz actions if some other app has the
-             * keyboard grabbed (like a VM or locked screensaver). LP: #806255
+             * 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
              */
-            if (err == AlreadyGrabbed)
-                return false;
+            possibleTap = NULL;
+            return false;
         }
     }
 

=== modified file 'src/screen.cpp'
--- src/screen.cpp	2012-03-07 09:31:30 +0000
+++ src/screen.cpp	2012-03-08 08:17:17 +0000
@@ -3073,13 +3073,17 @@
 {
     if (grab)
     {
+	/*
+	 * Always grab the keyboard Sync-ronously. This is so that we can
+	 * choose to ReplayKeyboard in alwaysHandleEvent if need be.
+	 */
 	XGrabKey (dpy,
 		  keycode,
 		  modifiers,
 		  root,
 		  true,
 		  GrabModeAsync,
-		  GrabModeAsync);
+		  GrabModeSync);
     }
     else
     {