Bug 465463 - For some extra buttons kwin emits key events set for another extra button
Summary: For some extra buttons kwin emits key events set for another extra button
Status: RESOLVED FIXED
Alias: None
Product: kwin
Classification: Plasma
Component: wayland-generic (other bugs)
Version First Reported In: 5.26.90
Platform: Exherbo Linux
: NOR normal
Target Milestone: ---
Assignee: KWin default assignee
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-02-08 08:55 UTC by Bernd Steinhauser
Modified: 2023-02-10 12:41 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed/Implemented In: 5.27.1
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Bernd Steinhauser 2023-02-08 08:55:28 UTC
Originally, I mentioned this issue in bug 464735, but I'm now pretty sure that it's a separate issue and especially is independent from the keyboard layout (i.e. also happens with the standard variant for de or us layout).

SUMMARY
Extra buttons can be configured in the kcm individually, but the key events emitted can be the same as set for another button.
This might suggesting that there is something wrong with the button numbering?

STEPS TO REPRODUCE
1. Use a Mouse with at least(?) 3 extra mouse buttons (e.g. forward, back, side/extra)
2. Assign keys or key sequences to each one of them 
3. Use wev to test whether the emitted keys match the setting

OBSERVED RESULT
Extra button 3 will always emit the key set for extra button 2, even if no action is set for extra button 3
If extra button 2 is not set, both with report their individual default function, even if an action is set for extra button 3.

EXPECTED RESULT
The set key events should be emitted.

SOFTWARE/OS VERSIONS
Linux/KDE Plasma: 5.26.90 (tested also on 5.26.4)
KDE Plasma Version: 5.26.90
KDE Frameworks Version: 5.102.0
Qt Version: 5.15.8

ADDITIONAL INFORMATION
I tested this on both Exherbo Linux and Arch Linux with the latest beta. I tested two mice: Logitech M705 and Logitech Anywhere 2S.
Both show this behavior. I don't have any mice available right now from other vendors to cross-check it there.
Comment 1 David Redondo 2023-02-09 08:32:41 UTC
Sounds like our filter somehow mixes up extra button 2 and 3. Could  you paste  the 
'[ButtonRebinds][Mouse]' section of ~/.config/kcminputrc just to be sure that it writes the correct config for button 2 and 3?
Comment 2 Bernd Steinhauser 2023-02-09 09:04:29 UTC
(In reply to David Redondo from comment #1)
> Sounds like our filter somehow mixes up extra button 2 and 3. Could  you
> paste  the 
> '[ButtonRebinds][Mouse]' section of ~/.config/kcminputrc just to be sure
> that it writes the correct config for button 2 and 3?

Sure:
[ButtonRebinds][Mouse]
ExtraButton1=Key,A
ExtraButton2=Key,B
ExtraButton3=Key,C
ExtraButton4=Key,D
ExtraButton5=Key,E

(I reassigned them to A-E before doing this, as this makes it easier to test follow the effects)
So the KCM is doing the right thing. But it's not just Button 3, the whole thing is messed up.
Here is what happens when I use wev with this setup:
Button 1: a
Button 2: b
Button 3: b
Button 4: b
Button 5: c

(Button 4 and 5 correspond to the left/right action on the scrolling wheel. I can map those to regular buttons instead of horizontal scrolling using ratbagd or solaar.)

So something is wrong with the way it increments, maybe?
There is also a button 13, which is not really a button, but the (mechanical) switch that activates/deactivates clickless scrolling.
I can set that, too and it will correctly output the key that I set for this button. So maybe just the increment isn't the answer either?
Comment 3 Bug Janitor Service 2023-02-10 08:53:24 UTC
A possibly relevant merge request was started @ https://invent.kde.org/plasma/kwin/-/merge_requests/3575
Comment 4 Vlad Zahorodnii 2023-02-10 09:41:07 UTC
Git commit cc4d99aea4948c10c159328ff15f6783ae974eeb by Vlad Zahorodnii, on behalf of David Redondo.
Committed on 10/02/2023 at 09:27.
Pushed by davidre into branch 'master'.

Fix button to Qt::MouseButton mapping

Even though the names seem to match, QtWayland maps button values
to enum values in ascendung order (as it does on X11). The wrong
mapping is usually not a problem because we send the native button
events to clients. However when the Qt names or values are used
for communication between KWin and a client this leads to
misunderstandings.
FIXED-IN:5.27.1

M  +4    -4    autotests/libinput/mock_libinput.cpp
M  +3    -5    src/mousebuttons.cpp

https://invent.kde.org/plasma/kwin/commit/cc4d99aea4948c10c159328ff15f6783ae974eeb
Comment 5 David Redondo 2023-02-10 09:42:32 UTC
Git commit e0b289a6f1a665c77b1112049b3c70536770f27f by David Redondo.
Committed on 10/02/2023 at 09:42.
Pushed by davidre into branch 'cherry-pick-cc4d99ae'.

Fix button to Qt::MouseButton mapping

Even though the names seem to match, QtWayland maps button values
to enum values in ascendung order (as it does on X11). The wrong
mapping is usually not a problem because we send the native button
events to clients. However when the Qt names or values are used
for communication between KWin and a client this leads to
misunderstandings.
FIXED-IN:5.27.1


(cherry picked from commit cc4d99aea4948c10c159328ff15f6783ae974eeb)

M  +4    -4    autotests/libinput/mock_libinput.cpp
M  +3    -5    src/mousebuttons.cpp

https://invent.kde.org/plasma/kwin/commit/e0b289a6f1a665c77b1112049b3c70536770f27f
Comment 6 Bernd Steinhauser 2023-02-10 10:29:10 UTC
Thanks for looking into this. I can confirm that the change fixes the issue.
Comment 7 David Redondo 2023-02-10 12:21:33 UTC
Thank you, for the the very detailed report!
Comment 8 Bernd Steinhauser 2023-02-10 12:41:19 UTC
(In reply to David Redondo from comment #7)
> Thank you, for the the very detailed report!

Well, directly after the whole kwin restarting issue, the ability to rebind mouse buttons was the major holdback for me personally regarding the switch to Wayland.
My muscle memory is just way too trained using these after +10 years of using imwheel. So the motivation to dig deep to analyze this was quite high for me. :)