After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 623223 - Handle video out keys in media-keys
Handle video out keys in media-keys
Status: RESOLVED FIXED
Product: gnome-settings-daemon
Classification: Core
Component: media-keys
unspecified
Other All
: Normal normal
: ---
Assigned To: gnome-settings-daemon-maint
gnome-settings-daemon-maint
: 613155 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2010-06-30 16:57 UTC by Bastien Nocera
Modified: 2014-07-15 17:45 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Handle video out keys in media-keys (5.28 KB, patch)
2010-06-30 16:57 UTC, Bastien Nocera
none Details | Review
Handle video out keys in media-keys (14.24 KB, patch)
2010-07-16 15:07 UTC, Bastien Nocera
none Details | Review
Handle video out keys in media-keys (16.53 KB, patch)
2010-07-19 15:30 UTC, Bastien Nocera
needs-work Details | Review
Use virtual modifier <Super> for the Windows key (2.78 KB, patch)
2010-07-21 10:07 UTC, Bastien Nocera
needs-work Details | Review
Use virtual modifier <Super> for the Windows key (3.36 KB, patch)
2010-07-23 15:57 UTC, Ray Strode [halfline]
none Details | Review
Handle video out keys in media-keys (18.63 KB, patch)
2010-07-23 21:45 UTC, Ray Strode [halfline]
needs-work Details | Review
Use virtual modifier <Super> for the Windows key (4.81 KB, patch)
2010-07-28 14:32 UTC, Ray Strode [halfline]
none Details | Review
Handle video out keys in media-keys (16.74 KB, patch)
2010-11-06 19:04 UTC, Bastien Nocera
committed Details | Review
Use virtual modifier <Super> for the Windows key (4.82 KB, patch)
2010-11-06 19:04 UTC, Bastien Nocera
committed Details | Review
Make VIDEO_OUT media key configurable via dconf. (1.99 KB, patch)
2011-03-31 08:29 UTC, Mikhail Vorozhtsov
none Details | Review

Description Bastien Nocera 2010-06-30 16:57:40 UTC
See patch
Comment 1 Bastien Nocera 2010-06-30 16:57:43 UTC
Created attachment 164978 [details] [review]
Handle video out keys in media-keys

Not finished, but gives you an idea.

1) add dbus method to do the "change output" action
2) remove Fn+F7 code from the xrandr backend
3) add hard-coded windows+p/fn+f7 handler in media-keys
4) make those hard-coded keys call the dbus method
Comment 2 Bastien Nocera 2010-06-30 17:07:57 UTC
The patch add 3), though I didn't check for the actual keys that should be used.
Comment 3 Bastien Nocera 2010-07-16 15:07:34 UTC
Created attachment 166026 [details] [review]
Handle video out keys in media-keys

Instead of reimplementing keyboard events capture in xrandr,
add D-Bus methods to rotate the screen, and switch video modes
to the xrandr plugin, and make the media-keys plugin handle call
those methods when the keys are pressed.

TODO:
4) make those hard-coded keys call the dbus method
Comment 4 Bastien Nocera 2010-07-19 15:30:28 UTC
Created attachment 166165 [details] [review]
Handle video out keys in media-keys

Instead of reimplementing keyboard events capture in xrandr,
add D-Bus methods to rotate the screen, and switch video modes
to the xrandr plugin, and make the media-keys plugin handle call
those methods when the keys are pressed.
Comment 5 Bastien Nocera 2010-07-19 15:32:13 UTC
Mildly tested on a GNOME 2.28 backport, seems to work as expected :)
Comment 6 Ray Strode [halfline] 2010-07-19 19:20:57 UTC
I don't think this is going to generally work:

+	{ VIDEO_OUT_KEY, NULL, "<Mod4>p", NULL },

Should be <Super>p right?

