necouchman commented on code in PR #621:
URL: https://github.com/apache/guacamole-server/pull/621#discussion_r2462923386
##########
src/protocols/rdp/keyboard.c:
##########
@@ -27,7 +27,6 @@
#include <guacamole/client.h>
#include <guacamole/mem.h>
#include <guacamole/rwlock.h>
-
Review Comment:
This line can stay.
##########
src/protocols/rdp/keyboard.c:
##########
@@ -562,7 +561,229 @@ static void
guac_rdp_keyboard_send_missing_key(guac_rdp_keyboard* keyboard,
guac_client* client = keyboard->client;
guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
+
+ /* If system modifiers (Ctrl/Alt) are held, prefer sending US scancode so
+ * that shortcuts like Ctrl+C/V/A work even in Unicode/failsafe layout. */
+ int ctrl_or_alt_pressed =
+ guac_rdp_keyboard_is_pressed(keyboard, GUAC_RDP_KEYSYM_LCTRL)
+ || guac_rdp_keyboard_is_pressed(keyboard, GUAC_RDP_KEYSYM_RCTRL)
+ || guac_rdp_keyboard_is_pressed(keyboard, GUAC_RDP_KEYSYM_LALT)
+ || guac_rdp_keyboard_is_pressed(keyboard, GUAC_RDP_KEYSYM_RALT);
+
+ /* Normalize Cyrillic letters to Latin equivalents when Ctrl/Alt pressed
+ * so that shortcuts like Ctrl+C, Ctrl+V, etc. work even in Russian layout.
+ */
+ if (ctrl_or_alt_pressed) {
+
+
+/* Treat Ctrl or Alt (but NOT AltGr) as shortcut modifiers */
+int ctrl_pressed =
+ guac_rdp_keyboard_is_pressed(keyboard, GUAC_RDP_KEYSYM_LCTRL)
+ || guac_rdp_keyboard_is_pressed(keyboard, GUAC_RDP_KEYSYM_RCTRL);
+
+int alt_pressed =
+ guac_rdp_keyboard_is_pressed(keyboard, GUAC_RDP_KEYSYM_LALT)
+ || guac_rdp_keyboard_is_pressed(keyboard, GUAC_RDP_KEYSYM_RALT);
+
+/* AltGr appears as Ctrl+Alt together; don’t hijack that. */
+int altgr = ctrl_pressed && alt_pressed;
+
+/* We only force scancodes for real shortcuts: Ctrl-only OR Alt-only. */
+int shortcut_mod = !altgr && (ctrl_pressed || alt_pressed);
+
+if (shortcut_mod) {
+ /* Map both Latin and Russian keysyms for C/V to US scancodes.
+ * US Set 1 scancodes: C=0x2E, V=0x2F.
+ * This makes Ctrl+C / Ctrl+V work in ru-RU and en-US equally.
+ */
+ int sc = 0;
+
+ switch (keysym) {
+ /* --- Ctrl+C --- */
+ case 'c': case 'C':
+ case 0x441: /* Cyrillic small 'с' */
+ case 0x421: /* Cyrillic capital 'С' */
+ sc = 0x2E; /* US 'C' key */
+ break;
+
+ /* --- Ctrl+V --- */
+ case 'v': case 'V':
+ case 0x43C: /* Cyrillic small 'м' */
+ case 0x41C: /* Cyrillic capital 'М' */
+ sc = 0x2F; /* US 'V' key */
+ break;
+
+ default:
+ sc = 0;
+ }
+ if (sc) {
+ guac_client_log(client, GUAC_LOG_DEBUG,
+ "Shortcut: modifiers active, sending US scancode 0x%X for keysym
0x%X",
+ sc, keysym);
+ /* press + release with no extended flags */
+ guac_rdp_send_key_event(rdp_client, sc, 0, 1);
+ guac_rdp_send_key_event(rdp_client, sc, 0, 0);
+ return;
+ }
+}
Review Comment:
Please fix indentation in this section to match with the style maintained
throughout the rest of the code.
##########
src/protocols/rdp/keyboard.c:
##########
@@ -562,7 +561,229 @@ static void
guac_rdp_keyboard_send_missing_key(guac_rdp_keyboard* keyboard,
guac_client* client = keyboard->client;
guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
+
+ /* If system modifiers (Ctrl/Alt) are held, prefer sending US scancode so
+ * that shortcuts like Ctrl+C/V/A work even in Unicode/failsafe layout. */
+ int ctrl_or_alt_pressed =
+ guac_rdp_keyboard_is_pressed(keyboard, GUAC_RDP_KEYSYM_LCTRL)
+ || guac_rdp_keyboard_is_pressed(keyboard, GUAC_RDP_KEYSYM_RCTRL)
+ || guac_rdp_keyboard_is_pressed(keyboard, GUAC_RDP_KEYSYM_LALT)
+ || guac_rdp_keyboard_is_pressed(keyboard, GUAC_RDP_KEYSYM_RALT);
+
+ /* Normalize Cyrillic letters to Latin equivalents when Ctrl/Alt pressed
+ * so that shortcuts like Ctrl+C, Ctrl+V, etc. work even in Russian layout.
+ */
+ if (ctrl_or_alt_pressed) {
+
+
+/* Treat Ctrl or Alt (but NOT AltGr) as shortcut modifiers */
+int ctrl_pressed =
+ guac_rdp_keyboard_is_pressed(keyboard, GUAC_RDP_KEYSYM_LCTRL)
+ || guac_rdp_keyboard_is_pressed(keyboard, GUAC_RDP_KEYSYM_RCTRL);
+
+int alt_pressed =
+ guac_rdp_keyboard_is_pressed(keyboard, GUAC_RDP_KEYSYM_LALT)
+ || guac_rdp_keyboard_is_pressed(keyboard, GUAC_RDP_KEYSYM_RALT);
+
+/* AltGr appears as Ctrl+Alt together; don’t hijack that. */
+int altgr = ctrl_pressed && alt_pressed;
+
+/* We only force scancodes for real shortcuts: Ctrl-only OR Alt-only. */
+int shortcut_mod = !altgr && (ctrl_pressed || alt_pressed);
+
+if (shortcut_mod) {
+ /* Map both Latin and Russian keysyms for C/V to US scancodes.
+ * US Set 1 scancodes: C=0x2E, V=0x2F.
+ * This makes Ctrl+C / Ctrl+V work in ru-RU and en-US equally.
+ */
+ int sc = 0;
+
+ switch (keysym) {
+ /* --- Ctrl+C --- */
+ case 'c': case 'C':
Review Comment:
These should be on separate lines.
##########
src/protocols/rdp/keyboard.c:
##########
@@ -562,7 +561,229 @@ static void
guac_rdp_keyboard_send_missing_key(guac_rdp_keyboard* keyboard,
guac_client* client = keyboard->client;
guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
+
+ /* If system modifiers (Ctrl/Alt) are held, prefer sending US scancode so
+ * that shortcuts like Ctrl+C/V/A work even in Unicode/failsafe layout. */
+ int ctrl_or_alt_pressed =
+ guac_rdp_keyboard_is_pressed(keyboard, GUAC_RDP_KEYSYM_LCTRL)
+ || guac_rdp_keyboard_is_pressed(keyboard, GUAC_RDP_KEYSYM_RCTRL)
+ || guac_rdp_keyboard_is_pressed(keyboard, GUAC_RDP_KEYSYM_LALT)
+ || guac_rdp_keyboard_is_pressed(keyboard, GUAC_RDP_KEYSYM_RALT);
+
+ /* Normalize Cyrillic letters to Latin equivalents when Ctrl/Alt pressed
+ * so that shortcuts like Ctrl+C, Ctrl+V, etc. work even in Russian layout.
+ */
+ if (ctrl_or_alt_pressed) {
+
+
+/* Treat Ctrl or Alt (but NOT AltGr) as shortcut modifiers */
+int ctrl_pressed =
+ guac_rdp_keyboard_is_pressed(keyboard, GUAC_RDP_KEYSYM_LCTRL)
+ || guac_rdp_keyboard_is_pressed(keyboard, GUAC_RDP_KEYSYM_RCTRL);
+
+int alt_pressed =
+ guac_rdp_keyboard_is_pressed(keyboard, GUAC_RDP_KEYSYM_LALT)
+ || guac_rdp_keyboard_is_pressed(keyboard, GUAC_RDP_KEYSYM_RALT);
+
+/* AltGr appears as Ctrl+Alt together; don’t hijack that. */
+int altgr = ctrl_pressed && alt_pressed;
+
+/* We only force scancodes for real shortcuts: Ctrl-only OR Alt-only. */
+int shortcut_mod = !altgr && (ctrl_pressed || alt_pressed);
+
+if (shortcut_mod) {
+ /* Map both Latin and Russian keysyms for C/V to US scancodes.
+ * US Set 1 scancodes: C=0x2E, V=0x2F.
+ * This makes Ctrl+C / Ctrl+V work in ru-RU and en-US equally.
+ */
+ int sc = 0;
+
+ switch (keysym) {
+ /* --- Ctrl+C --- */
+ case 'c': case 'C':
+ case 0x441: /* Cyrillic small 'с' */
+ case 0x421: /* Cyrillic capital 'С' */
+ sc = 0x2E; /* US 'C' key */
+ break;
+
+ /* --- Ctrl+V --- */
+ case 'v': case 'V':
Review Comment:
These should be on separate lines.
##########
src/protocols/rdp/keyboard.c:
##########
@@ -562,7 +561,229 @@ static void
guac_rdp_keyboard_send_missing_key(guac_rdp_keyboard* keyboard,
guac_client* client = keyboard->client;
guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
+
+ /* If system modifiers (Ctrl/Alt) are held, prefer sending US scancode so
+ * that shortcuts like Ctrl+C/V/A work even in Unicode/failsafe layout. */
+ int ctrl_or_alt_pressed =
+ guac_rdp_keyboard_is_pressed(keyboard, GUAC_RDP_KEYSYM_LCTRL)
+ || guac_rdp_keyboard_is_pressed(keyboard, GUAC_RDP_KEYSYM_RCTRL)
+ || guac_rdp_keyboard_is_pressed(keyboard, GUAC_RDP_KEYSYM_LALT)
+ || guac_rdp_keyboard_is_pressed(keyboard, GUAC_RDP_KEYSYM_RALT);
+
+ /* Normalize Cyrillic letters to Latin equivalents when Ctrl/Alt pressed
+ * so that shortcuts like Ctrl+C, Ctrl+V, etc. work even in Russian layout.
+ */
+ if (ctrl_or_alt_pressed) {
+
+
+/* Treat Ctrl or Alt (but NOT AltGr) as shortcut modifiers */
+int ctrl_pressed =
+ guac_rdp_keyboard_is_pressed(keyboard, GUAC_RDP_KEYSYM_LCTRL)
+ || guac_rdp_keyboard_is_pressed(keyboard, GUAC_RDP_KEYSYM_RCTRL);
+
+int alt_pressed =
+ guac_rdp_keyboard_is_pressed(keyboard, GUAC_RDP_KEYSYM_LALT)
+ || guac_rdp_keyboard_is_pressed(keyboard, GUAC_RDP_KEYSYM_RALT);
+
+/* AltGr appears as Ctrl+Alt together; don’t hijack that. */
+int altgr = ctrl_pressed && alt_pressed;
+
+/* We only force scancodes for real shortcuts: Ctrl-only OR Alt-only. */
+int shortcut_mod = !altgr && (ctrl_pressed || alt_pressed);
+
+if (shortcut_mod) {
+ /* Map both Latin and Russian keysyms for C/V to US scancodes.
+ * US Set 1 scancodes: C=0x2E, V=0x2F.
+ * This makes Ctrl+C / Ctrl+V work in ru-RU and en-US equally.
+ */
+ int sc = 0;
+
+ switch (keysym) {
+ /* --- Ctrl+C --- */
+ case 'c': case 'C':
+ case 0x441: /* Cyrillic small 'с' */
+ case 0x421: /* Cyrillic capital 'С' */
+ sc = 0x2E; /* US 'C' key */
+ break;
+
+ /* --- Ctrl+V --- */
+ case 'v': case 'V':
+ case 0x43C: /* Cyrillic small 'м' */
+ case 0x41C: /* Cyrillic capital 'М' */
+ sc = 0x2F; /* US 'V' key */
+ break;
+
+ default:
+ sc = 0;
+ }
+ if (sc) {
+ guac_client_log(client, GUAC_LOG_DEBUG,
+ "Shortcut: modifiers active, sending US scancode 0x%X for keysym
0x%X",
+ sc, keysym);
+ /* press + release with no extended flags */
+ guac_rdp_send_key_event(rdp_client, sc, 0, 1);
+ guac_rdp_send_key_event(rdp_client, sc, 0, 0);
+ return;
+ }
+}
+ switch (keysym) {
+ /* Lowercase Cyrillic */
+ case 0x430: keysym = 'f'; break; // а
+ case 0x431: keysym = ','; break; // б
+ case 0x432: keysym = 'd'; break; // в
+ case 0x433: keysym = 'u'; break; // г
+ case 0x434: keysym = 'l'; break; // д
+ case 0x435: keysym = 't'; break; // е
+ case 0x436: keysym = ';'; break; // ж
+ case 0x437: keysym = 'p'; break; // з
+ case 0x438: keysym = 'b'; break; // и
+ case 0x439: keysym = 'q'; break; // й
+ case 0x43A: keysym = 'r'; break; // к
+ case 0x43B: keysym = 'k'; break; // л
+ case 0x43C: keysym = 'v'; break; // м
+ case 0x43D: keysym = 'y'; break; // н
+ case 0x43E: keysym = 'j'; break; // о
+ case 0x43F: keysym = 'g'; break; // п
+ case 0x440: keysym = 'h'; break; // р
+ case 0x441: keysym = 'c'; break; // с
+ case 0x442: keysym = 'n'; break; // т
+ case 0x443: keysym = 'e'; break; // у
+ case 0x444: keysym = 'a'; break; // ф
+ case 0x445: keysym = '['; break; // х
+ case 0x446: keysym = 'w'; break; // ц
+ case 0x447: keysym = 'x'; break; // ч
+ case 0x448: keysym = 'i'; break; // ш
+ case 0x449: keysym = 'o'; break; // щ
+ case 0x44A: keysym = ']'; break; // ъ
+ case 0x44B: keysym = 's'; break; // ы
+ case 0x44C: keysym = 'm'; break; // ь
+ case 0x44D: keysym = '\''; break; // э
+ case 0x44E: keysym = '.'; break; // ю
+ case 0x44F: keysym = 'z'; break; // я
+
+ /* Uppercase Cyrillic */
+ case 0x410: keysym = 'F'; break; // А
+ case 0x411: keysym = '<'; break; // Б
+ case 0x412: keysym = 'D'; break; // В
+ case 0x413: keysym = 'U'; break; // Г
+ case 0x414: keysym = 'L'; break; // Д
+ case 0x415: keysym = 'T'; break; // Е
+ case 0x416: keysym = ':'; break; // Ж
+ case 0x417: keysym = 'P'; break; // З
+ case 0x418: keysym = 'B'; break; // И
+ case 0x419: keysym = 'Q'; break; // Й
+ case 0x41A: keysym = 'R'; break; // К
+ case 0x41B: keysym = 'K'; break; // Л
+ case 0x41C: keysym = 'V'; break; // М
+ case 0x41D: keysym = 'Y'; break; // Н
+ case 0x41E: keysym = 'J'; break; // О
+ case 0x41F: keysym = 'G'; break; // П
+ case 0x420: keysym = 'H'; break; // Р
+ case 0x421: keysym = 'C'; break; // С
+ case 0x422: keysym = 'N'; break; // Т
+ case 0x423: keysym = 'E'; break; // У
+ case 0x424: keysym = 'A'; break; // Ф
+ case 0x425: keysym = '{'; break; // Х
+ case 0x426: keysym = 'W'; break; // Ц
+ case 0x427: keysym = 'X'; break; // Ч
+ case 0x428: keysym = 'I'; break; // Ш
+ case 0x429: keysym = 'O'; break; // Щ
+ case 0x42A: keysym = '}'; break; // Ъ
+ case 0x42B: keysym = 'S'; break; // Ы
+ case 0x42C: keysym = 'M'; break; // Ь
+ case 0x42D: keysym = '"'; break; // Э
+ case 0x42E: keysym = '>'; break; // Ю
+ case 0x42F: keysym = 'Z'; break; // Я
+ }
+ }
Review Comment:
A few issues, here:
* I'm not sure about maintaining keymap translations directly in the code
like this - this is one of the things that I believe the individual keymaps are
supposed to handle. Is there some reason this has to be done in the code?
* Is there any way to do this without the `switch` statement, like maybe
some math that takes in the values for the `case` statements and automatically
converts it to the expected keysym?
* Please match style as maintained throughout the rest of the code - putting
the entirety of each `case` statement on a single line makes for more difficult
readability.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
