mike-jumper commented on code in PR #895:
URL: https://github.com/apache/guacamole-client/pull/895#discussion_r1242850582
##########
guacamole-common-js/src/main/webapp/modules/Keyboard.js:
##########
@@ -1362,6 +1362,15 @@ Guacamole.Keyboard = function Keyboard(element) {
e.preventDefault();
+ // If unreliable caps lock was pressed and event was not marked,
then
+ // we need to pretend that this is a keydown event because we
obviously
+ // did not receive it (issue on macos with chrome)
+ if (e.keyCode == 20 && quirks.capsLockKeyupUnreliable) {
+ eventLog.push(new KeydownEvent(e));
+ interpret_events();
+ return;
+ }
Review Comment:
> If unreliable caps lock was pressed and event was not marked, then we need
to pretend that this is a keydown event because we obviously did not receive it
(issue on macos with chrome)
This doesn't look correct:
1. It's unclear what is meant by "event was not marked", but this doesn't
appear to align with the logic in the `if` that follows.
2. From the description in the PR:
> Chrome on MacOS sends only a single `keydown` event when turning on
caps-lock, and only a single `keyup` event when turning off caps-lock.
That would mean that we should send our own keydown for Caps Lock if we
receive a keyup without a corresponding keydown.
3. The intent of the structure of `eventLog` and these various event types
is to prevent having too much logic directly within the JS event handlers. Such
logic should instead be in the portion of the keyboard code that
retrospectively inspects the contents of `eventLog` to produce its final
interpretation.
As the keyboard already tracks modifier states, attempting to synchronize
them whenever they change externally, I think a better approach would be to:
1. Track and synchronize lock states in the same way.
2. Synchronize lock states when inspecting the lone `KeyUpEvent` event in
the `eventLog`.
--
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]
