selkies-project / selkies-gstreamer

Open-Source Low-Latency Accelerated Linux WebRTC HTML5 Remote Desktop Streaming Platform for Self-Hosting, Containers, Kubernetes, or Cloud/HPC
https://selkies-project.github.io/selkies-gstreamer/
Mozilla Public License 2.0
381 stars 50 forks source link

Long hold ESC to exit full screen in the web interface does not work in certain cases and many key combinations leak to the client OS #107

Closed ehfd closed 5 months ago

ehfd commented 1 year ago

Behavior reproduced in Chromium (Chrome, Edge) and Firefox.

Issue 1:

When the "Enter full screen" button is pressed in the side menu, it is supposed to turn into a mode where a press and hold on ESC is required to exit the full screen.

Expected behavior: image

Actual behavior: image

The expected behavior only happens when Ctrl+Shift+F is pressed in Chromium.

Press and hold ESC to exit does not work in Firefox even if Ctrl+Shift+F is pressed.

Issue 2:

Edit: Turns out the only thing that didn't work when the "Enter full screen" button wasn't the "Press and hold ESC" issue. An additional critical issue is that relative cursors also don't work correctly. It works when using Ctrl+Shift+F, but not when using the "Enter full screen" button in the menu panel.

Issue 3:

Moreover, even when the expected behavior with Ctrl+Shift+F is triggered, certain keyboard button combinations are passed to the client, instead of the remote host.

For example, in touchpads, a three-finger tap should be equal to clicking the middle mouse button. Instead, it triggers the start menu in the Windows client.

I understand that the browser has limitations on what keys can be captured by the browser and which will be sent to the client, but Guacamole's Tomcat HTML5 client supports many more keys to be passed to the remote host than what we do now.

Issue 4: Users don't know that there is a Ctrl+Shift+F and Ctrl+Shift+LtClick shortcut. It would be useful to add information of all shortcuts in the menu.

ehfd commented 1 year ago

This turns out to be an urgent bug, restricting capabilities in a major way.

@danisla

ehfd commented 1 year ago

Issue 1 and 2: _onFullscreenChange of input.js is not triggered when triggering enterFullscreen of app.js by pressing the Enter fullscreen mode (CTRL+SHIFT+F) button.

Issue 3: Add additional keys to: https://github.com/selkies-project/selkies-gstreamer/blob/61f6b6c834337a02a9ad264fbd1be237418ce5f8/addons/gst-web/src/input.js#L510-L534

danisla commented 1 year ago

To clarify on how the browser works, there are two kinds of full screen behavior:

  1. A JavaScript full screen action initiated via the Fullscreen API, this gives the short "Press ESC to exit full screen" message.
  2. A JavaScript full screen session with Keyboard Lock, same as 1. but with the addition of the keyboard lock function, when in this mode, you see the 'Press and hold ESC to exit full screen". Only in this mode can some additional keyboard and mouse actions be trapped and sent to the remote session, like the ESC key...

I think what you are asking for is to have the CTRL+SHIFT+F behavior match the "Fullscreen" button on the side panel? correct?

And when either action is triggered this should the expected behavior:

  1. full keyboard lock
  2. relative mouse mode enabled
  3. pass additional (what exactly?) keys to the keyboard lock mode.
  4. Have the behavior work exactly the same on Chrome and Firefox.

Is that everything?

ehfd commented 1 year ago

I think what you are asking for is to have the CTRL+SHIFT+F behavior match the "Fullscreen" button on the side panel? correct?

Opposite. That the "Fullscreen" button match the CTRL+SHIFT+F behavior.

  1. full keyboard lock
  2. relative mouse mode enabled
  3. pass additional (what exactly?) keys to the keyboard lock mode.
  4. Have the behavior work exactly the same on Chrome and Firefox.

We need to solve 1 and 2 for now. Then the urgent tag goes away. I'll analyze more about number 3.

ehfd commented 1 year ago

I think number 3 is a false alarm. It was a configuration aspect from Windows, not Selkies.

A button to pass Ctrl+Alt+Del (Like how Guacamole does) would be useful though, because it would not be possible elsewise.

ehfd commented 1 year ago

Seemingly, the fullscreen button WORKS in v1.3.8. I tested it in v1.3.8 and that is the case.

CC @xhejtman

I can reproduce the similarities between v1.3.8 (18.04-legacy) and now. I have also confirmed that no relevant code was changed between then and now. https://github.com/selkies-project/selkies-gstreamer/compare/v1.3.8..v1.5.1

After a Ctrl + Shift + F and then long hold ESC (the first time fullscreen is activated), both Ctrl + Shift + F and the Fullscreen button work as intended.

This means that when something is done, the long hold ESC operation is preserved in subsequent button clicks (and we can start being creative about that).

But without the first Ctrl + Shift + F and then long hold ESC, the Fullscreen button doesn't work correctly.

Perhaps, could we use the "Start" button (since it's needed anyways) to make it pass an operation in conjunction to solve this Fullscreen button issue? @danisla

xhejtman commented 1 year ago

For me, the Fullscreen button does not work even if Ctrl + Shift + F is the first thing. The Ctrl + Shift + F works ok though.

ehfd commented 1 year ago

I have went far as v1.1.2 and the behavior is the same or becomes worse (e.g. long press ESC is activated but remote cursor is not). Possibly affected by browser-side changes.

ehfd commented 1 year ago

Comparison with noVNC, KasmVNC, Guacamole, etc. on the same behavior is required. A possible solution may be found.

ehfd commented 5 months ago

Fixed in https://github.com/selkies-project/selkies-gstreamer/commit/c28639060076f2d583665118bef9d40fb97f4f09.

There's still a small issue where the keystrokes while inside the menu still get passed into the remote desktop, but petty compared to the issue that was before.