Presumably, you had <Super>p and that didn't work and then <Mod4>p did work.  In that case, maybe g-s-d needs a patch in the same vein as the panel got recently in bug 624572 ?
Comment 7 Bastien Nocera 2010-07-20 14:33:37 UTC
<Mod4>p is what the keyboard shortcuts app detects, I didn't do any additional work in trying to use Super instead.
Comment 8 Ray Strode [halfline] 2010-07-20 16:35:36 UTC
The problem is <Super> might be Mod2 or Mod5 on a different machine.  Virtual modifiers like super, hyper, meta, etc are arbitrarily assigned to the real modifiers (mod2 to mod5).
Comment 9 Bastien Nocera 2010-07-21 10:07:36 UTC
Created attachment 166264 [details] [review]
Use virtual modifier <Super> for the Windows key

By making sure that we convert virtual modifiers to real ones when
capturing keys with XGrabKey, and vice-versa when checking for
XEvents.
Comment 10 Bastien Nocera 2010-07-21 10:09:31 UTC
Right. The egg code to parse the shortcuts works fine (eg. <Super> is parsed properly). The problem is that we need to resolve GDK_SUPER_MASK to its real value depending on the keymap for use with XGrabKey (which does not understand the Super modifier), and also when checking for key equality.

I'd expect some slight problems if somebody changed the ModX value for the Super key a few times during the gnome-settings-daemon run, but nothing worse than what we have now...
Comment 11 Michael Terry 2010-07-21 13:48:17 UTC
There was some discussion about this video key (super+p) in Ubuntu: 
https://bugs.launchpad.net/ubuntu/+source/gnome-settings-daemon/+bug/539477

One issue we found was that an Enter press was sometimes sent to the system after a delay:
https://bugs.launchpad.net/ubuntu/+source/gnome-settings-daemon/+bug/539477/comments/39

