Freeze break request: #598820 Add tap-to-click button settings to gnome-settings-daemon



Hello GNOME Team,

I would like to request a feature freeze break to get the bug
#598820[0] fixed in GNOME 2.30.

The nature of the bug is insufficient configurability: After
tap-to-click enable/disable switch was added to
gnome-settings-daemon[1], users no longer can configure which taps
trigger which buttons, persistently. A workaround involving synclient
is available, but gnome-settings-daemon restores its hardcoded mapping
on each user logon and whenever the user toggles the enable switch via
the UI. The hardcoded mapping is: one finger -> primary (left) button;
two fingers -> secondary (right) button; three fingers -> middle
button.

Impact of the bug: The bug affects users who have multitouch-capable
touchpads. Because a lot of touchpads have two hardware buttons, many
users want a simpler gesture than a three-button tap for a middle
click. Further, users are accustomed to 1-L, 2-M, 3-R mapping because
it was default for the synaptics driver.

Impact of the patch: The patch is a feature change with no UI except
for three new gconf settings that the user can set to map one-, two-
and three-finger taps to emulate any mouse button click events.

Why this should not wait for next release: The bug being fixed is a
usability regression between gnome-settings-daemon v2.27.3 and
v2.27.4.

Review status: The patch was reviewed by Peter Hutterer
<peter hutterer who-t net>, the developer who originally introduced
the tap-to-click feature to gnome-settings-daemon; and Jens Granseuer
<jensgr gmx net>, gnome-settings-daemon developer/maintainer. Jens
Granseuer also said he would accept the patch if freeze break is
granted[2].

Testing status: A few people have reported that an earlier version of
this patch works for them[3,4].

[0] https://bugzilla.gnome.org/show_bug.cgi?id=598820
[1] https://bugzilla.gnome.org/show_bug.cgi?id=578444
[2] https://bugzilla.gnome.org/show_bug.cgi?id=598820#c36
[3] https://bugzilla.gnome.org/show_bug.cgi?id=598820#c13
[4] https://bugzilla.gnome.org/show_bug.cgi?id=598820#c18
From 9824bab1f577b2e88dceb1f68161ba8d358a9c27 Mon Sep 17 00:00:00 2001
From: Yuri Khan <yurivkhan gmail com>
Date: Thu, 4 Feb 2010 10:45:23 +0600
Subject: [PATCH] Add tap-to-click button settings

Fixes bug #598820.
---
 data/desktop_gnome_peripherals_touchpad.schemas.in |   36 ++++++
 plugins/mouse/gsd-mouse-manager.c                  |  113 ++++++++++++++------
 2 files changed, 116 insertions(+), 33 deletions(-)

diff --git a/data/desktop_gnome_peripherals_touchpad.schemas.in b/data/desktop_gnome_peripherals_touchpad.schemas.in
index 78734e7..6c391a2 100644
--- a/data/desktop_gnome_peripherals_touchpad.schemas.in
+++ b/data/desktop_gnome_peripherals_touchpad.schemas.in
@@ -27,6 +27,42 @@
     </schema>
 
     <schema>
+      <key>/schemas/desktop/gnome/peripherals/touchpad/tap_button_1</key>
+      <applyto>/desktop/gnome/peripherals/touchpad/tap_button_1</applyto>
+      <owner>gnome</owner>
+      <type>int</type>
+      <default>1</default>
+      <locale name="C">
+        <short>Button assigned to one-finger tap</short>
+        <long>Set this to the button number that you want to click by tapping one finger.</long>
+      </locale>
+    </schema>
+
+    <schema>
+      <key>/schemas/desktop/gnome/peripherals/touchpad/tap_button_2</key>
+      <applyto>/desktop/gnome/peripherals/touchpad/tap_button_2</applyto>
+      <owner>gnome</owner>
+      <type>int</type>
+      <default>3</default>
+      <locale name="C">
+        <short>Button assigned to two-finger tap</short>
+        <long>Set this to the button number that you want to click by tapping two fingers.</long>
+      </locale>
+    </schema>
+
+    <schema>
+      <key>/schemas/desktop/gnome/peripherals/touchpad/tap_button_3</key>
+      <applyto>/desktop/gnome/peripherals/touchpad/tap_button_3</applyto>
+      <owner>gnome</owner>
+      <type>int</type>
+      <default>2</default>
+      <locale name="C">
+        <short>Button assigned to three-finger tap</short>
+        <long>Set this to the button number that you want to click by tapping three fingers.</long>
+      </locale>
+    </schema>
+
+    <schema>
       <key>/schemas/desktop/gnome/peripherals/touchpad/scroll_method</key>
       <applyto>/desktop/gnome/peripherals/touchpad/scroll_method</applyto>
       <owner>gnome</owner>
