GNOME Bugzilla – Bug 601330
Touchpad features should be available based on the touchpad's capabilities.
Last modified: 2009-12-17 10:01:30 UTC
The "Touchpad" tab in the mouse properties dialog allows for enabling tapping and a scroll method (amongst other things). These two should be conditional on the touchpad's capabilities, i.e. - A device that doesn't have physical buttons should have tapping on by default (see Bug 598287) and the checkbox should be disabled. Otherwise, disabling tapping leaves the user with a unusable touchpad. - A device that doesn't do multi-finger detection should disable the two-finger scrolling method. Otherwise, it appears the scrolling method doesn't work, leading to false bug-reports. The synaptics driver provides a "Synaptics Capabilities" property with 5 boolean 8 bit values in the order left, middle, right, double, triple. Any of those values set to 0 means the capability is not present on the device. http://cgit.freedesktop.org/xorg/driver/xf86-input-synaptics/tree/include/synaptics-properties.h
Created attachment 147355 [details] [review] 0001-Disable-two-finger-scrolling-tapping-based-on-touchp.patch How about something like this? Simply disables the GUI options if the touchpad doesn't have the matching capabilities.
Review of attachment 147355 [details] [review]: How does this work with two (more?) touchpads (if that's a valid scenario)? ::: capplets/mouse/gnome-mouse-properties.c @@ +329,3 @@ +#ifdef HAVE_XINPUT +static void synaptics_check_capabilities(GtkBuilder *dialog, XDevice *device) The function signature should be broken up and is missing a space in frint of the "(". @@ +348,3 @@ + if (!data[0] || !data[1] || !data[2]) { + gtk_widget_set_sensitive (WID ("tap_to_click_toggle"), 0); + gtk_toggle_button_set_active (GTK_TOGGLE_BUTTON (WID ("tap_to_click_toggle")), 1); I'm a bit worried we might display bogus data here. If you set the widget to insensitive it won't send notifications when its value changes (I think), so the "set_active" part only has the effect of changing the knob in the UI so that things look ok to the user even though the current setting in use might be something entirely diferent. @@ +352,3 @@ + + if (!data[3]) + gtk_widget_set_sensitive (WID ("scroll_twofinger_radio"), 0); Use TRUE/FALSE for these calls instead of 1/0.
(In reply to comment #2) > Review of attachment 147355 [details] [review]: > How does this work with two (more?) touchpads (if that's a valid scenario)? It doesn't. The question though is whether we should cater for that scenario or deal with it when it actually arrives. I don't know if future laptops with multiple touchpads will have two different models or the same one anyway. i think usb-connected touchpads are a rather narrow case. anyway, the input device configuration applets are currently designed for only one mouse, one keyboard, one touchpad, etc. they need some sort of rewrite soon, but this isn't part of it yet :) > ::: capplets/mouse/gnome-mouse-properties.c > @@ +329,3 @@ > > +#ifdef HAVE_XINPUT > +static void synaptics_check_capabilities(GtkBuilder *dialog, XDevice *device) > > The function signature should be broken up and is missing a space in frint of > the "(". sigh. one day I'll manage to spot all of them before sending a patch. thanks. > @@ +348,3 @@ > + if (!data[0] || !data[1] || !data[2]) { > + gtk_widget_set_sensitive (WID ("tap_to_click_toggle"), 0); > + gtk_toggle_button_set_active (GTK_TOGGLE_BUTTON (WID > ("tap_to_click_toggle")), 1); > > I'm a bit worried we might display bogus data here. If you set the widget to > insensitive it won't send notifications when its value changes (I think), so > the "set_active" part only has the effect of changing the knob in the UI so > that things look ok to the user even though the current setting in use might be > something entirely diferent. if we just swap the two around, the set_active should still send the notification, right? that'd be the easy solution to this. (fwiw, when I tested the patch, I removed the checkbox tick with gconf-editor and it came back after running the capplet) we could have similar code in g-s-d to force the values based on the same capabilities. though that would in some ways require a communication channel with the capplet, just in case g-s-d prohibits a capplet setting and reset it to default. whether this is necessary is questionable though. i'll attach the fixed up patch once I hear back from you about this point.
(In reply to comment #3) > > How does this work with two (more?) touchpads (if that's a valid scenario)? > > It doesn't. [...] > anyway, the input device configuration applets are currently designed for only > one mouse, one keyboard, one touchpad, etc. they need some sort of rewrite > soon, but this isn't part of it yet :) Fair enough. ;-) > if we just swap the two around, the set_active should still send the > notification, right? Yes. > (fwiw, when I tested the patch, I removed the checkbox tick with gconf-editor > and it came back after running the capplet) Hm, ok. In that case (I didn't test) I guess it fine as is and doesn't need changing. > we could have similar code in g-s-d to force the values based on the same > capabilities. though that would in some ways require a communication channel > with the capplet, just in case g-s-d prohibits a capplet setting and reset it > to default. whether this is necessary is questionable though. Doing this in the capplet only seems to be sufficient to me.
Created attachment 147525 [details] [review] 0001-Disable-two-finger-scrolling-tapping-based-on-touchp.patch Previous patch was flawed, not sure why that worked yesterday. New attempt, tested with my physical synaptics (no multifinger) and a uninput software emulation (no buttons). If the capabilities are checked during find_synaptics() it seems the gconf_peditor_new_whatever overwrite the checks with the settings from gconf. i.e. once it's disabled, it doesn't get enabled again. So we have to check for capabilities after hooking everything up.
Review of attachment 147525 [details] [review]: I've committed the patch with these niggles fixed. ::: capplets/mouse/gnome-mouse-properties.c @@ +328,3 @@ } +static void synaptics_check_capabilities (GtkBuilder *dialog) static void synaptics_check_capabilities (GtkBuilder *dialog) @@ +501,3 @@ NULL); + synaptics_check_capabilities(dialog); synaptics_check_capabilities (dialog);
*** Bug 598287 has been marked as a duplicate of this bug. ***