This is extra epically annoying.  Super as window modifier has always made more sense to me as I can dedicate it to window management (super-c to close, super-t for a terminal, super-arrows to switch workspaces, and now in GNOME 3 just super for the overlay).

Funny thing is, it starts working again after switching windows using Alt-Tab.

Created attachment 203309 [details] [review]
display: Handle overlay key overlapping with grab modifiers

If the overlay key is also used as mouse button modifier, its
handling breaks after a grab operation. Detect that case and
reset overlay_key_only_pressed to fix.

*** Bug 669304 has been marked as a duplicate of this bug. ***

Note that while I agree that those Bugs are similar, I would be interested how to change the 'Grab Modifiers' (or whatever is consuming Alt-Drag at the moment) at all. I already found out how to remap the overlay_key but no luck so far with the Drag/Grab stuff

gsettings set org.gnome.desktop.wm.preferences mouse-button-modifier '"<Super>"' as mentioned in the other bug (except that it doesn't work correctly due to this bug)

thanks a bunch, works for me as the overlay key is already mapped to something else ... 
all available information pointed to the old metacity settings in gconf-editor, which obviously isn't used anymore.

Review of attachment 203309 [details] [review]:

With the overlay key _not_ overlapping try pressing the super key then:

 - clicking in an application, then release super. Overlay activates
 - clicking on a gtk+ menu item, then release super. Overlay activates, menu goes into a bit of a weird state because mutter had a keyboard grab but no pointer grab
 - moving a window, then release super. Overlay key is in a broken state and doesn't open the overview, until you open the overview manaually once. (not sure why)

Should we switch the overlay key to grab mouse and pointer synchronously, and if we get a buttonp-press/release of any type, we unset overlay_key_only XAllowEvents() to reply the event and ungrab , as we do for the key case? (Since we don't have any root window grabs for the pointer, we can just let the event come back to us and hit window grabs if appropriate.)

If we did need to go something more like this patch, I'd definitely like the complex operation of determining overlay_key_modifiers not to be inlined in some random place in the code that happens to need it.

Created attachment 210982 [details] [review]
display: Cancel overlay key presses on mouse button events

Currently pressing the overlay key only triggers the overview if
no other key is pressed between KeyPress and KeyRelease. Extend
this logic to pointer events, so that KeyPress + ButtonPress actions
are treated explicitly different from "pure" overlay key presses.
In particular, this change allows to re-use the overlay key as mouse
button modifier.


(In reply to comment #8)
> Should we switch the overlay key to grab mouse and pointer synchronously, and
> if we get a buttonp-press/release of any type, we unset overlay_key_only
> XAllowEvents() to reply the event and ungrab , as we do for the key case?

You mean something like


meta_grab_key():
  XGrabKey (display->xdisplay, ...,
            GrabModeSync, GrabModeSync);

event_callback():
  case ButtonPress:
  case ButtonRelease:
    display->overlay_key_only_pressed = FALSE;
    XAllowEvents (display->xdisplay, ReplayPointer, event->xbutton.time);


But doesn't a pointer_mode of GrabModeSync mean that we won't receive any pointer events until the key grab is released? I'm actually wondering whether we need a grab at all - the attached patch appears to work fine in a quick test (and doesn't mess up the mouse-button-modifier != overlay-key case) ...

(In reply to comment #9)
> Created an attachment (id=210982) [details] [review]
> display: Cancel overlay key presses on mouse button events
> 
> Currently pressing the overlay key only triggers the overview if
> no other key is pressed between KeyPress and KeyRelease. Extend
> this logic to pointer events, so that KeyPress + ButtonPress actions
> are treated explicitly different from "pure" overlay key presses.
> In particular, this change allows to re-use the overlay key as mouse
> button modifier.

Tested this patch out and it seems to uniformly make things better. It doesn't fix things like the case where the modifier is <Alt> and you do:

 press <Super> 
 Click on menu in GTK+ 3 application (GTK+ 2 is OK)
 Release <Super>

But it doesn't make that worse either. I'm going to land this patch for 3.4.1 and we can consider whether we want to try something fancier for 3.6.
 
> (In reply to comment #8)
> > Should we switch the overlay key to grab mouse and pointer synchronously, and
> > if we get a buttonp-press/release of any type, we unset overlay_key_only
> > XAllowEvents() to reply the event and ungrab , as we do for the key case?

> You mean something like
> 
> meta_grab_key():
>   XGrabKey (display->xdisplay, ...,
>             GrabModeSync, GrabModeSync);
> 
> event_callback():
>   case ButtonPress:
>   case ButtonRelease:
>     display->overlay_key_only_pressed = FALSE;
>     XAllowEvents (display->xdisplay, ReplayPointer, event->xbutton.time);
>  
> But doesn't a pointer_mode of GrabModeSync mean that we won't receive any
> pointer events until the key grab is released?

You can get more events without releasing the grab using XAllowEvents- see the Xlib documentation. Don't remember offhand if you have to do one call for each motion, or you can allow events up to the next press.

> I'm actually wondering whether
> we need a grab at all - the attached patch appears to work fine in a quick test
> (and doesn't mess up the mouse-button-modifier != overlay-key case) ...

I can't think of any obvious problem with it - setting it to false won't change the behavior except that we won't trigger the overview in cases we know are problematical.

- Owen

Comment on attachment 210982 [details] [review]
display: Cancel overlay key presses on mouse button events

Attachment 210982 [details] pushed as 8809673 - display: Cancel overlay key presses on mouse button events

Pushing patch, but leaving bug open in case we want something "fancier for 3.6".

We've had it as the default for quite a while, so I don't think we need anything "fancier for 3.6" anymore :)