Are you seeing that on your hardware?  Any thought of how to handle that?
Comment 12 Bastien Nocera 2010-07-21 13:54:02 UTC
(In reply to comment #11)
> There was some discussion about this video key (super+p) in Ubuntu: 
> https://bugs.launchpad.net/ubuntu/+source/gnome-settings-daemon/+bug/539477
> 
> One issue we found was that an Enter press was sometimes sent to the system
> after a delay:
> https://bugs.launchpad.net/ubuntu/+source/gnome-settings-daemon/+bug/539477/comments/39
> 
> Are you seeing that on your hardware?  Any thought of how to handle that?

No, and get the manufacturer to fix the BIOS.
Comment 13 Michael Terry 2010-07-21 15:30:52 UTC
Ah, I wish I could make that so.  Linux support is not usually top of BIOS makers' lists.

As you can see in the Ubuntu bug, Jerone references an internal Dell spec that specifies the Enter behavior.  Since we're not seeing it on non-Dell machines, it presumably is a Dell-specific spec.

I understand your lack of enthusiasm for working around one manufacturer's behavior.  But even if Dell changes it's apparently-desired Windows behavior [1] to accommodate Linux for future machines, there are shipped devices with the Enter issue.

The best workaround right now for Dell machines is to tell the BIOS we are not Windows 7 (acpi_osi=\"!Windows 2009\").

Perhaps this should be moved to a different bug about the Enter press on Dell hardware, as it is not really relevant to broader support for the video out key press.

[1] The theory is they do that to automatically dismiss the Windows dialog that pops up when you press the Video key if the user just wants to quickly switch displays.
Comment 14 Bastien Nocera 2010-07-21 15:33:59 UTC
(In reply to comment #13)
> Ah, I wish I could make that so.  Linux support is not usually top of BIOS
> makers' lists.
> 
> As you can see in the Ubuntu bug, Jerone references an internal Dell spec that
> specifies the Enter behavior.  Since we're not seeing it on non-Dell machines,
> it presumably is a Dell-specific spec.
> 
> I understand your lack of enthusiasm for working around one manufacturer's
> behavior.  But even if Dell changes it's apparently-desired Windows behavior
> [1] to accommodate Linux for future machines, there are shipped devices with
> the Enter issue.
> 
> The best workaround right now for Dell machines is to tell the BIOS we are not
> Windows 7 (acpi_osi=\"!Windows 2009\").
> 
> Perhaps this should be moved to a different bug about the Enter press on Dell
> hardware, as it is not really relevant to broader support for the video out key
> press.
> 
> [1] The theory is they do that to automatically dismiss the Windows dialog that
> pops up when you press the Video key if the user just wants to quickly switch
> displays.

Probably, but pressing Enter in the BIOS is just a completely stupid way to do this.
Comment 15 Ray Strode [halfline] 2010-07-23 15:50:16 UTC
Review of attachment 166264 [details] [review]:

::: plugins/common/gsd-keygrab.c
@@ +129,3 @@
+        /* XGrabKey requires real modifiers, not virtual ones */
+        egg_keymap_resolve_virtual_modifiers (gdk_keymap_get_default (),
+					      cleaned_mask, &mask);

This part isn't right.  We need to resolve modifiers in the key->state not the ignored modifiers.  Basically, we shouldn't ever be passing key->state to grab_key_real, but instead a bitlist of real modifiers
Comment 16 Ray Strode [halfline] 2010-07-23 15:57:36 UTC
Created attachment 166454 [details] [review]
Use virtual modifier <Super> for the Windows key

By making sure that we convert virtual modifiers to real ones when
capturing keys with XGrabKey, and vice-versa when checking for
XEvents.

This is based heavily on a patch from

Bastien Nocera <hadess@hadess.net>

with just minor fixes.
Comment 17 Ray Strode [halfline] 2010-07-23 21:38:24 UTC
Review of attachment 166165 [details] [review]:

::: plugins/media-keys/gsd-media-keys-manager.c
@@ +943,3 @@
+				 G_TYPE_INT64, timestamp,
+				 G_TYPE_INVALID,
+				 G_TYPE_INVALID);

You can't do a blocking call to yourself.  It will block until timeout adding a 25 second artificial delay.
Comment 18 Ray Strode [halfline] 2010-07-23 21:45:34 UTC
Created attachment 166469 [details] [review]
Handle video out keys in media-keys

Instead of reimplementing keyboard events capture in xrandr,
add D-Bus methods to rotate the screen, and switch video modes
to the xrandr plugin, and make the media-keys plugin handle call
those methods when the keys are pressed.

This patch almost entirely written by:

Bastien Nocera <hadess@hadess.net>

with minor fixes.
Comment 19 Ray Strode [halfline] 2010-07-28 14:27:31 UTC
So on a downstream report, one user has run into a problem with

 attachment 166454 [details] [review]

They have an xmodmap file that removes Super from their map entirely.  This means this call:

+        /* XGrabKey requires real modifiers, not virtual ones */
+        egg_keymap_resolve_virtual_modifiers (gdk_keymap_get_default (),
+					      key->state, &modifiers);
+

is unable to resolve Super to a real modifier, and so XGrabKey gets called with no modifier at all causing a broken p key.
Comment 20 Ray Strode [halfline] 2010-07-28 14:32:57 UTC
Created attachment 166715 [details] [review]
Use virtual modifier <Super> for the Windows key

By making sure that we convert virtual modifiers to real ones when
capturing keys with XGrabKey, and vice-versa when checking for
XEvents.

This is based heavily on a patch from

Bastien Nocera <hadess@hadess.net>

with just minor fixes.
Comment 21 Ray Strode [halfline] 2010-10-05 16:08:33 UTC
*** Bug 613155 has been marked as a duplicate of this bug. ***
Comment 22 Federico Mena Quintero 2010-10-05 21:48:20 UTC
Very nice!  I'm happy to let something else handle the arcana of grabbing keys.  Feel free to commit this.
Comment 23 Ray Strode [halfline] 2010-10-06 14:09:33 UTC
I was talking to Bastien about this yesterday, and we decided we should probably update the patches to gdbus before committing them.

I think they're a little bitrotted anyway now, so will need some work regardless.
Comment 24 Bastien Nocera 2010-11-01 19:11:39 UTC
I added a few parts of the first patch to master, as it was needed to handle bug 633726.
Comment 25 Bastien Nocera 2010-11-06 19:04:19 UTC
Created attachment 173963 [details] [review]
Handle video out keys in media-keys

Instead of reimplementing keyboard events capture in xrandr,
add D-Bus methods to rotate the screen, and switch video modes
to the xrandr plugin, and make the media-keys plugin handle call
those methods when the keys are pressed.

This patch almost entirely written by:

Bastien Nocera <hadess@hadess.net>

with minor fixes.
Comment 26 Bastien Nocera 2010-11-06 19:04:26 UTC
Created attachment 173964 [details] [review]
Use virtual modifier <Super> for the Windows key

By making sure that we convert virtual modifiers to real ones when
capturing keys with XGrabKey, and vice-versa when checking for
XEvents.

This is based heavily on a patch from

Bastien Nocera <hadess@hadess.net>

with just minor fixes.
Comment 27 Bastien Nocera 2010-11-06 19:06:49 UTC
Ported all those to the latest tree, and GDbus. Needs testing.

I left the old XRandR code still using dbus-glib, as the additions were fairly minimal. Let's do that separately.
Comment 28 Bastien Nocera 2010-11-08 21:00:53 UTC
Attachment 173963 [details] pushed as d1a4f8c - Handle video out keys in media-keys
Attachment 173964 [details] pushed as 469145f - Use virtual modifier <Super> for the Windows key
Comment 29 Mikhail Vorozhtsov 2011-03-31 08:27:40 UTC
Sorry, but using hardcoded keys is just a sloppy work. I upgrade gsd and suddenly "Previous Window" Compiz action makes my screen go blank for a moment and there is no way to find out why. No GUI, no g/dconf knobs. I'll attach the patch I use to make VIDEO_OUT configurable via dconf-editor (against gsd-2.91.93). See also https://bugs.launchpad.net/ubuntu/+source/gnome-settings-daemon/+bug/694910.
Comment 30 Mikhail Vorozhtsov 2011-03-31 08:29:03 UTC
Created attachment 184767 [details] [review]
Make VIDEO_OUT media key configurable via dconf.
Comment 31 Eduardo Habkost 2011-06-10 12:00:52 UTC
(In reply to comment #29)
> I'll attach the
> patch I use to make VIDEO_OUT configurable via dconf-editor (against
> gsd-2.91.93). See also
> https://bugs.launchpad.net/ubuntu/+source/gnome-settings-daemon/+bug/694910.

FYI, I have requested the patch to be inclued by opening bug 651571, but it was rejected.
Comment 32 Federico Mena Quintero 2011-06-13 23:00:35 UTC
FWIW, I used to keep a patch around for debugging purposes, that did exactly that - make the XF86Display hotkey configurable.  It was useful for testing the RANDR plugin on my desktop machine, which of course doesn't have the laptop's special keys to switch video modes.
Comment 33 Bastien Nocera 2011-06-20 14:36:36 UTC
(In reply to comment #32)
> FWIW, I used to keep a patch around for debugging purposes, that did exactly
> that - make the XF86Display hotkey configurable.  It was useful for testing the
> RANDR plugin on my desktop machine, which of course doesn't have the laptop's
> special keys to switch video modes.

But most likely has a Windows, so you could type "Windows+P" and have it do the same thing.
Comment 34 Federico Mena Quintero 2011-06-20 16:50:44 UTC
(In reply to comment #33)
> But most likely has a Windows, so you could type "Windows+P" and have it do the
> same thing.

Heh, good point :)
Comment 35 ans+gnome 2011-12-21 12:21:48 UTC
Could you PLEASE apply the patch from comment 30! The current situation is unacceptable, your squatting the shortcut i use since ever to open programs. It's hardcoded in my brain!

Also many WMs use <mod>-p per default for this task, eg. wmii, ion, i3, dwm, ... It is a common usecase to run a different WM on top of a gnome-session, so please make this configurable! Having to disable the xrandr-plugin is quite a strange workaround.
Comment 36 Łukasz Michalak 2014-07-15 16:58:38 UTC
Current solution (hardcoding <Super>+p in addition to XF86Display) does not work in case of using some different keyboard layouts - e.g. dvorak. In case of using dvorak the scancodes sent with video out key press convert to <Super>+l and this is not handled by gnome-settings-daemon.
I think that either the key combination recognized by settings-daemon should depend on the keyboard layout set or the key combination should be user adjustable.