Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update xrdp-neutrinordp.c to support side button (forward/back) click #2860

Merged

Conversation

naruhito
Copy link
Contributor

Hello, thank you for the NeutrinoRDP proxy module feature of xrdp.

While using the feature, we found that so-called forward/back button won't work, and in this PR, we would like you to consider merging this patch to solve the issue. @matt335672

forard-back

What are the buttons on the side of my mouse called?

@metalefty
Copy link
Member

Just for reference, similar fix for the VNC module: #2426

@metalefty
Copy link
Member

I don't think sending the mouse button as a keyboard event is a good way to implement this.

Is there any reason not to send mouse event as mouse event as-is?

@matt335672
Copy link
Member

I don't have suitable mouse to look at this, but agreed with @metalefty. These should be send as mouse events somehow, if this is possible.

The event numbers you're using (115-118) correspond to the TS_POINTERX_EVENT in [MS-RDPBCGR] 2.2.8.1.1.3.1.1.4. On the input side, we map these events from the input PDU in xrdp_wm_process_input_mousex() if you want to look at that code.

This suggests to me the code you want should look more like this:-

--- a/neutrinordp/xrdp-neutrinordp.c
+++ b/neutrinordp/xrdp-neutrinordp.c
@@ -423,6 +423,26 @@ lxrdp_event(struct mod *mod, int msg, long param1, long param2,
         case 110:
             break;
 
+        case WM_BUTTON8UP:
+            flags = PTR_XFLAGS_BUTTON1;
+            mod->inst->input->ExtendedMouseEvent(mod->inst->input, flags, 0, 0);
+            break;
+
+        case WM_BUTTON8DOWN:
+            flags = PTR_XFLAGS_BUTTON1 | PTR_XFLAGS_DOWN;
+            mod->inst->input->ExtendedMouseEvent(mod->inst->input, flags, 0, 0);
+            break;
+
+        case WM_BUTTON9UP:
+            flags = PTR_XFLAGS_BUTTON2;
+            mod->inst->input->ExtendedMouseEvent(mod->inst->input, flags, 0, 0);
+            break;
+
+        case WM_BUTTON9DOWN:
+            flags = PTR_XFLAGS_BUTTON2 | PTR_XFLAGS_DOWN;
+            mod->inst->input->ExtendedMouseEvent(mod->inst->input, flags, 0, 0);
+            break;
+
         case 200:
             LOG_DEVEL(LOG_LEVEL_DEBUG, "Invalidate request sent from client");
             x = (param1 >> 16) & 0xffff;

I can compile this but I can't test it. Can you try it?

@naruhito
Copy link
Contributor Author

I agree with you both; yes, mouse events should be treated as they are if possible. The code I made was a quick fix when I was asked by colleagues to address the issue.
I will talk to him and try your code. Please allow me some time, and thanks a lot for the review and advice. @metalefty @matt335672

naruhito pushed a commit to naruhito/xrdp that referenced this pull request Nov 26, 2023
@naruhito
Copy link
Contributor Author

Hello @matt335672 @metalefty I tried the code using MX Ergo mouse device from my colleague.

The code almost worked as-is, except for that param1(=x) and param2(=y) were also needed to be passed, as forward/back works only when mouse pointer is over the window of the Web browser.

Because the commit history has become a little bit messy, I will create another PR if needed.

Also, in this PR, 115-118 magic numbers are used, following the other message types.

Constifying the existing magic numbers were a bit hard for me, as some of them are not in the xrdp_constants.h#L236-L258.

@matt335672
Copy link
Member

@naruhito - thanks for that. The magic numbers are fine for now - we can sort that out later. I'm happy to merge this.

Regarding the commit history, you could create another PR, or you can squash the 4 commits into one, and we can merge that one.

Are you comfortable with squashing? I can give you a hand if you want to learn about it.

@naruhito naruhito force-pushed the neutrinordp-proxy-sidebutton-click branch from 2da892c to ca049dc Compare November 27, 2023 11:56
@naruhito
Copy link
Contributor Author

I've now force-pushed one squashed commit (, was that what you meant?) Thank you again for the advice. @matt335672

For my note:

git rebase -i neutrinolabs/v0.9
pick/squash/squash
git push origin neutrinordp-proxy-sidebutton-click -f

Copy link
Member

@metalefty metalefty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Let's backport this to v0.9.

@matt335672
Copy link
Member

@naruhito has made this request against v0.9

I'll merge this and handle the devel branch merge too.

@naruhito - thanks for raising this and testing it too

@matt335672 matt335672 merged commit 111ee54 into neutrinolabs:v0.9 Nov 27, 2023
13 checks passed
matt335672 pushed a commit to matt335672/xrdp that referenced this pull request Nov 27, 2023
seflerZ pushed a commit to seflerZ/xrdp that referenced this pull request May 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants