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 578444 - touchpad support
touchpad support
Status: RESOLVED FIXED
Product: gnome-settings-daemon
Classification: Core
Component: plugins
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gnome-settings-daemon-maint
gnome-settings-daemon-maint
: 590149 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2009-04-09 01:28 UTC by Matthias Clasen
Modified: 2009-08-02 03:58 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch (15.65 KB, patch)
2009-04-09 01:28 UTC, Matthias Clasen
none Details | Review
0001-Support-touchpads-v-2.patch (16.12 KB, patch)
2009-04-10 01:13 UTC, Peter Hutterer
needs-work Details | Review
0001-Support-touchpads-v-3.patch (19.33 KB, patch)
2009-04-13 22:12 UTC, Peter Hutterer
committed Details | Review

Description Matthias Clasen 2009-04-09 01:28:26 UTC
This patch is based on Ubuntu's patch to support touchpads. 
They never upstreamed it. Boo. So we had to do the work, 
in particular, Peter Hutterer.

Adds support for the following keys:
  /desktop/gnome/peripherals/touchpad/disable_while_typing (boolean)
  /desktop/gnome/peripherals/touchpad/tap_to_click (boolean)
  /desktop/gnome/peripherals/touchpad/horiz_scroll_enabled (boolean)
  /desktop/gnome/peripherals/touchpad/scroll_method (integer)

Depending on the key setting, the matching synaptics properties are triggered
on all (!) touchpads. Horizontal scrolling is tied to the vertical scrolling
method.
Tap-to-click sets up the default 1/3/2 button mapping for one finger, two
fingers, and three fingers respectively.
The "disabled while typing" feature spawns off syndaemon.
Comment 1 Matthias Clasen 2009-04-09 01:28:59 UTC
Created attachment 132377 [details] [review]
patch
Comment 2 Matthias Clasen 2009-04-09 01:29:37 UTC
I'm attaching the corresponding control-center patch to bug 154029
Comment 3 Jens Granseuer 2009-04-09 18:34:10 UTC
A few minor beautifications would be nice (mostly function() vs. function ()).

I suppose "Disable while typing" should be hidden if syndaemon isn't available.

+		manager->priv->syndaemon_spawned = False;

I'd prefer to see the GTK define used here (and in other places) instead of the X one.

+		char	*args[4];
+
+		args[0] = "syndaemon";

constify.

+		if (error) {
+			GConfClient *client;
+			client = gconf_client_get_default();
+			gconf_client_set_bool(client, KEY_TOUCHPAD_DISABLE_W_TYPING, FALSE, NULL);
+		}
+

is leaking the error.

Otherwise looks good to go once we've branched.
Comment 4 Matthias Clasen 2009-04-09 18:54:45 UTC
Yeah, sorry. This was written by an X developer and I haven't done the fixups yet.
Comment 5 Peter Hutterer 2009-04-10 01:13:02 UTC
Created attachment 132440 [details] [review]
0001-Support-touchpads-v-2.patch

Updated patch. Amendments:
- constify arguments to syndaemon
- use FALSE instead of False
- don't leak the error when enabling the disable_while_typing feature
- style changes: foo() -> foo ()
Comment 6 Jens Granseuer 2009-04-11 16:13:18 UTC
The new patch has some indentation issues.

Also,
- style changes: foo() -> foo ()

Looks like you missed a few.

+		if (error) {
+			GConfClient *client;
+			client = gconf_client_get_default ();
+			gconf_client_set_bool (client, KEY_TOUCHPAD_DISABLE_W_TYPING, FALSE, NULL);
+		}

Doesn't unref the client, either.

+        g_error_free (error);

Does g_error_free handle NULL errors nowadays? Since the error's only used in the first branch, anyway, why don't you put the variable in there, move the _spawned update before the conditional and immediately free the error in the if branch?
Comment 7 Peter Hutterer 2009-04-13 22:12:20 UTC
Created attachment 132618 [details] [review]
0001-Support-touchpads-v-3.patch

next version, I think this should do it now.

amendments from previous version:
- whitespaces fixed from tabs to spaces
- remaining foo() -> foo ()
- hunk in set_xinput_devices_left_handed removed, it had no effect anyway
- gconf client is freed now.

the interesting part of the diff is:

diff --git a/plugins/mouse/gsd-mouse-manager.c b/plugins/mouse/gsd-mouse-manager.c
index b4a2f57..1d0628a 100644
--- a/plugins/mouse/gsd-mouse-manager.c
+++ b/plugins/mouse/gsd-mouse-manager.c
@@ -487,9 +487,9 @@ device_is_touchpad (XDeviceInfo deviceinfo)
 static int
 set_disable_w_typing (GsdMouseManager *manager, gboolean state)
 {
-	GError	*error = NULL;
 
 	if (state) {
+		GError	*error = NULL;
 		const char *args[4];
 
 		args[0] = "syndaemon";
@@ -504,14 +504,16 @@ set_disable_w_typing (GsdMouseManager *manager, gboolean state)
 				G_SPAWN_SEARCH_PATH, NULL, NULL,
 				&manager->priv->syndaemon_pid, &error);
 
+		manager->priv->syndaemon_spawned = (error == NULL);
+
 		if (error) {
 			GConfClient *client;
 			client = gconf_client_get_default ();
 			gconf_client_set_bool (client, KEY_TOUCHPAD_DISABLE_W_TYPING, FALSE, NULL);
+			g_object_unref (client);
+			g_error_free (error);
 		}
 
-		manager->priv->syndaemon_spawned = (error == NULL);
-
 	} else if (manager->priv->syndaemon_spawned)
 	{
 		kill (manager->priv->syndaemon_pid, SIGHUP);
@@ -519,7 +521,6 @@ set_disable_w_typing (GsdMouseManager *manager, gboolean state)
 		manager->priv->syndaemon_spawned = FALSE;
 	}
 
-        g_error_free (error);
 	return 0;
 }
Comment 8 Jens Granseuer 2009-04-14 16:57:03 UTC
Thanks, looking good.
Comment 9 Peter Hutterer 2009-04-19 23:05:26 UTC
I wonder if we should set the default for scroll_method to 1 in the schema. As it is now, when users install the updated g-s-d scrolling becomes disabled because of this default key. This again leads users to think that the driver broke somehow.
Comment 10 André Klapper 2009-05-03 19:33:17 UTC
Now that gnome-settings-daemon has branched, can this please get committed to master?
Comment 11 Hiroyuki Ikezoe 2009-08-02 03:58:59 UTC
*** Bug 590149 has been marked as a duplicate of this bug. ***