GNOME Bugzilla – Bug 775755
Add tag-and-drag setting from libinput into mutter
Last modified: 2018-01-12 11:01:21 UTC
The problem is that libinput offers the possibility to not enabled dragging when tap-to-click is enabled but mutter doesn't. For people who have a sensitive touchpad and who like tap-to-click option, dragging is launched even when you don't want it : for example, when you select a folder, most of the time the folder is dragging whereas just selected or when you want to select some lines of a text file, several lines are moved as a cut-paste which is not expected and erase datas. To fix it, you need to have the possibility to desactivate the drag option when you use tap-to-click in mutter. Because it's already a specification of libinput, it remains to add it to mutter.
Created attachment 341542 [details] [review] Suggestion of code
tap-and-drag is already in specification of libinput but not in mutter, gnome-control-center nor gsettings-desktop-deamons. So, 3 commits are necessary to let the possibility to people to choose tap-and-drag or not. That's why I opened 3 bugs for 3 modules of gnome : mutter: https://bugzilla.gnome.org/show_bug.cgi?id=775755 gsettings-desktop-deamons: https://bugzilla.gnome.org/show_bug.cgi?id=775756 gnome-control-center: https://bugzilla.gnome.org/show_bug.cgi?id=775324
Mutter can be updated now because gsettings-desktop-deamons were updated to add this functionality.
Review of attachment 341542 [details] [review]: This should also contain an the X11 implementation. Otherwise looks good. ::: src/backends/meta-input-settings.c @@ +473,3 @@ static void +update_touchpad_tap_and_drag_enabled (MetaInputSettings *input_settings, + ClutterInputDevice *device) nit: indentation wrong ::: src/backends/native/meta-input-settings-native.c @@ +138,3 @@ + enabled ? + LIBINPUT_CONFIG_DRAG_ENABLED : + LIBINPUT_CONFIG_DRAG_DISABLED); nit: indentation wrong.
(In reply to freeroot from comment #1) > Created attachment 341542 [details] [review] [review] > Suggestion of code Can you add an X11 implementation like Jonas said please ? It's a really important feature to be able to disable tap and drag because it can lead to usability problem like I had : https://bugs.freedesktop.org/show_bug.cgi?id=100122
(In reply to Jonas Ådahl from comment #4) > Review of attachment 341542 [details] [review] [review]: > > This should also contain an the X11 implementation. Otherwise looks good. > > ::: src/backends/meta-input-settings.c > @@ +473,3 @@ > static void > +update_touchpad_tap_and_drag_enabled (MetaInputSettings *input_settings, > + ClutterInputDevice *device) > > nit: indentation wrong > > ::: src/backends/native/meta-input-settings-native.c > @@ +138,3 @@ > + enabled ? > + LIBINPUT_CONFIG_DRAG_ENABLED : > + LIBINPUT_CONFIG_DRAG_DISABLED); > > nit: indentation wrong. I don't know how to add the X11 implementation, I hope for the moment.^^ If someone knows it, she/he will be welcome :) But like Bastien Nocera said ( https://bugzilla.gnome.org/show_bug.cgi?id=775324#c7 ), it's still changeable in Xorg, so, is it necessary to validate this patch ?
(In reply to freeroot from comment #6) > (In reply to Jonas Ådahl from comment #4) > > Review of attachment 341542 [details] [review] [review] [review]: > > > > This should also contain an the X11 implementation. Otherwise looks good. > > > > ::: src/backends/meta-input-settings.c > > @@ +473,3 @@ > > static void > > +update_touchpad_tap_and_drag_enabled (MetaInputSettings *input_settings, > > + ClutterInputDevice *device) > > > > nit: indentation wrong > > > > ::: src/backends/native/meta-input-settings-native.c > > @@ +138,3 @@ > > + enabled ? > > + LIBINPUT_CONFIG_DRAG_ENABLED : > > + LIBINPUT_CONFIG_DRAG_DISABLED); > > > > nit: indentation wrong. > > I don't know how to add the X11 implementation, I hope for the moment.^^ If > someone knows it, she/he will be welcome :) But like Bastien Nocera said ( > https://bugzilla.gnome.org/show_bug.cgi?id=775324#c7 ), it's still > changeable in Xorg, so, is it necessary to validate this patch ? It should be easy enough to just copy paste meta_input_settings_x11_set_tap_enabled(), change the name of the function, change "libinput Tapping Enabled" to "libinput TappingDrag Enabled", add it to the class in the same way as in -native.c.
Created attachment 357845 [details] [review] Update of suggestion of code Ok. I've done it with a new patch and indentations are the same as those in copied functions. That's it ?
Review of attachment 357845 [details] [review]: Yepp. Now there are just a few coding style issues left. ::: src/backends/meta-input-settings-private.h @@ +57,3 @@ + void (* set_tap_and_drag_enabled) (MetaInputSettings *settings, + ClutterInputDevice *device, + gboolean enabled); Wrong indentation. ::: src/backends/meta-input-settings.c @@ +529,3 @@ } + static void Stray whitespace @@ +531,3 @@ + static void +update_touchpad_tap_and_drag_enabled (MetaInputSettings *input_settings, + ClutterInputDevice *device) Wrong indentation @@ +559,3 @@ +} + + Stray newline ::: src/backends/native/meta-input-settings-native.c @@ +137,3 @@ + enabled ? + LIBINPUT_CONFIG_DRAG_ENABLED : + LIBINPUT_CONFIG_DRAG_DISABLED); Wrong indentation @@ +140,3 @@ +} + + Stray newline ::: src/backends/x11/meta-input-settings-x11.c @@ +236,3 @@ +meta_input_settings_x11_set_tap_and_drag_enabled (MetaInputSettings *settings, + ClutterInputDevice *device, + gboolean enabled) Wrong indentation
I don't understand how to correct those wrong indentations because I just copy paste previous functions that have those indentations. And why is it an issue now whereas it were accepted for previous functions ?
Because function parameters should be aligned: foo (bar, baz, quz); That is, the correct indentation depends on the function name. Unless you make sure to copy and paste a function whose name has the exact same length as the new one, you will mess up the indentation.
Created attachment 358000 [details] [review] Update 2 of suggestion of code Ok my mistake. This update 2 seems to be the good one :)
Review of attachment 358000 [details] [review]: lgtm.
It didn't compile (the x11 function was never used), and the commit message was not formatted correctly (very long lines). Fixed these two things before pushing.
Thank you very much !
I'm on ubuntu 17.10 (gnome 3.26) and the setting doesn't seem to work for X11. It work on wayland.
Created attachment 366585 [details] [review] input-settings/x11: Fix tap-and-drag libinput property name It's "libinput Tapping Drag Enabled", not "libinput TappingDrag Enabled".
Review of attachment 366585 [details] [review]: LGTM
Comment on attachment 366585 [details] [review] input-settings/x11: Fix tap-and-drag libinput property name Attachment 366585 [details] pushed as 01e27a4 - input-settings/x11: Fix tap-and-drag libinput property name