jitsi-meet
jitsi-meet copied to clipboard
Fix Keyboard Shortcuts clashing with whiteboard
Add canvas to blacklisted elements Add return if ctrl modifier is used on the keypress.
- none of jitsis shortcuts use ctrl - so we should ignore those
Currently shortcuts like CTRL+C and CTRL + V and other system shortcuts will trigger jitsis shortcut handler, even if not intended.
The guard check in this commit will prevent interference with system shortcuts by ignoring events with ctrl completely.
Hi, thanks for your contribution! If you haven't already done so, could you please make sure you sign our CLA (https://jitsi.org/icla for individuals and https://jitsi.org/ccla for corporations)? We would unfortunately be unable to merge your patch unless we have that piece :(.
The Second commit also adds the metaKey property to the check.
shortcuts with the windows-key or the mac cmd-key should also be ignored.
LGTM at a glance. @mihhu can you PTAL?
I'm not sure why the metaKey boolean doesn't work in this case, I tested it separately and my Chrome detects it correctly. Could you please add an extra check for the left and right command button? Maybe just checking for the key to be "Meta" would be enough.
I'm not sure why the metaKey boolean doesn't work in this case, I tested it separately and my Chrome detects it correctly. Could you please add an extra check for the left and right command button? Maybe just checking for the key to be "Meta" would be enough.
I was relying on Mozilla MDN for that specific code, thus was unable to verify its functionality.
@mihhu did you try an actual shortcut with command ?
From the screenshot it seems that you pressed the command button, which from my perspective looks like intended behavior.
The boolean should be true, if the command key was already pressed before the next key event occurs.
Which means, that the first command key Event which will trigger the onKeyDown will not have the boolean set, but while holding it - crating another keyDown or KeyUo event ( CMD + c ) for example, should show code "c" in with the meta boolean on true.
Could you please verify if my assumption is correct ? ( due to my lack of apple devices. - is it possible to emulate this apple behaviour myself inside the browser - I'll check and add this info here. )
@mihhu did you try an actual shortcut with command ?
Yes, cmd+A, cmd+C, cmd+V. It triggered the Jitsi shortcut, so my guess is it didn't return as it should've. I'll try to look into it too.
I did an extensive test on some apple devices: Macbook and Mac-Mini - Both had the same results (I tested both devices with chrome and safari each):
I was unable to trigger Jitsi Shortcuts while holding the cmd key, but for a different reason than my code working.
TLDR: In my tests the boolean worked more or less intentionally as it is supposed to. There was no keyup event for any letter-key whenever a key combination included the cmd-key (e.g. CTRL + CMD + R )
Long Version:
I tested multiple key command combinations to figure out what happens inside the keyevents at specific times.
Those include - Keydown and Keyup events each:
- Single Letters ( a, c, v)
- Single Modifier Keys ( Ctrl, Meta, Alt, Shift) (including different keyboard sites, e.g. Ctrl-right, meta-right,... )
- Simple Combinations ( Ctrl + a , ... )
- Advanced Combinations ( Ctrl + Meta + C , ...)
key-combination | keydown | keyup |
---|---|---|
letter | :heavy_check_mark: | :heavy_check_mark: |
single modifier | :heavy_check_mark: | :heavy_check_mark: |
single combinations | as intended - Meta Key Boolean is true if meta key is pressed | :no_entry: never happend - probably system side blocked |
advanced combinations | as intended - Meta Key Boolean is true if meta key is pressed | :no_entry: never happend - probably system side blocked |
Findings
- I was able to copy and paste from/to the whiteboard on the tested browsers, even without the added modification to the code above
- the keyup event never happend when the cmd key was involved. - the keydown event happened as intended. ( this is contradicting to your test. - did you perhaps use firefox for the test ? - eventually its also a os mismatch which causes the difference in behaviour)
- the keydown events do include the correct information
Result Questions:
While it may be possible to store cmd key being pressed in a variable to fix this, i'd personally not want to do that, as it might be too much of a code change for a relatively simple issue.
Alternatively we may be able to change the keyup to a keydown and filter out repeating keyevents with the e.repeat boolean ? - Which would result in a working key-event with working ctrl, meta, and other modifier booleans set.
This PR has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
@mihhu what's the state of this?
This PR has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
Ping @mihhu
This PR has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.