Github user mike-jumper commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/261#discussion_r172060530
--- Diff: guacamole-common-js/src/main/webapp/modules/OnScreenKeyboard.js
---
@@ -455,6 +455,17 @@ Guacamole.OnScreenKeyboard = function(layout) {
};
+ /**
+ * Resets the state of this keyboard, releasing all keys, and firing
keyup
+ * events for each released key.
+ */
+ this.reset = function() {
+ for (var keysym in pressed) {
+ var key = getActiveKey(keysym);
--- End diff --
Rather than manually pull the active key, and manually invoke `onkeyup`, it
would be better to invoke `release()`:
https://github.com/apache/guacamole-client/blob/5db2e3cae75a658b33515112a39eb2f5f8eef4a8/guacamole-common-js/src/main/webapp/modules/OnScreenKeyboard.js#L346-L377
The `release()` function takes care of invoking `onkeyup` (if it's defined)
as well as proper updating of `pressed`.
Directly calling `onkeyup` like this is problematic because:
1. The `pressed` structure tracks all currently-pressed keys, but this
function does not update that structure. After manually invoking `onkeyup`, the
`pressed` structure will be out of sync with the actual key state.
2. Usages of `Guacamole.OnScreenKeyboard` are not required to provide a
function `onkeyup`. If that function is omitted, `reset()` as written here will
fail.
---
