GNOME Bugzilla – Bug 594831
Fn-F8 should disable/enable touch points
Last modified: 2010-02-15 14:10:11 UTC
On my thinkpad, I have an Fn-F8 key, which should cycle between the touchpoint/touchpad being on and off. We have the gconf keys for that; we should also listen for that keypress to toggle them.
libX11-1.3 now provides XF86TouchpadToggle and xkeyboard-config 1.7 by default on FK22 (keycode 200 with evdev). This means now we just need g-s-d to listen to that keysym
Created attachment 146315 [details] [review] 0001-Add-touchpad_enabled-gconf-key.patch Add touchpad_enabled gconf key. Boolean key that toggles the "Device Enabled" property on all touchpad devices. If TRUE, touchpads are enabled, otherwise, all touchpads are disabled. Touchpad disabled with this key will be visible to other X Input clients but cannot send events unless re-enabled.
Created attachment 146316 [details] [review] 0002-Add-keybinding-schema-to-enable-disable-touchpad.patch Add keybinding schema to enable/disable touchpad Not sure about this patch tbh. The integration seems a bit off, it's not quite as well contained to the touchpad parts as the other patch. There's string duplication for the touchpad key and the key doesn't show up in the keyboard shortcuts dialog. If you can help me what's missing here, that'd be much appreciated. Thanks.
Review of attachment 146316 [details] [review]: If you want the shortcut to appear in the keybindings preferences you need to patch the control-center as well. There's a config file containing all built-in (i.e. handled by g-s-d) shortcuts in capplets/keybindings/00-multimedia-key.xml.in. Apart from the minor nitpicks below the patch looks fine to me. ::: plugins/media-keys/gsd-media-keys-manager.c @@ +617,3 @@ +{ + GConfClient *client = manager->priv->conf_client; + const char *key = "/desktop/gnome/peripherals/touchpad/touchpad_enabled"; I'd prefer a define for this key, too. @@ +945,3 @@ break; + case TOUCHPAD_KEY: + do_touchpad_action(manager); Please add a space here.
Move this addition to before the media playback keys: + TOUCHPAD_KEY, HANDLED_KEYS I don't think we want that key showing up in the keybindings UI, so no need for a control-center patch.
(In reply to comment #4) > ::: plugins/media-keys/gsd-media-keys-manager.c > @@ +617,3 @@ > +{ > + GConfClient *client = manager->priv->conf_client; > + const char *key = > "/desktop/gnome/peripherals/touchpad/touchpad_enabled"; > > I'd prefer a define for this key, too. this is what I meant with the key duplication, the define for this key is part of the first patch but it is in plugins/mouse/gsd-mouse-manager.c. gsd-media-keys-manager.c doesn't have any defines for keys yet, e.g. do_www_action and do_media_action use the keys as a hardcoded string. so having the key as a char string seems more consistent with the current stuff. of course, I can simply replace the const char* with a #define (if that is what you meant anyway). Just let me know what to do and I'll prepare the patch.
(In reply to comment #6) > of course, I can simply replace the const char* with a #define (if that is what > you meant anyway). Yeah, that's what I meant. I don't see a good way to avoid the duplication in this case.
Created attachment 146403 [details] [review] 0001-Add-keybinding-schema-to-enable-disable-touchpad.patch Updated patch, changes to previous patch: - touchpad key moved up to before the media keys in the enum, the schema and the keys[HANDLED_KEYS] array. - const char* changed to a #define - space after do_touchpad_action
Just to add to this, it would be worthwhile adding some user feedback, when this is triggered. For example, Lenovo pops up an on-screen-display informing you that it changed and giving you the chance to cancel if you hit it accidentally.
Then we'd need an icon for it. I must admit that, yes, adding a popup for it is definitely a blocker, otherwise we'll get bug reports about it.
Created attachment 148569 [details] [review] 0001-Display-a-dialog-box-when-enabling-disabling-the-tou.patch Shows an OSD when disabling/enabling the touchpad. Patch applies on top of the other two. The actual code changes are quite small, most of the patch is adding the various icons. The icon itself was taken from gnome-icon-theme, with the X overlay from gnome-bluetooth, the Makefile.am resembles what the xrandr plugin does.
Move that to the top of the file just below the includes, and we should be good. +#define TOUCHPAD_ENABLED_KEY "/desktop/gnome/peripherals/touchpad/touchpad_enabled" The other patch look good to me. Jens?
Created attachment 148572 [details] [review] 0001-Add-keybinding-schema-to-enable-disable-touchpad.patch Changes to previous version: - define moved up to top of the file (see hunk 4) Feel free to squash those patches into one before committing, I mostly kept them separate for easier review.
One question, though: bug 602986 says the icon has been added to g-i-t. I suppose that means we don't need to ship it with g-s-d, then?
the icon in gnome-icon-theme adds it to the 'devices' icon list. what we need in g-s-d seems to be the icons in 'actions', and we need the disable one with the little red cross in addition. (The need for the icons to be in 'actions' seems to be a requirement from the OSD, I didn't dig further into that since it makes sense for this anyway) so the touchpad-enable icons are is the same as the input-touchpad icon in g-i-t. This introduces duplication which isn't ideal but until we have a way to programmatically overlay icons with the red X that duplication seems unavoidable.
The original bug description explains that Fn-F8 is a three-way toggle between trackpoint and touchpad. The key is even printed with a split trackpoint|touchpad pictogram. I would like to be able to toggle the touchpad off, but leave the trackpoint on.
It's a three-way toggle in windows and that is a pure software decision. This patch does not disable the trackpoint, regardless of the state of the touchpad. I wonder about the usefulness of disabling the trackpoint. The touchpad is easily triggered accidentally and with tapping on that can be quite annoying. But the trackpoint? Even a relatively hard accidental touch doesn't move the cursor by much and it doesn't change focus so the impact is quite minimal. I don't quite see why disabling the trackpoint is needed at all other than to have the same behaviour as windows.
Fair enough, I can’t think of a straight-forward and common scenario where one would want to disable the stick either. As long as the trackpoint remains enabled my own needs are satisfied.
Ping, anything holding this up, still ? It would be nice to have...