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

Proposed by Daniel van Vugt
Status: Merged
Approved by: Alan Griffiths
Approved revision: 3046
Merged at revision: 3046
Proposed branch: lp:~vanvugt/compiz-core/fix-alt-bugs
Merge into: lp:compiz-core
Diff against target: 160 lines (+36/-15)
4 files modified
metadata/core.xml.in (+7/-0)
src/event.cpp (+25/-6)
src/privatescreen.h (+1/-2)
src/screen.cpp (+3/-7)
To merge this branch: bzr merge lp:~vanvugt/compiz-core/fix-alt-bugs
Reviewer Review Type Date Requested Status
Alan Griffiths Approve
Review via email: mp+96314@code.launchpad.net

Description of the change

Don't keep an active keyboard grab the whole time a shortcut key is held
down. Doing so was causing multiple bugs with Unity 5, which binds to
the Alt key. Instead use some simple heurisitics to decide if a key has
been "tapped" or not. (LP: #943194) (LP: #943851) (LP: #945373)

Yes, it is a compromise to go back to detecting taps based on timing.
However it is the only existing solution to all of these bugs related to
the Alt key.

To post a comment you must log in.
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

Looks sane, seems to work (not entirely sure about reproducing 943194, but it doesn't seems worse.)

review: Approve
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'metadata/core.xml.in'
--- metadata/core.xml.in 2010-06-12 07:43:36 +0000
+++ metadata/core.xml.in 2012-03-07 10:06:17 +0000
@@ -31,6 +31,13 @@
31 <min>0</min>31 <min>0</min>
32 <max>10000</max>32 <max>10000</max>
33 </option>33 </option>
34 <option name="tap_time" type="int">
35 <_short>Key Tap Time</_short>
36 <_long>Maximim amount of time (milliseconds) a key may be held down and still be reported as a "tap".</_long>
37 <default>150</default>
38 <min>0</min>
39 <max>10000</max>
40 </option>
34 <option name="ping_delay" type="int">41 <option name="ping_delay" type="int">
35 <_short>Ping Delay</_short>42 <_short>Ping Delay</_short>
36 <_long>Interval between ping messages</_long>43 <_long>Interval between ping messages</_long>
3744
=== modified file 'src/event.cpp'
--- src/event.cpp 2012-03-06 11:05:05 +0000
+++ src/event.cpp 2012-03-07 10:06:17 +0000
@@ -143,13 +143,20 @@
143143
144 if (state == CompAction::StateInitKey && grabs.empty ())144 if (state == CompAction::StateInitKey && grabs.empty ())
145 {145 {
146 possibleTap = action;146 int pressTime = arguments[7].value ().i ();
147 int err = XGrabKeyboard (dpy, grabWindow, True,147 int err = XGrabKeyboard (dpy, grabWindow, True,
148 GrabModeAsync, GrabModeSync, arguments[7].value ().i ());148 GrabModeAsync, GrabModeAsync, pressTime);
149 /*
150 * We don't actually need this active grab for anything other than
151 * to test if it fails (AlreadyGrabbed). So we know if some other
152 * app has an active grab like a virtual machine or lock screen.
153 * (LP: #806255)
154 */
149 if (err == GrabSuccess)155 if (err == GrabSuccess)
150 {156 {
151 XAllowEvents (dpy, SyncKeyboard, CurrentTime);157 XUngrabKeyboard (dpy, pressTime);
152 tapGrab = true;158 possibleTap = action;
159 tapStart = pressTime;
153 }160 }
154 else161 else
155 {162 {
@@ -183,7 +190,10 @@
183{190{
184 if (action == possibleTap)191 if (action == possibleTap)
185 {192 {
186 state |= CompAction::StateTermTapped;193 int releaseTime = arguments[7].value ().i ();
194 int tapDuration = releaseTime - tapStart;
195 if (tapDuration < optionGetTapTime ())
196 state |= CompAction::StateTermTapped;
187 possibleTap = NULL;197 possibleTap = NULL;
188 }198 }
189199
@@ -964,6 +974,8 @@
964 o[3].value ().set ((int) xkbEvent->time);974 o[3].value ().set ((int) xkbEvent->time);
965 o[4].reset ();975 o[4].reset ();
966 o[5].reset ();976 o[5].reset ();
977 o[6].reset ();
978 o[7].value ().set ((int) xkbEvent->time);
967979
968 if (stateEvent->event_type == KeyPress)980 if (stateEvent->event_type == KeyPress)
969 possibleTap = NULL;981 possibleTap = NULL;
@@ -1076,7 +1088,6 @@
1076 if (priv->grabs.empty () && event->type == KeyPress)1088 if (priv->grabs.empty () && event->type == KeyPress)
1077 {1089 {
1078 XUngrabKeyboard (priv->dpy, event->xkey.time);1090 XUngrabKeyboard (priv->dpy, event->xkey.time);
1079 priv->tapGrab = false;
1080 }1091 }
1081}1092}
10821093
@@ -1905,6 +1916,14 @@
1905 break;1916 break;
1906 case FocusIn:1917 case FocusIn:
1907 {1918 {
1919 /* When a menu etc gets a grab, it's safe to say we're not tapping
1920 any key right now. e.g. Detecting taps of "Alt" and cancelling
1921 when a menu is opened */
1922 if (event->xfocus.mode == NotifyGrab &&
1923 event->xfocus.window != priv->root &&
1924 event->xfocus.window != priv->grabWindow)
1925 priv->possibleTap = NULL;
1926
1908 if (!XGetWindowAttributes (priv->dpy, event->xfocus.window, &wa))1927 if (!XGetWindowAttributes (priv->dpy, event->xfocus.window, &wa))
1909 priv->setDefaultWindowAttributes (&wa);1928 priv->setDefaultWindowAttributes (&wa);
19101929
19111930
=== modified file 'src/privatescreen.h'
--- src/privatescreen.h 2012-03-05 09:04:16 +0000
+++ src/privatescreen.h 2012-03-07 10:06:17 +0000
@@ -559,6 +559,7 @@
559 CompOption::Value plugin;559 CompOption::Value plugin;
560 bool dirtyPluginList;560 bool dirtyPluginList;
561 void *possibleTap;561 void *possibleTap;
562 Time tapStart;
562};563};
563564
564class EventManager :565class EventManager :
@@ -607,8 +608,6 @@
607608
608 CompIcon *defaultIcon;609 CompIcon *defaultIcon;
609610
610 bool tapGrab;
611
612 private:611 private:
613 virtual bool initDisplay (const char *name);612 virtual bool initDisplay (const char *name);
614};613};
615614
=== modified file 'src/screen.cpp'
--- src/screen.cpp 2012-03-05 09:04:16 +0000
+++ src/screen.cpp 2012-03-07 10:06:17 +0000
@@ -2950,8 +2950,6 @@
2950 XUngrabPointer (priv->dpy, CurrentTime);2950 XUngrabPointer (priv->dpy, CurrentTime);
2951 return NULL;2951 return NULL;
2952 }2952 }
2953 else
2954 priv->tapGrab = false;
2955 }2953 }
2956 else2954 else
2957 return NULL;2955 return NULL;
@@ -3014,7 +3012,6 @@
30143012
3015 XUngrabPointer (priv->dpy, CurrentTime);3013 XUngrabPointer (priv->dpy, CurrentTime);
3016 XUngrabKeyboard (priv->dpy, CurrentTime);3014 XUngrabKeyboard (priv->dpy, CurrentTime);
3017 priv->tapGrab = false;
3018 }3015 }
3019}3016}
30203017
@@ -3591,7 +3588,6 @@
35913588
3592 XUngrabPointer (priv->dpy, CurrentTime);3589 XUngrabPointer (priv->dpy, CurrentTime);
3593 XUngrabKeyboard (priv->dpy, CurrentTime);3590 XUngrabKeyboard (priv->dpy, CurrentTime);
3594 priv->tapGrab = false;
35953591
3596 XSendEvent (priv->dpy, priv->root, false,3592 XSendEvent (priv->dpy, priv->root, false,
3597 StructureNotifyMask, &ev);3593 StructureNotifyMask, &ev);
@@ -5033,7 +5029,8 @@
5033 CoreOptions (false),5029 CoreOptions (false),
5034 plugin (),5030 plugin (),
5035 dirtyPluginList (true),5031 dirtyPluginList (true),
5036 possibleTap (NULL)5032 possibleTap (NULL),
5033 tapStart (0)
5037{5034{
5038}5035}
50395036
@@ -5049,8 +5046,7 @@
5049 edgeDelayTimer (),5046 edgeDelayTimer (),
5050 desktopWindowCount (0),5047 desktopWindowCount (0),
5051 mapNum (1),5048 mapNum (1),
5052 defaultIcon (0),5049 defaultIcon (0)
5053 tapGrab (false)
5054{5050{
5055 ValueHolder::SetDefault (static_cast<ValueHolder *> (this));5051 ValueHolder::SetDefault (static_cast<ValueHolder *> (this));
5056 TimeoutHandler *dTimeoutHandler = new TimeoutHandler ();5052 TimeoutHandler *dTimeoutHandler = new TimeoutHandler ();

Subscribers

People subscribed via source and target branches