diff --git a/plugins/mouse/gsd-mouse-manager.c b/plugins/mouse/gsd-mouse-manager.c
index 4371081..b75c3f6 100644
--- a/plugins/mouse/gsd-mouse-manager.c
+++ b/plugins/mouse/gsd-mouse-manager.c
@@ -64,6 +64,9 @@
 #define KEY_DELAY_ENABLE        GCONF_MOUSE_A11Y_DIR "/delay_enable"
 #define KEY_TOUCHPAD_DISABLE_W_TYPING    GCONF_TOUCHPAD_DIR "/disable_while_typing"
 #define KEY_TAP_TO_CLICK        GCONF_TOUCHPAD_DIR "/tap_to_click"
+#define KEY_TAP_BUTTON_1        GCONF_TOUCHPAD_DIR "/tap_button_1"
+#define KEY_TAP_BUTTON_2        GCONF_TOUCHPAD_DIR "/tap_button_2"
+#define KEY_TAP_BUTTON_3        GCONF_TOUCHPAD_DIR "/tap_button_3"
 #define KEY_SCROLL_METHOD       GCONF_TOUCHPAD_DIR "/scroll_method"
 #define KEY_PAD_HORIZ_SCROLL    GCONF_TOUCHPAD_DIR "/horiz_scroll_enabled"
 #define KEY_TOUCHPAD_ENABLED    GCONF_TOUCHPAD_DIR "/touchpad_enabled"
@@ -85,8 +88,10 @@ static void     gsd_mouse_manager_class_init  (GsdMouseManagerClass *klass);
 static void     gsd_mouse_manager_init        (GsdMouseManager      *mouse_manager);
 static void     gsd_mouse_manager_finalize    (GObject             *object);
 static void     set_mouse_settings            (GsdMouseManager      *manager);
-static int      set_tap_to_click              (gboolean state, gboolean left_handed);
+static int      set_tap_to_click              (gboolean state, guint tap_button_1, guint tap_button_2, guint tap_button_3);
+static int      update_tap_to_click           (void);
 static XDevice* device_is_touchpad            (XDeviceInfo deviceinfo);
+static gint     get_device_button_mapping     (guchar *buttons, gsize *buttons_capacity);
 
 G_DEFINE_TYPE (GsdMouseManager, gsd_mouse_manager, G_TYPE_OBJECT)
 
@@ -291,6 +296,25 @@ touchpad_has_single_button (XDevice *device)
         return is_single_button;
 }
 
+static gint
+get_device_button_mapping (guchar *buttons, gsize *buttons_capacity)
+{
+        gint n_buttons = XGetDeviceButtonMapping (GDK_DISPLAY (), device,
+                                                  buttons,
+                                                  *buttons_capacity);
+
+        while (n_buttons > *buttons_capacity) {
+                *buttons_capacity = n_buttons;
+                buttons = (guchar *) g_realloc (buttons,
+                                                *buttons_capacity * sizeof (guchar));
+
+                n_buttons = XGetDeviceButtonMapping (GDK_DISPLAY (), device,
+                                                     buttons,
+                                                     *buttons_capacity);
+        }
+
+        return n_buttons;
+}
 
 static void
 set_xinput_devices_left_handed (gboolean left_handed)
@@ -322,11 +346,8 @@ set_xinput_devices_left_handed (gboolean left_handed)
                 device = device_is_touchpad (device_info[i]);
                 if (device != NULL) {
                         GConfClient *client = gconf_client_get_default ();
-                        gboolean tap = gconf_client_get_bool (client, KEY_TAP_TO_CLICK, NULL);
                         gboolean single_button = touchpad_has_single_button (device);
 
-                        if (tap && !single_button)
-                                set_tap_to_click (tap, left_handed);
                         XCloseDevice (GDK_DISPLAY (), device);
                         g_object_unref (client);
 
@@ -342,19 +363,7 @@ set_xinput_devices_left_handed (gboolean left_handed)
                     (device == NULL))
                         continue;
 
-                n_buttons = XGetDeviceButtonMapping (GDK_DISPLAY (), device,
-                                                     buttons,
-                                                     buttons_capacity);
-
-                while (n_buttons > buttons_capacity) {
-                        buttons_capacity = n_buttons;
-                        buttons = (guchar *) g_realloc (buttons,
-                                                        buttons_capacity * sizeof (guchar));
-
-                        n_buttons = XGetDeviceButtonMapping (GDK_DISPLAY (), device,
-                                                             buttons,
-                                                             buttons_capacity);
-                }
+                n_buttons = get_device_button_mapping (buttons, &buttons_capacity);
 
                 configure_button_layout (buttons, n_buttons, left_handed);
 
@@ -365,6 +374,8 @@ set_xinput_devices_left_handed (gboolean left_handed)
 
         if (device_info != NULL)
                 XFreeDeviceList (device_info);
+
+        update_tap_to_click ();
 }
 
 static GdkFilterReturn
@@ -581,14 +592,20 @@ set_disable_w_typing (GsdMouseManager *manager, gboolean state)
 }
 
 static int
-set_tap_to_click (gboolean state, gboolean left_handed)
+set_tap_to_click (gboolean state, guint tap_button_1, guint tap_button_2, guint tap_button_3)
 {
-        int numdevices, i, format, rc;
+        int numdevices, i, j, format, rc;
         unsigned long nitems, bytes_after;
         XDeviceInfo *devicelist = XListInputDevices (GDK_DISPLAY (), &numdevices);
         XDevice * device;
         unsigned char* data;
         Atom prop, type;
+        guchar *buttons;
+        gsize buttons_capacity = 16;
+        gint n_buttons;
+        gint tap_physical_1 = tap_button_1,
+             tap_physical_2 = tap_button_2,
+             tap_physical_3 = tap_button_3;
 
         if (devicelist == NULL)
                 return 0;
@@ -598,19 +615,35 @@ set_tap_to_click (gboolean state, gboolean left_handed)
         if (!prop)
                 return 0;
 
+        buttons = g_new (guchar, buttons_capacity);
+
         for (i = 0; i < numdevices; i++) {
                 if ((device = device_is_touchpad (devicelist[i]))) {
                         gdk_error_trap_push ();
+
+                        n_buttons = get_device_button_mapping (buttons, &buttons_capacity);
+
+                        /* Synaptics maps taps to physical buttons. X then
+                         * maps physical buttons to logical buttons. Since we
+                         * want to let the user specify logical buttons
+                         * directly for taps, we need to reverse the X mapping.
+                         */
+                        for (j = 0; j < n_buttons; j++)
+                        {
+                                if (buttons[j] == tap_button_1) tap_physical_1 = j + 1;
+                                if (buttons[j] == tap_button_2) tap_physical_2 = j + 1;
+                                if (buttons[j] == tap_button_3) tap_physical_3 = j + 1;
+                        }
+
                         rc = XGetDeviceProperty (GDK_DISPLAY (), device, prop, 0, 2,
                                                 False, XA_INTEGER, &type, &format, &nitems,
                                                 &bytes_after, &data);
 
                         if (rc == Success && type == XA_INTEGER && format == 8 && nitems >= 7)
                         {
-                                /* Set RLM mapping for 1/2/3 fingers*/
-                                data[4] = (state) ? ((left_handed) ? 3 : 1) : 0;
-                                data[5] = (state) ? ((left_handed) ? 1 : 3) : 0;
-                                data[6] = (state) ? 2 : 0;
+                                data[4] = (state) ? tap_physical_1 : 0;
+                                data[5] = (state) ? tap_physical_2 : 0;
+                                data[6] = (state) ? tap_physical_3 : 0;
                                 XChangeDeviceProperty (GDK_DISPLAY (), device, prop, XA_INTEGER, 8,
                                                         PropModeReplace, data, nitems);
                         }
@@ -625,11 +658,28 @@ set_tap_to_click (gboolean state, gboolean left_handed)
                 }
         }
 
+        g_free (buttons);
+
         XFreeDeviceList (devicelist);
         return 0;
 }
 
 static int
