Created attachment 132377 [details] [review]
patch

I'm attaching the corresponding control-center patch to bug 154029

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.

Yeah, sorry. This was written by an X developer and I haven't done the fixups yet.

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 ()

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?

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;
 }

Thanks, looking good.

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.

Now that gnome-settings-daemon has branched, can this please get committed to master?

*** Bug 590149 has been marked as a duplicate of this bug. ***