You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@guacamole.apache.org by GitBox <gi...@apache.org> on 2020/06/26 05:00:12 UTC
[GitHub] [guacamole-server] gpittarelli opened a new pull request #295: GUACAMOLE-1113: Add support for right modifier keys to SSH/Telnet
gpittarelli opened a new pull request #295:
URL: https://github.com/apache/guacamole-server/pull/295
Fixes RCtrl and AltGr being silently lost in terminal connections.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-server] mike-jumper commented on a change in pull request #295: GUACAMOLE-1113: Add support for right modifier keys to SSH/Telnet
Posted by GitBox <gi...@apache.org>.
mike-jumper commented on a change in pull request #295:
URL: https://github.com/apache/guacamole-server/pull/295#discussion_r445981210
##########
File path: src/terminal/terminal.c
##########
@@ -1473,9 +1473,9 @@ static int __guac_terminal_send_key(guac_terminal* term, int keysym, int pressed
}
/* Track modifiers */
- if (keysym == 0xFFE3)
+ if (keysym == 0xFFE3 /* LCtrl */ || keysym == 0xFFE4 /* RCtrl */)
term->mod_ctrl = pressed;
- else if (keysym == 0xFFE9)
+ else if (keysym == 0xFFE9 /* LAlt */ || keysym == 0xFE03 /* AltGr */)
Review comment:
Care will need to be taken here, and there may need to be additional changes elsewhere for this to be safe. Currently, this will have unintended side effects for characters that require AltGr.
`0xFFEA` (Right Alt) should probably also be handled.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-server] mike-jumper commented on a change in pull request #295: GUACAMOLE-1113: Add support for right modifier keys to SSH/Telnet
Posted by GitBox <gi...@apache.org>.
mike-jumper commented on a change in pull request #295:
URL: https://github.com/apache/guacamole-server/pull/295#discussion_r445981118
##########
File path: src/terminal/terminal.c
##########
@@ -1473,9 +1473,9 @@ static int __guac_terminal_send_key(guac_terminal* term, int keysym, int pressed
}
/* Track modifiers */
- if (keysym == 0xFFE3)
+ if (keysym == 0xFFE3 /* LCtrl */ || keysym == 0xFFE4 /* RCtrl */)
term->mod_ctrl = pressed;
- else if (keysym == 0xFFE9)
+ else if (keysym == 0xFFE9 /* LAlt */ || keysym == 0xFE03 /* AltGr */)
term->mod_alt = pressed;
Review comment:
This will generally work, however the state of `mod_ctrl` and `mod_alt` will be arguably incorrect if the both the Left and Right versions of a modifier are held and then one is released.
##########
File path: src/terminal/terminal.c
##########
@@ -1473,9 +1473,9 @@ static int __guac_terminal_send_key(guac_terminal* term, int keysym, int pressed
}
/* Track modifiers */
- if (keysym == 0xFFE3)
+ if (keysym == 0xFFE3 /* LCtrl */ || keysym == 0xFFE4 /* RCtrl */)
term->mod_ctrl = pressed;
- else if (keysym == 0xFFE9)
+ else if (keysym == 0xFFE9 /* LAlt */ || keysym == 0xFE03 /* AltGr */)
term->mod_alt = pressed;
else if (keysym == 0xFFE1)
Review comment:
Why not Shift as well?
##########
File path: src/terminal/terminal.c
##########
@@ -1473,9 +1473,9 @@ static int __guac_terminal_send_key(guac_terminal* term, int keysym, int pressed
}
/* Track modifiers */
- if (keysym == 0xFFE3)
+ if (keysym == 0xFFE3 /* LCtrl */ || keysym == 0xFFE4 /* RCtrl */)
term->mod_ctrl = pressed;
- else if (keysym == 0xFFE9)
+ else if (keysym == 0xFFE9 /* LAlt */ || keysym == 0xFE03 /* AltGr */)
Review comment:
`0xFFEA` (Right Alt) should probably also be handled, however care will need to be taken here. This will have unintended side effects for characters that require AltGr.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org