+update_tap_to_click ()
+{
+        GConfClient *client = gconf_client_get_default ();
+
+        int result = set_tap_to_click (gconf_client_get_bool (client, KEY_TAP_TO_CLICK, NULL),
+                                       gconf_client_get_int  (client, KEY_TAP_BUTTON_1, NULL),
+                                       gconf_client_get_int  (client, KEY_TAP_BUTTON_2, NULL),
+                                       gconf_client_get_int  (client, KEY_TAP_BUTTON_3, NULL));
+
+        g_object_unref (client);
+
+        return result;
+}
+
+static int
 set_horiz_scroll (gboolean state)
 {
         int numdevices, i, rc;
@@ -891,14 +941,12 @@ static void
 set_mouse_settings (GsdMouseManager *manager)
 {
         GConfClient *client = gconf_client_get_default ();
-        gboolean left_handed = gconf_client_get_bool (client, KEY_LEFT_HANDED, NULL);
 
-        set_left_handed (manager, left_handed);
+        set_left_handed (manager, gconf_client_get_bool (client, KEY_LEFT_HANDED, NULL));
         set_motion_acceleration (manager, gconf_client_get_float (client, KEY_MOTION_ACCELERATION , NULL));
         set_motion_threshold (manager, gconf_client_get_int (client, KEY_MOTION_THRESHOLD, NULL));
 
         set_disable_w_typing (manager, gconf_client_get_bool (client, KEY_TOUCHPAD_DISABLE_W_TYPING, NULL));
-        set_tap_to_click (gconf_client_get_bool (client, KEY_TAP_TO_CLICK, NULL), left_handed);
         set_edge_scroll (gconf_client_get_int (client, KEY_SCROLL_METHOD, NULL));
         set_horiz_scroll (gconf_client_get_bool (client, KEY_PAD_HORIZ_SCROLL, NULL));
         set_touchpad_enabled (gconf_client_get_bool (client, KEY_TOUCHPAD_ENABLED, NULL));
@@ -927,11 +975,11 @@ mouse_callback (GConfClient        *client,
         } else if (! strcmp (entry->key, KEY_TOUCHPAD_DISABLE_W_TYPING)) {
                 if (entry->value->type == GCONF_VALUE_BOOL)
                         set_disable_w_typing (manager, gconf_value_get_bool (entry->value));
-        } else if (! strcmp (entry->key, KEY_TAP_TO_CLICK)) {
-                if (entry->value->type == GCONF_VALUE_BOOL) {
-                        set_tap_to_click (gconf_value_get_bool (entry->value),
-                                          gconf_client_get_bool (client, KEY_LEFT_HANDED, NULL));
-                }
+        } else if ((! strcmp (entry->key, KEY_TAP_TO_CLICK) && entry->value->type == GCONF_VALUE_BOOL)
+               ||  (! strcmp (entry->key, KEY_TAP_BUTTON_1) && entry->value->type == GCONF_VALUE_INT)
+               ||  (! strcmp (entry->key, KEY_TAP_BUTTON_2) && entry->value->type == GCONF_VALUE_INT)
+               ||  (! strcmp (entry->key, KEY_TAP_BUTTON_3) && entry->value->type == GCONF_VALUE_INT)) {
+                        update_tap_to_click ();
         } else if (! strcmp (entry->key, KEY_SCROLL_METHOD)) {
                 if (entry->value->type == GCONF_VALUE_INT) {
                         set_edge_scroll (gconf_value_get_int (entry->value));
@@ -1015,8 +1063,7 @@ gsd_mouse_manager_idle_cb (GsdMouseManager *manager)
                                 gconf_client_get_bool (client, KEY_DELAY_ENABLE, NULL));
 
         set_disable_w_typing (manager, gconf_client_get_bool (client, KEY_TOUCHPAD_DISABLE_W_TYPING, NULL));
-        set_tap_to_click (gconf_client_get_bool (client, KEY_TAP_TO_CLICK, NULL),
-                          gconf_client_get_bool (client, KEY_LEFT_HANDED, NULL));
+        update_tap_to_click ();
         set_edge_scroll (gconf_client_get_int (client, KEY_SCROLL_METHOD, NULL));
         set_horiz_scroll (gconf_client_get_bool (client, KEY_PAD_HORIZ_SCROLL, NULL));
         set_touchpad_enabled (gconf_client_get_bool (client, KEY_TOUCHPAD_ENABLED, NULL));
-- 
1.6.3.3



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]