Plugins can't tell the difference between a modifier key-tap, and a modifier key-release (after being used to modify other keys)

Bug #925293 reported by Daniel van Vugt
34
This bug affects 7 people
Affects Status Importance Assigned to Milestone
Compiz Core
Fix Released
High
Daniel van Vugt
Unity Distro Priority
Fix Released
High
Unassigned
compiz (Ubuntu)
Fix Released
Undecided
Daniel van Vugt

Bug Description

If you tap a modifier key, or if you use it to modify another key, any plugin listening for that modifier will have its key-Terminate callback called. But that callback has no way of knowing whether the modifier was tapped (no other keys pressed) or if it was simply held as a regular modifier of another key.

We need a simple way for the Terminate callback (or any other function like a timer started in Initiate) to tell if any other key events have happened between our press and release events (initiate/terminate callbacks). Thus a plugin would be able to respond to a single modifier being tapped or held, without getting confused with the modifier being used with other keys.

Related branches

Changed in compiz-core:
status: New → In Progress
assignee: nobody → Daniel van Vugt (vanvugt)
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Presently, the difference between responding on tap, vs press, release or hold is entirely coded into the plugins. For example, unityshell has some timing code that tries to tell the difference between the different kinds of key events for the dash. But I think a generic solution in compiz-core would be preferable.

Omer Akram (om26er)
Changed in compiz-core:
importance: Undecided → Medium
summary: - Using <Alt> as a plugin hotkey prevents apps from receiving
- <Alt>+<some_other_key>
+ Plugins can't tell the difference between a modifier key-tap, and a
+ modifier key-release (after being used to modify other keys)
description: updated
Revision history for this message
Sam Spilsbury (smspillaz) wrote :

Ideally I think you'd only care if <alt>someotherkey happened if a passive grab was activated on that other key.

So for example in X we have XGrabKey which allows you to specify a key and modifiers (or just a keycode which happens to be a modifier) which makes a keyboard grab activate as soon as that key is pressed. This means that you get the KeyPress and KeyRelease, but *only* for that key, unless you do a full keyboard grab (but note that there's a race condition here between when the key itself is grabbed and when the plugin explicitly grabs the full keyboard. At least in compiz, the rule now is that if you didn't request for the keyboard to be grabbed, it will be ungrabbed as soon as your action callback is finished.

I think that, the best way to handle this case is to keep a watch on which keybindings are still being held down and whether or not a window lost focus due to a grab (eg FocusOut with NotifyGrab on the xevent->xfocus.mode) - this would most likely mean that some other keybinding in either compiz or another application was activated, and in that case, we should send CompAction::StateCancel to the original action rather than CompAction::StateTerminate.

Revision history for this message
Sam Spilsbury (smspillaz) wrote :

(And to be honest, if another application takes a full grab while we're triggering keybindings, it probably makes sense to send CompAction::StateCancel to it anyways)

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Held up, tracking down bug 925979 which blocked this one.

Revision history for this message
Tim Penhey (thumper) wrote :

I'm bumping to high due to a strong desire to get this into 0.9.7.0 release.

Changed in compiz-core:
importance: Medium → High
tags: added: compiz-ff-precise
Tim Penhey (thumper)
Changed in compiz-core:
status: In Progress → Fix Committed
milestone: none → 0.9.7.0
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

I have to say it scares me that this fix is in 0.9.7.0. There is still potential for a serious regression if someone finds a way to lock up the keyboard, due to the new synchronous grabbing.

Changed in unity-distro-priority:
status: New → Fix Committed
importance: Undecided → High
Changed in compiz-core:
status: Fix Committed → Fix Released
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Linked recently related branches. The latest version of this fix is lp:compiz-core revision 3050.

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

This bug was fixed in the package compiz - 1:0.9.7.0+bzr3035-0ubuntu1

---------------
compiz (1:0.9.7.0+bzr3035-0ubuntu1) precise; urgency=low

  [ Łukasz 'sil2100' Zemczak ]
  * New upstream snapshot:
    - Fix gtk-window-decorator crash upon demaximizing a window (LP: #930071)
    - Fix core keybindings (LP: #930412)
    - Fixes compiz crash with SIGSEGV on shutdown (LP: #931283)
    - Plugins can't tell the difference between a key-tap and modifier
      key-release (LP: #925293)
    - compiz-core r3001 (and 3002) ftbfs (LP: #933226)
    - Semi-maximized windows have no shadow or frame (LP: #924736)
    - Untranslated strings in gtk-window-decorator (LP: #780505)
    - Initialize the _NET_WM_STATE_FOCUSED (LP: #932087)
    - [regression] Customized shortcuts don't work (LP: #931927)
    - Window stacking problem (LP: #936675)
    - Quickly demaximized windows can receive maximized window decorations if
      they were initially maximized (LP: #936778)
    - Maximized windows do not get shadows at all (LP: #936774)
    - [regression] Launcher, top panel and keyboard un-responsive after using
      any Super-x shortcut (LP: #934058)
    - No draggable border if mutter isn't installed (LP: #936781)
    - Fix compiz crash with SIGSEGV in XDefineCursor() (LP: #936487)
    - Fixes memory leak at DecorWindow::updateSwitcher() (LP: #940115)
    - Unresolved symbols in plugins cause compiz to exit (LP: #938478)
    - Fix compiz spending about 51% of its CPU time in CompRegion
      construction/destruction (LP: #940139)
    - Fix Conditional jump or move depends on uninitialised value(s) in
      decor_match_pixmap (LP: #940066)
    - Fix 'show desktop' behaviour (LP: #871801)
    - Tweak algorithm used to cast shadows on maximized windows (LP: #936784)
    - "Svg" and "Png" should be "SVG and "PNG" (LP: #942890)
    - Fix invalid memory usage after free() in DecorWindow (LP: #943116)
    - Fix alt + F10 (LP: #943223)
  * Removed cherry-picked patches
  * debian/patches/fix_944631.patch:
    - Always replay the keyboard if something was grabbed and didn't trigger
      an action and don't trigger actions which aren't added accidentally
      (LP: #943612) (LP: #944631)
  * debian/patches/fix_923683.patch:
    - Backports a patch which prevents the shift race condition

  [ Didier Roche ]
  * debian/patches/fix_alt_pressing.patch:
    - Patch from ddv to fix all the regressions with the alt key fix and other
      (LP: #943851, #945373)
    - Fix Quicklist are not showing if right-clicking a launcher icon in Expo
      mode if triggered by Super + S (LP: #944979)
  * debian/patches/fix_806255.patch:
    - Unity/compiz intercepts keystrokes from grabbed windows (LP: #806255)
  * debian/patches/fix_943194.patch:
    - second part for the alt key fix (LP: #943194)
  * debian/patches/additional_alt_tapping_fix.patch:
    - again another alt tapping related fix for some regressions from the
      previous branch. Taken from "tapping-panacea" upstream branch.
 -- Didier Roche <email address hidden> Mon, 12 Mar 2012 10:22:10 +0100

Changed in compiz (Ubuntu):
status: New → Fix Released
assignee: nobody → Daniel van Vugt (vanvugt)
Changed in unity-distro-priority:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.