Only grab the mouse buttons that need to be grabbed. (#2290)

This is a followup to #2049. While we had fixed that bug by only grabbing
buttons 4 and 5 if there is a whole-window binding for that button, this
did not consider buttons higher than 5 as found on many mice.

Therefore, we now ditch the special handling for scrollwheel buttons and
instead do the same for all buttons higher than 3.

fixes #2271

diff --git a/include/bindings.h b/include/bindings.h
index 9b14e98822f34e6751fc7a824c65e77e553c07ad..d15598ea7cab863fa54ccaa28e4c2dbdb56d91e5 100644 (file)
--- a/include/bindings.h
+++ b/include/bindings.h
@@ -104,10 +104,10 @@ CommandResult *run_binding(Binding *bind, Con *con);
 bool load_keymap(void);
 
 /**
- * Returns true if the current config has any binding to a scroll wheel button
- * (4 or 5) which is a whole-window binding.
- * We need this to figure out whether we should grab all buttons or just 1-3
- * when managing a window. See #2049.
- *
+ * Returns a list of buttons that should be grabbed on a window.
+ * This list will always contain 1–3, all higher buttons will only be returned
+ * if there is a whole-window binding for it on some window in the current
+ * config.
+ * The list is terminated by a 0.
  */
-bool bindings_should_grab_scrollwheel_buttons(void);
+int *bindings_get_buttons_to_grab(void);

diff --git a/include/xcb.h b/include/xcb.h
index 86019c5d958dbe0284af58df8ee328d4100ff051..b7eed2cb3321c4d87a089da2bdf0dd7cdf0b8c5e 100644 (file)
--- a/include/xcb.h
+++ b/include/xcb.h
@@ -171,4 +171,4 @@ void xcb_remove_property_atom(xcb_connection_t *conn, xcb_window_t window, xcb_a
  * Grab the specified buttons on a window when managing it.
  *
  */
-void xcb_grab_buttons(xcb_connection_t *conn, xcb_window_t window, bool bind_scrollwheel);
+void xcb_grab_buttons(xcb_connection_t *conn, xcb_window_t window, int *buttons);

diff --git a/src/bindings.c b/src/bindings.c
index 351a5862e7dfd292fae0cad234656c0429ec75fb..759cfff96926d401b1271a28f20ea1ec0032d33f 100644 (file)
--- a/src/bindings.c
+++ b/src/bindings.c
@@ -152,7 +152,7 @@ void grab_all_keys(xcb_connection_t *conn) {
  *
  */
 void regrab_all_buttons(xcb_connection_t *conn) {
-    bool grab_scrollwheel = bindings_should_grab_scrollwheel_buttons();
+    int *buttons = bindings_get_buttons_to_grab();
     xcb_grab_server(conn);
 
     Con *con;
@@ -161,9 +161,10 @@ void regrab_all_buttons(xcb_connection_t *conn) {
             continue;
 
         xcb_ungrab_button(conn, XCB_BUTTON_INDEX_ANY, con->window->id, XCB_BUTTON_MASK_ANY);
-        xcb_grab_buttons(conn, con->window->id, grab_scrollwheel);
+        xcb_grab_buttons(conn, con->window->id, buttons);
     }
 
+    FREE(buttons);
     xcb_ungrab_server(conn);
 }
 
@@ -811,15 +812,30 @@ bool load_keymap(void) {
 }
 
 /*
- * Returns true if the current config has any binding to a scroll wheel button
- * (4 or 5) which is a whole-window binding.
- * We need this to figure out whether we should grab all buttons or just 1-3
- * when managing a window. See #2049.
- *
+ * Returns a list of buttons that should be grabbed on a window.
+ * This list will always contain 1–3, all higher buttons will only be returned
+ * if there is a whole-window binding for it on some window in the current
+ * config.
+ * The list is terminated by a 0.
  */
-bool bindings_should_grab_scrollwheel_buttons(void) {
+int *bindings_get_buttons_to_grab(void) {
+    /* Let's make the reasonable assumption that there's no more than 25
+     * buttons. */
+    int num_max = 25;
+
+    int buffer[num_max];
+    int num = 0;
+
+    /* We always return buttons 1 through 3. */
+    buffer[num++] = 1;
+    buffer[num++] = 2;
+    buffer[num++] = 3;
+
     Binding *bind;
     TAILQ_FOREACH(bind, bindings, bindings) {
+        if (num + 1 == num_max)
+            break;
+
         /* We are only interested in whole window mouse bindings. */
         if (bind->input_type != B_MOUSE || !bind->whole_window)
             continue;
@@ -831,11 +847,18 @@ bool bindings_should_grab_scrollwheel_buttons(void) {
             continue;
         }
 
-        /* If the binding is for either scrollwheel button, we need to grab everything. */
-        if (button == 4 || button == 5) {
-            return true;
+        /* Avoid duplicates. */
+        for (int i = 0; i < num_max; i++) {
+            if (buffer[i] == button)
+                continue;
         }
+
+        buffer[num++] = button;
     }
+    buffer[num++] = 0;
+
+    int *buttons = scalloc(num, sizeof(int));
+    memcpy(buttons, buffer, num * sizeof(int));
 
-    return false;
+    return buttons;
 }

diff --git a/src/manage.c b/src/manage.c
index 81a62ab8dab7c9a5a185ac5f2833a834a9b9a4af..f86e98f5564d656bdb49b646472c383325fd6aea 100644 (file)
--- a/src/manage.c
+++ b/src/manage.c
@@ -170,7 +170,9 @@ void manage_window(xcb_window_t window, xcb_get_window_attributes_cookie_t cooki
     cwindow->id = window;
     cwindow->depth = get_visual_depth(attr->visual);
 
-    xcb_grab_buttons(conn, window, bindings_should_grab_scrollwheel_buttons());
+    int *buttons = bindings_get_buttons_to_grab();
+    xcb_grab_buttons(conn, window, buttons);
+    FREE(buttons);
 
     /* update as much information as possible so far (some replies may be NULL) */
     window_update_class(cwindow, xcb_get_property_reply(conn, class_cookie, NULL), true);

diff --git a/src/xcb.c b/src/xcb.c
index 9d181cfa8c8ba28dbb4b5a35e14aca1c2ec2ce4d..630c68f9cf6ccb847aa236a57378342948354e92 100644 (file)
--- a/src/xcb.c
+++ b/src/xcb.c
@@ -346,20 +346,12 @@ release_grab:
  * Grab the specified buttons on a window when managing it.
  *
  */
-void xcb_grab_buttons(xcb_connection_t *conn, xcb_window_t window, bool bind_scrollwheel) {
-    uint8_t buttons[3];
-    int num = 0;
-
-    if (bind_scrollwheel) {
-        buttons[num++] = XCB_BUTTON_INDEX_ANY;
-    } else {
-        buttons[num++] = XCB_BUTTON_INDEX_1;
-        buttons[num++] = XCB_BUTTON_INDEX_2;
-        buttons[num++] = XCB_BUTTON_INDEX_3;
-    }
-
-    for (int i = 0; i < num; i++) {
+void xcb_grab_buttons(xcb_connection_t *conn, xcb_window_t window, int *buttons) {
+    int i = 0;
+    while (buttons[i] > 0) {
         xcb_grab_button(conn, false, window, XCB_EVENT_MASK_BUTTON_PRESS, XCB_GRAB_MODE_SYNC,
                         XCB_GRAB_MODE_ASYNC, root, XCB_NONE, buttons[i], XCB_BUTTON_MASK_ANY);
+
+        i++;
     }
 }