mike-jumper commented on code in PR #1164:
URL: https://github.com/apache/guacamole-client/pull/1164#discussion_r2814089351
##########
guacamole-common-js/src/main/webapp/modules/Keyboard.js:
##########
@@ -1514,13 +1747,13 @@ Guacamole.Keyboard.ModifierState = function() {
* The current state of keyboard modifiers.
*/
Guacamole.Keyboard.ModifierState.fromKeyboardEvent = function(e) {
-
+
var state = new Guacamole.Keyboard.ModifierState();
// Assign states from old flags
state.shift = e.shiftKey;
- state.ctrl = e.getModifierState('AltGraph') || e.ctrlKey;
- state.alt = e.getModifierState('AltGraph') || e.altKey;
+ state.ctrl = e.ctrlKey;
+ state.alt = e.altKey;
Review Comment:
Does removing this impact compatibility with Windows?
##########
guacamole-common-js/src/main/webapp/modules/Keyboard.js:
##########
@@ -1461,6 +1646,33 @@ Guacamole.Keyboard = function Keyboard(element) {
*/
Guacamole.Keyboard._nextID = 0;
+/**
+ * Event types for {@link Guacamole.Keyboard.KeyEvent}.
+ *
+ * @constant
+ * @type {!Object.<string, string>}
+ */
+Guacamole.Keyboard.EventType = {
+ /**
+ * A key has been pressed.
+ *
+ * @type {!string}
+ */
+ KEYDOWN: "keydown",
+ /**
+ * A key press has produced a character.
+ *
+ * @type {!string}
+ */
+ KEYPRESS: "keypress",
+ /**
+ * A key has been released.
+ *
+ * @type {!string}
+ */
+ KEYUP: "keyup"
+};
+
Review Comment:
Is there a need for this? Established code has been using `instanceof` for
the same purpose:
https://github.com/apache/guacamole-client/blob/27c47d6f19206f55be65b88d60e0314ef1059c8a/guacamole-common-js/src/main/webapp/modules/Keyboard.js#L1147
##########
guacamole-common-js/src/main/webapp/modules/Keyboard.js:
##########
@@ -1379,6 +1508,62 @@ Guacamole.Keyboard = function Keyboard(element) {
}, true);
+ /**
+ * Handles non-keyboard events, resynchronizing lock states
+ * based on modifier flags.
+ *
+ * @private
+ * @param {!Event} e
+ * The event to handle.
+ */
+ var handleModifierSync = function handleModifierSync(e) {
+
+ // Only intercept if handler set
+ if (!guac_keyboard.onkeydown && !guac_keyboard.onkeyup) return;
+
+ // Ignore events which have already been handled
+ if (!markEvent(e)) return;
+
+ // Only sync if modifier state is available
+ if (!e.getModifierState) return;
+
+ // Resync lock states based modifier flags
+ var modifierState =
Guacamole.Keyboard.ModifierState.fromKeyboardEvent(e);
+
+ updateToggleModifierState('capsLock', [
+ capsLockKeysym
+ ], {
+ modifiers: modifierState,
+ keysym: null,
+ eventType: e.type
+ });
+
+ updateToggleModifierState('numLock', [
+ numLockKeysym
+ ], {
+ modifiers: modifierState,
+ keysym: null,
+ eventType: e.type
+ });
+
+ updateToggleModifierState('scrollLock', [
+ scrollLockKeysym
+ ], {
+ modifiers: modifierState,
+ keysym: null,
+ eventType: e.type
+ });
+
+ };
+
+ // Resync lock states on mouse/touch events which may reflect lock
flags
+ element.addEventListener("mousedown", handleModifierSync, true);
+ element.addEventListener("mouseup", handleModifierSync, true);
+ element.addEventListener("pointerdown", handleModifierSync, true);
+ element.addEventListener("pointerup", handleModifierSync, true);
+ element.addEventListener("touchstart", handleModifierSync, true);
+ element.addEventListener("touchend", handleModifierSync, true);
Review Comment:
The `element` provided to `listenTo()` is not necessarily the element that
will receive mouse/touch events. Perhaps this should make use of the
`handleMouseEvent()` function proposed by #1155?
--
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]
