Bug 1018766 - virt-viewer: Ctrl-alt-end conflicts with default keyboard release
virt-viewer: Ctrl-alt-end conflicts with default keyboard release
Status: CLOSED NOTABUG
Product: Red Hat Enterprise Virtualization Manager
Classification: Red Hat
Component: mingw-virt-viewer (Show other bugs)
3.3.0
Unspecified Unspecified
low Severity low
: ovirt-4.0.0-beta
: 4.0.0
Assigned To: Jonathon Jongsma
SPICE QE bug list
:
: 1244627 (view as bug list)
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2013-10-14 07:45 EDT by Tomas Jamrisko
Modified: 2016-05-10 15:01 EDT (History)
9 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2016-05-10 15:01:33 EDT
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: Spice
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Tomas Jamrisko 2013-10-14 07:45:40 EDT
Description of problem:

Ctrl-alt-end, as default shortcut for ctrl-alt-del, sometimes conflicts with the default release cursor which is ctrl-alt. It happens consistently when the agent is not running, otherwise for some reason  noticeable only after maximizing the window.


Version-Release number of selected component (if applicable):
mingw-virt-viewer-0.5.6-6
Comment 5 David Blechter 2013-11-08 15:00:12 EST
in rhevm the cursor release key is shift-F11 and as the result there are no conflicts. Low severity - moving to 3.4
Comment 6 Marc-Andre Lureau 2014-03-04 13:08:07 EST
(In reply to Tomas Jamrisko from comment #0)
> Description of problem:
> 
> Ctrl-alt-end, as default shortcut for ctrl-alt-del, sometimes conflicts with
> the default release cursor which is ctrl-alt. It happens consistently when
> the agent is not running, otherwise for some reason  noticeable only after
> maximizing the window.

What happens *when* there is a conflict for those 2 shortcuts? I would assume the 2 actions to happen, but this is arguable...

This a bit hairy, conflicting shortcuts are.. conflicting, so, what should it do? refuse to start with an error?
Comment 7 Tomas Jamrisko 2014-03-05 05:32:10 EST
(In reply to Marc-Andre Lureau from comment #6)
> What happens *when* there is a conflict for those 2 shortcuts? I would
> assume the 2 actions to happen, but this is arguable...

That is not the case. Only the cursor gets released. Guest doesn't get the event at all. 
 
> This a bit hairy, conflicting shortcuts are.. conflicting, so, what should
> it do? refuse to start with an error?
 
These are default/hardcoded shortcuts, so they should *not* cause conflicts. The way virt-viewer handles shortcuts that are prefixes of different shortcuts is just wrong. A shortcut, that is a prefix of a different shortcut shouldn't be handled until any key is released... 

When a 'user' sets conflicting shortcuts, the reasonable thing to do would be a warning and then ignoring his settings. But no one complained up until now...
Comment 8 Jonathon Jongsma 2014-03-11 18:09:53 EDT
Marc-Andre: is there a particular reason that we handle the ungrab sequence on key-press instead of key-release?
Comment 9 Marc-Andre Lureau 2014-03-11 18:17:08 EDT
(In reply to Jonathon Jongsma from comment #8)
> Marc-Andre: is there a particular reason that we handle the ungrab sequence
> on key-press instead of key-release?

not that I know, the code was based on gtk-vnc, and the first commits had:

+       if (key->type == GDK_KEY_PRESS &&
+           ((key->keyval == GDK_Control_L && (key->state & GDK_MOD1_MASK)) ||
+            (key->keyval == GDK_Alt_L && (key->state & GDK_CONTROL_MASK))))
+               toggle_grab(VNC_DISPLAY(widget));


But I think it makes more sense on release as well.
Comment 10 Jonathon Jongsma 2014-03-11 18:25:48 EDT
OK, I have a patch here to switch to key-release.  I think that should help resole this conflict.  Will post it after a bit more testing and cleanup.
Comment 11 Marc-Andre Lureau 2014-03-11 18:30:28 EDT
(In reply to Jonathon Jongsma from comment #10)
> OK, I have a patch here to switch to key-release.  I think that should help
> resole this conflict.  Will post it after a bit more testing and cleanup.

please do the same change in gtk-vnc (sigh, some day, we will have a common lib or submodule, there are a number of common bits :)
Comment 12 Jonathon Jongsma 2014-03-12 16:07:31 EDT
(In reply to Tomas Jamrisko from comment #7)
> (In reply to Marc-Andre Lureau from comment #6)
> > What happens *when* there is a conflict for those 2 shortcuts? I would
> > assume the 2 actions to happen, but this is arguable...
> 
> That is not the case. Only the cursor gets released. Guest doesn't get the
> event at all. 
>  

Hi Tomas,

I'm a little bit confused here.  I've got a patch that I'm trying to test, but I haven't been able to reproduce the original behavior you describe here.  Can you describe exactly how you start remote-viewer? Since this bug is about the default keybindings, I assume you're not passing any --hotkeys parameters?

A bit of background about hotkeys and keybindings (since I had to refresh my memory about this stuff when I started investigating this): virt-viewer tries to keep global hotkeys to a minimum so that the application does not prevent useful key sequences from being used with the guest.  When I say 'global hotkeys', I mean keys that will be interpreted/intercepted by virt-viewer even when the spice display has focus.  By default, the only global hotkey is the sequence to release the cursor grab (ctrl+alt by default).  All other keybindings are 'non-global' unless they are specified in the --hotkeys parameter (or equivelent .vv file options, etc).  

This means that in the default configuration, if the spice display has the cursor grab, ctrl+alt+end is *not supposed* to send ctrl+alt+del to the guest.  Since it's a non-global keybinding, it will literally send ctrl+alt+end to the guest if this sequence is pressed while the cursor is grabbed.  So in this scenario, it is expected that only the cursor gets released.  By default, ctrl+alt+end only works as a secure attention sequence if the mouse is not grabbed (e.g. the pointer is over the menu).

On the other hand, if you started remote-viewer with something like '--hotkeys secure-attention=ctrl+alt+end', then we would expect this key sequence to result in a ctrl+alt+del being sent to the guest even when the mouse is grabbed. 

So, now that I've covered that, can you give a bit more information about how you're testing this?
Comment 13 Tomas Jamrisko 2014-03-13 08:38:21 EDT
Hi Jonathon,

(In reply to Jonathon Jongsma from comment #12)
> I'm a little bit confused here.  I've got a patch that I'm trying to test,
> but I haven't been able to reproduce the original behavior you describe
> here.  Can you describe exactly how you start remote-viewer? Since this bug
> is about the default keybindings, I assume you're not passing any --hotkeys
> parameters?

The exact description: 
- you need a Windows client and a Windows guest. The guest has to have mouse in server mode -- disabled vdagent and no tablet device in case you're running it from libvirt. You should have the 'Press ctrl+alt to release cursor' message in titlebar.
- when you press ctrl-alt-end now, the only thing that will happen is released cursor.
- I do not start it with any special parameters, just the minimum -- protocol://host:port

> A bit of background about hotkeys and keybindings (since I had to refresh my
> memory about this stuff when I started investigating this): virt-viewer
> tries to keep global hotkeys to a minimum so that the application does not
> prevent useful key sequences from being used with the guest. When I say
> 'global hotkeys', I mean keys that will be interpreted/intercepted by
> virt-viewer even when the spice display has focus.  By default, the only
> global hotkey is the sequence to release the cursor grab (ctrl+alt by
> default).  All other keybindings are 'non-global' unless they are specified
> in the --hotkeys parameter (or equivelent .vv file options, etc).  
> 
> This means that in the default configuration, if the spice display has the
> cursor grab, ctrl+alt+end is *not supposed* to send ctrl+alt+del to the
> guest.  Since it's a non-global keybinding, it will literally send
> ctrl+alt+end to the guest if this sequence is pressed while the cursor is
> grabbed.  So in this scenario, it is expected that only the cursor gets
> released.  By default, ctrl+alt+end only works as a secure attention
> sequence if the mouse is not grabbed (e.g. the pointer is over the menu).
> So, now that I've covered that, can you give a bit more information about
> how you're testing this?

Your explanation makes perfect sense. And yes, from this perspective it behaves as expected. If you insist, that this is the way it should be, you could very well close this as NOTABUG. Especially since most costumers/users will use it with RHEVM and hardly ever stand-alone...

However, I believe that the current behaviour should be changed as the way shortcuts were set is not important from the user's point of view, yet the behaviour is different. Also the whole point of setting ctrl-alt-end as ctrl-alt-del is not to force user to use 'Send key' menu or change focus, as it's always intercepted by OS on windows clients. So if this isn't a bug, Ctrl-alt-end should be removed from default shortcuts.
Comment 14 Jonathon Jongsma 2014-03-13 10:19:53 EDT
Thanks for the additional info.  So this is indeed working as designed. Too bad I didn't realize that before starting to work on the patch...

Of course, as you suggest, the question of whether the design is correct is a valid question.  Marc-andre has been advocating (and I've been convinced) that we should keep our default keybindings to the bare minimum so that the virt-viewer app does not prevent the user from doing certain things in the guest.  Of course, there are special cases (such as the cursor-release hotkey), and you could argue that this is another special case (at least for the windows client). I think there are strong arguments for both viewpoints...
Comment 15 Tomas Jamrisko 2015-07-20 08:34:44 EDT
*** Bug 1244627 has been marked as a duplicate of this bug. ***
Comment 16 Sandro Bonazzola 2015-10-26 08:37:07 EDT
this is an automated message. oVirt 3.6.0 RC3 has been released and GA is targeted to next week, Nov 4th 2015.
Please review this bug and if not a blocker, please postpone to a later release.
All bugs not postponed on GA release will be automatically re-targeted to

- 3.6.1 if severity >= high
- 4.0 if severity < high
Comment 17 Yaniv Lavi 2016-05-09 07:03:23 EDT
oVirt 4.0 Alpha has been released, moving to oVirt 4.0 Beta target.
Comment 20 David Blechter 2016-05-10 15:01:33 EDT
Closing based on comment #14 and following discussion with developers

Note You need to log in before you can comment on or make changes to this bug.