From 035813ff0b8938b5fa999b017781a0548e7b710d Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Tue, 2 Oct 2018 22:18:17 -0700 Subject: [PATCH 1/5] GUACAMOLE-232: Take best guess of key being pressed/released into account before assuming modifier states need to be resynced. --- .../src/main/webapp/modules/Keyboard.js | 59 ++++++++++++++----- 1 file changed, 44 insertions(+), 15 deletions(-) diff --git a/guacamole-common-js/src/main/webapp/modules/Keyboard.js b/guacamole-common-js/src/main/webapp/modules/Keyboard.js index 022b21618..f9706512e 100644 --- a/guacamole-common-js/src/main/webapp/modules/Keyboard.js +++ b/guacamole-common-js/src/main/webapp/modules/Keyboard.js @@ -918,20 +918,45 @@ Guacamole.Keyboard = function Keyboard(element) { * * @param {Number[]} keysyms * The keysyms which represent the key being updated. + * + * @param {KeyEvent} keyEvent + * Guacamole's current best interpretation of the key event being + * processed. */ - var updateModifierState = function updateModifierState(remoteState, localState, keysyms) { + var updateModifierState = function updateModifierState(remoteState, + localState, keysyms, keyEvent) { + + var i; + + // Do not trust changes in modifier state for events directly involving + // that modifier: (1) the flag may erroneously be cleared despite + // another version of the same key still being held and (2) the change + // in flag may be due to the current event being processed, thus + // updating things here is at best redundant and at worst incorrect + if (keysyms.indexOf(keyEvent.keysym) !== -1) + return; // Release all related keys if modifier is implicitly released if (remoteState && localState === false) { - for (var i = 0; i < keysyms.length; i++) { + for (i = 0; i < keysyms.length; i++) { guac_keyboard.release(keysyms[i]); } } // Press if modifier is implicitly pressed - else if (!remoteState && localState) + else if (!remoteState && localState) { + + // Verify that modifier flag isn't set due to another version of + // the same key being held down + for (i = 0; i < keysyms.length; i++) { + if (guac_keyboard.pressed[keysyms[i]]) + return; + } + guac_keyboard.press(keysyms[0]); + } + }; /** @@ -942,8 +967,12 @@ Guacamole.Keyboard = function Keyboard(element) { * @private * @param {KeyboardEvent} e * The keyboard event containing the flags to update. + * + * @param {KeyEvent} keyEvent + * Guacamole's current best interpretation of the key event being + * processed. */ - var syncModifierStates = function syncModifierStates(e) { + var syncModifierStates = function syncModifierStates(e, keyEvent) { // Get state var state = Guacamole.Keyboard.ModifierState.fromKeyboardEvent(e); @@ -953,31 +982,31 @@ Guacamole.Keyboard = function Keyboard(element) { 0xFFE9, // Left alt 0xFFEA, // Right alt 0xFE03 // AltGr - ]); + ], keyEvent); // Resync state of shift updateModifierState(guac_keyboard.modifiers.shift, state.shift, [ 0xFFE1, // Left shift 0xFFE2 // Right shift - ]); + ], keyEvent); // Resync state of ctrl updateModifierState(guac_keyboard.modifiers.ctrl, state.ctrl, [ 0xFFE3, // Left ctrl 0xFFE4 // Right ctrl - ]); + ], keyEvent); // Resync state of meta updateModifierState(guac_keyboard.modifiers.meta, state.meta, [ 0xFFE7, // Left meta 0xFFE8 // Right meta - ]); + ], keyEvent); // Resync state of hyper updateModifierState(guac_keyboard.modifiers.hyper, state.hyper, [ 0xFFEB, // Left hyper 0xFFEC // Right hyper - ]); + ], keyEvent); // Update state guac_keyboard.modifiers = state; @@ -1222,7 +1251,8 @@ Guacamole.Keyboard = function Keyboard(element) { else if (e.which) keyCode = e.which; // Fix modifier states - syncModifierStates(e); + var keydownEvent = new KeydownEvent(keyCode, e.keyIdentifier, e.key, getEventLocation(e)); + syncModifierStates(e, keydownEvent); // Ignore (but do not prevent) the "composition" keycode sent by some // browsers when an IME is in use (see: http://lists.w3.org/Archives/Public/www-dom/2010JulSep/att-0182/keyCode-spec.html) @@ -1230,7 +1260,6 @@ Guacamole.Keyboard = function Keyboard(element) { return; // Log event - var keydownEvent = new KeydownEvent(keyCode, e.keyIdentifier, e.key, getEventLocation(e)); eventLog.push(keydownEvent); // Interpret as many events as possible, prevent default if indicated @@ -1253,10 +1282,10 @@ Guacamole.Keyboard = function Keyboard(element) { else if (e.which) charCode = e.which; // Fix modifier states - syncModifierStates(e); + var keypressEvent = new KeypressEvent(charCode); + syncModifierStates(e, keypressEvent); // Log event - var keypressEvent = new KeypressEvent(charCode); eventLog.push(keypressEvent); // Interpret as many events as possible, prevent default if indicated @@ -1281,10 +1310,10 @@ Guacamole.Keyboard = function Keyboard(element) { else if (e.which) keyCode = e.which; // Fix modifier states - syncModifierStates(e); + var keyupEvent = new KeyupEvent(keyCode, e.keyIdentifier, e.key, getEventLocation(e)); + syncModifierStates(e, keyupEvent); // Log event, call for interpretation - var keyupEvent = new KeyupEvent(keyCode, e.keyIdentifier, e.key, getEventLocation(e)); eventLog.push(keyupEvent); interpret_events(); From 8c096778bc0235b2059495715fee91c31b719c67 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Tue, 2 Oct 2018 22:19:15 -0700 Subject: [PATCH 2/5] GUACAMOLE-232: Fall back to using recent keysym only after failing to determine released key by keycode. --- guacamole-common-js/src/main/webapp/modules/Keyboard.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/guacamole-common-js/src/main/webapp/modules/Keyboard.js b/guacamole-common-js/src/main/webapp/modules/Keyboard.js index f9706512e..83e7944bd 100644 --- a/guacamole-common-js/src/main/webapp/modules/Keyboard.js +++ b/guacamole-common-js/src/main/webapp/modules/Keyboard.js @@ -386,8 +386,8 @@ Guacamole.Keyboard = function Keyboard(element) { this.location = location; // If key is known from keyCode or DOM3 alone, use that - this.keysym = recentKeysym[keyCode] - || keysym_from_keycode(keyCode, location) + this.keysym = keysym_from_keycode(keyCode, location) + || recentKeysym[keyCode] || keysym_from_key_identifier(key, location); // keyCode is still more reliable for keyup when dead keys are in use // Keyup is as reliable as it will ever be From 6f0787f0c13d187498ae0055c1963fe922832d19 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Tue, 2 Oct 2018 22:19:40 -0700 Subject: [PATCH 3/5] GUACAMOLE-232: Reset tracking of recent keysym after key is released. --- guacamole-common-js/src/main/webapp/modules/Keyboard.js | 1 + 1 file changed, 1 insertion(+) diff --git a/guacamole-common-js/src/main/webapp/modules/Keyboard.js b/guacamole-common-js/src/main/webapp/modules/Keyboard.js index 83e7944bd..a4ebe36c9 100644 --- a/guacamole-common-js/src/main/webapp/modules/Keyboard.js +++ b/guacamole-common-js/src/main/webapp/modules/Keyboard.js @@ -1149,6 +1149,7 @@ Guacamole.Keyboard = function Keyboard(element) { var keysym = first.keysym; if (keysym) { guac_keyboard.release(keysym); + delete recentKeysym[first.keyCode]; first.defaultPrevented = true; } From 2ec7e48ca9a31900237363cc71ee99416774bb37 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Tue, 2 Oct 2018 22:42:58 -0700 Subject: [PATCH 4/5] GUACAMOLE-232: Track whether keys were pressed implicitly. Automatically release all keys if only implicitly pressed keys remain. --- .../src/main/webapp/modules/Keyboard.js | 48 ++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-) diff --git a/guacamole-common-js/src/main/webapp/modules/Keyboard.js b/guacamole-common-js/src/main/webapp/modules/Keyboard.js index a4ebe36c9..8eea4424e 100644 --- a/guacamole-common-js/src/main/webapp/modules/Keyboard.js +++ b/guacamole-common-js/src/main/webapp/modules/Keyboard.js @@ -615,6 +615,19 @@ Guacamole.Keyboard = function Keyboard(element) { */ this.pressed = {}; + /** + * The state of every key, indexed by keysym, for strictly those keys whose + * status has been indirectly determined thorugh observation of other key + * events. If a particular key is implicitly pressed, the value of + * implicitlyPressed for that keysym will be true. If a key + * is not currently implicitly pressed (the key is not pressed OR the state + * of the key is explicitly known), it will not be defined. + * + * @private + * @tyle {Object.} + */ + var implicitlyPressed = {}; + /** * The last result of calling the onkeydown handler for each key, indexed * by keysym. This is used to prevent/allow default actions for key events, @@ -851,6 +864,7 @@ Guacamole.Keyboard = function Keyboard(element) { // Mark key as released delete guac_keyboard.pressed[keysym]; + delete implicitlyPressed[keysym]; // Stop repeat window.clearTimeout(key_repeat_timeout); @@ -953,7 +967,13 @@ Guacamole.Keyboard = function Keyboard(element) { return; } - guac_keyboard.press(keysyms[0]); + // Press key and mark as implicitly pressed (if not already + // explicitly pressed) + var keysym = keysyms[0]; + if (!guac_keyboard.pressed(keysym)) { + implicitlyPressed[keysym] = true; + guac_keyboard.press(keysym); + } } @@ -1013,6 +1033,27 @@ Guacamole.Keyboard = function Keyboard(element) { }; + /** + * Returns whether all currently pressed keys were implicitly pressed. A + * key is implicitly pressed if its status was inferred indirectly from + * inspection of other key events. + * + * @private + * @returns {Boolean} + * true if all currently pressed keys were implicitly pressed, false + * otherwise. + */ + var isStateImplicit = function isStateImplicit() { + + for (var keysym in guac_keyboard.pressed) { + if (!implicitlyPressed[keysym]) + return true; + } + + return false; + + }; + /** * Reads through the event log, removing events from the head of the log * when the corresponding true key presses are known (or as known as they @@ -1036,6 +1077,11 @@ Guacamole.Keyboard = function Keyboard(element) { handled_event = interpret_event(); } while (handled_event !== null); + // Reset keyboard state if we cannot expect to receive any further + // keyup events + if (isStateImplicit()) + guac_keyboard.reset(); + return last_event.defaultPrevented; } From eead01944e8b6c52197d183df3bd3431783c1b02 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Tue, 2 Oct 2018 23:01:10 -0700 Subject: [PATCH 5/5] GUACAMOLE-232: Rely on recentKeysym for keyup only when the guessed key doesn't seem to actually be pressed. --- guacamole-common-js/src/main/webapp/modules/Keyboard.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/guacamole-common-js/src/main/webapp/modules/Keyboard.js b/guacamole-common-js/src/main/webapp/modules/Keyboard.js index 8eea4424e..d19904151 100644 --- a/guacamole-common-js/src/main/webapp/modules/Keyboard.js +++ b/guacamole-common-js/src/main/webapp/modules/Keyboard.js @@ -387,9 +387,13 @@ Guacamole.Keyboard = function Keyboard(element) { // If key is known from keyCode or DOM3 alone, use that this.keysym = keysym_from_keycode(keyCode, location) - || recentKeysym[keyCode] || keysym_from_key_identifier(key, location); // keyCode is still more reliable for keyup when dead keys are in use + // Fall back to the most recently pressed keysym associated with the + // keyCode if the inferred key doesn't seem to actually be pressed + if (!guac_keyboard.pressed[this.keysym]) + this.keysym = recentKeysym[keyCode] || this.keysym; + // Keyup is as reliable as it will ever be this.reliable = true;