m1k1o commented on code in PR #895:
URL: https://github.com/apache/guacamole-client/pull/895#discussion_r1247124198
##########
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:
I am not fully familiar with core design of `Guacamole.Keyboard` since this
is my first contibution, but thanks for explaining to me and I can move my code
inside `eventLog` to match the design principle. I'll submit my changes
throughout the next days.
> capsLockkeyupUnreliable is specific to keyup behavior, not the lack of a
keydown
It could be renamed to `capsLockUnreliable` to match the semantics.
> The adding of a keydown event for each Caps Lock keyup does not actually
test whether such an event has already been processed. The assumption is made
that a keydown is always missing.
That assumption is coming directly from `capsLockkeyupUnreliable`, what
processes custom `keyup` event if there is `keydown` event. Even if there would
be `keyup` event with said quirk enabled, it would not pass through `if
(!markEvent(e)) return;` condition. But I agree, that this logic might be
implicit and not transparent.
> tracking locks similarly to modifiers
I might look into that, but I would prefer to keep this refactoring
separated to a different PR and focus now on fixing the bug according to the
contribution guideline `Avoid commits which cover multiple, distinct goals that
could (and should) be handled separately. `.
--
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]
