GNOME Bugzilla – Bug 685941
mouse: re-enable touchpad when mouse isn't present
Last modified: 2015-08-06 22:54:42 UTC
Created attachment 226240 [details] [review] mouse: re-enable touchpad when mouse isn't present We need to re-enable touchpad when mouse isn't present. It is corresponding with: https://bugzilla.gnome.org/show_bug.cgi?id=685583
Review of attachment 226240 [details] [review]: ::: plugins/mouse/gsd-mouse-manager.c @@ +1165,3 @@ + + /* Re-enable touchpad when mouse isn't present. */ + if (!mouse_is_present () && touchpad_is_present ()) I prefer: mouse_is_present() == FALSE @@ +1240,3 @@ + /* Re-enable touchpad when mouse isn't present. */ + if (!mouse_is_present () && touchpad_is_present ()) + g_settings_set_boolean (manager->priv->touchpad_settings, KEY_TOUCHPAD_ENABLED, TRUE); Given that the 2 pieces of code are the exact same, maybe move it to a separate function? (ensure_touchpad_active() or something). Also, what happens if I have a touchscreen instead of a mouse? It should probably check for the touchscreen not being there before enabling this (and we might add support for trackballs too, which would again complicate matters).
Created attachment 226244 [details] [review] common: add trackball detection
Created attachment 226245 [details] [review] mouse: re-enable touchpad when mouse isn't present
Actually I haven't trackball so I haven't tested trackball detection yet.
Review of attachment 226244 [details] [review]: You'll need to add the output to the test application.
Review of attachment 226245 [details] [review]: Looks good. ::: plugins/mouse/gsd-mouse-manager.c @@ +1129,3 @@ +ensure_touchpad_active (GsdMouseManager *manager) +{ + if ((mouse_is_present () == FALSE) && (touchscreen_is_present () == FALSE) && (trackball_is_present () == FALSE) && touchpad_is_present ()) You don't need that many parenthesis: - if ((mouse_is_present () == FALSE) && (touchscreen_is_present () == FALSE) && (trackball_is_present () == FALSE) && touchpad_is_present ()) + if (mouse_is_present () == FALSE && touchscreen_is_present () == FALSE && trackball_is_present () == FALSE && touchpad_is_present ())
Created attachment 226246 [details] [review] common: Add trackball test
Review of attachment 226244 [details] [review]: Except that it doesn't work with my test trackball: ⎜ ↳ Logitech USB Trackball id=15 [slave pointer (2)]
The udev properties for the device: P: /devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.1/2-1.1:1.0/input/input12/event12 N: input/event12 S: input/by-id/usb-Logitech_USB_Trackball-event-mouse S: input/by-path/pci-0000:00:1d.0-usb-0:1.1:1.0-event-mouse E: DEVLINKS=/dev/input/by-id/usb-Logitech_USB_Trackball-event-mouse /dev/input/by-path/pci-0000:00:1d.0-usb-0:1.1:1.0-event-mouse E: DEVNAME=/dev/input/event12 E: DEVPATH=/devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.1/2-1.1:1.0/input/input12/event12 E: ID_BUS=usb E: ID_INPUT=1 E: ID_INPUT_MOUSE=1 E: ID_MODEL=USB_Trackball E: ID_MODEL_ENC=USB\x20Trackball E: ID_MODEL_ID=c408 E: ID_PATH=pci-0000:00:1d.0-usb-0:1.1:1.0 E: ID_PATH_TAG=pci-0000_00_1d_0-usb-0_1_1_1_0 E: ID_REVISION=1400 E: ID_SERIAL=Logitech_USB_Trackball E: ID_TYPE=hid E: ID_USB_DRIVER=usbhid E: ID_USB_INTERFACES=:030102: E: ID_USB_INTERFACE_NUM=00 E: ID_VENDOR=Logitech E: ID_VENDOR_ENC=Logitech E: ID_VENDOR_ID=046d E: MAJOR=13 E: MINOR=76 E: SUBSYSTEM=input E: USEC_INITIALIZED=213188749
Filed: https://bugzilla.freedesktop.org/show_bug.cgi?id=55867 You can carry on by adding a search for "trackball" to the is_trackball function.
Created attachment 226255 [details] [review] common: add trackball detection Added trackball detection from the product name. Does it work for you?
Created attachment 226256 [details] [review] mouse: re-enable touchpad when mouse isn't present Without redundant parentheses.
Review of attachment 226255 [details] [review]: ::: plugins/common/gsd-input-helper.c @@ +201,3 @@ +{ + gint retval; + gchar *lowercase; Move those declarations to the if branch. @@ +205,3 @@ + + retval = (device_info->type == XInternAtom (GDK_DISPLAY_XDISPLAY (gdk_display_get_default ()), XI_TRACKBALL, False)); + if (retval == FALSE) { Check for device_info->name not being NULL. @@ +207,3 @@ + if (retval == FALSE) { + length = strlen (device_info->name); + lowercase = g_ascii_strdown (device_info->name, length); Use -1, instead of length, and you can use that. @@ +208,3 @@ + length = strlen (device_info->name); + lowercase = g_ascii_strdown (device_info->name, length); + retval = g_strstr_len (lowercase, length, "trackball"); Use strstr() and don't use the length.
Review of attachment 226256 [details] [review]: ++
All committed with the changes discussed, thanks for the patches!
The fix proposed here is causing problems with laptops that have a touchpad toggle hotkey which is handled by userspace (sometimes the HW itself enables/disables the touchpad when this key is pressed). This is what happens when pressing the touchpad toggle hotkey: * 1st press: OSD "touchpad disabled" shows up, cursor movement is disabled, but the touchpad settings are still available under "Mouse & Touchpad" in g-c-c. * 2nd press: OSD "touchpad disabled" shows up, cursor movement remains disabled, and now the touchpad settings are greyed out under "Mouse & Touchpad" in g-c-c. * 3rd press: OSD "touchpad enabled" shows up, cursor movement is re-enabled and touchpad settings are available again under "Mouse & Touchpad" in g-c-c, but a new ON|OFF switch is created in the touchpad part of the "Mouse & Touchpad" dialog (which was not initially there). Reverting the patch introduced here makes the touchpad toggle key behave sanely, that is, on the 1st press cursor movement and g-c-c settings are disabled, and on the 2nd press both are enabled again, but of course the feature introduced here is lost. My attempt to fix both problems at the same time is bellow (path can be found at https://github.com/endlessm/gnome-settings-daemon/commit/a0e016081705cc5f437a17476588c258d1b1af4a for now), but this triggers a SEGV when removing a USB keyboard+touchpad (Logitech K400r) combo from the system. I ll continue to investigate this problem but any ideas are welcome in the mean time. diff --git a/plugins/mouse/gsd-mouse-manager.c b/plugins/mouse/gsd-mouse-manager.c index 3815a2e..99843e1 100644 --- a/plugins/mouse/gsd-mouse-manager.c +++ b/plugins/mouse/gsd-mouse-manager.c @@ -1155,12 +1155,16 @@ device_removed_cb (GdkDeviceManager *device_manager, GINT_TO_POINTER (id)); if (device_is_ignored (manager, device) == FALSE) { + XDevice *xdevice; run_custom_command (device, COMMAND_DEVICE_REMOVED); /* If a touchpad was to disappear... */ set_disable_w_typing (manager, g_settings_get_boolean (manager->priv->touchpad_settings, KEY_TOUCHPAD_DISABLE_W_TYPING)); - ensure_touchpad_active (manager); + xdevice = open_gdk_device (device); + if (xdevice != NULL && device_is_touchpad (xdevice) == FALSE) + ensure_touchpad_active (manager); + xdevice_close (xdevice); } }
The SEGV here comes from the fact that open_gdk_device() is returning NULL since the X device is already gone, which shows this approach would not work anyway.
This was actually fixed in https://git.gnome.org/browse/gnome-settings-daemon/commit/?id=ca754de5039fed6cb96b883dd8e41d8b22ebeea6. Sorry for the noise.