Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add allowQEMUExtKeyEvent option, which allows the qemu keyboard exten… #667

Closed
wants to merge 1 commit into from

Conversation

devicenull
Copy link
Contributor

Add allowQEMUExtKeyEvent option, which allows the qemu keyboard extensions to be disabled. This is required for compatability with instances that are still running with a '-k' option on the command line.

Setting this to false would fix the issues reported in #666

// If you have qemu guests that are still running with the '-k en-us' (or any other keymap), you'll need
// to set this to false, or each keypress will result in messages like:
// Unknown key released (translated set 2, code 0x0 on isa0060/serio0).
'allowQEMUExtKeyEvent': true});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't be placed around the callback functions, place it a few lines up please

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add this for ui.js as well?

Copy link
Member

@samhed samhed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few comments, otherwise it LGTM

@devicenull
Copy link
Contributor Author

Updated

…sions to be disabled. This is required for compatability with instances that are still running with a '-k' option on the command line
@berrange
Copy link

This kind of hack should NOT be needed. The Extended keyboard event message contains both the scancode and keysym. The keysym sent should be exactly the same as the keysym sent with the non-extended keyboard event message. If QEMU has a keymap set, it will then ignore the scancode and only use the keysym.

If this is not working, then it suggests that the keysym being included in the extended keyboard event message is somehow different from the keysym being sent in the non-extended keyboard event message.

This bit of code in core/rfb.js looks pretty suspect

    _handleKeyPress: function (keyevent) {
        if (this._view_only) { return; } // View only, skip keyboard, events

        var down = (keyevent.type == 'keydown');
        if (this._qemuExtKeyEventSupported) {
            var scancode = XtScancode[keyevent.code];
            if (scancode) {
                var keysym = keyevent.keysym;
                RFB.messages.QEMUExtendedKeyEvent(this._sock, keysym, down, scancode);
            } else {
                Util.Error('Unable to find a xt scancode for code = ' + keyevent.code);
            }
        } else {
            keysym = keyevent.keysym.keysym;
            RFB.messages.keyEvent(this._sock, keysym, down);
        }
    },

Notice that in the extended event scenario it is sending "keyevent.keysym" but in the non-extended scenario it is sending "keyevent.keysym.keysym". Surely those should both be keyevent.keysym.keysym

@DirectXMan12 DirectXMan12 added this to the v0.7.0 milestone Sep 30, 2016
@DirectXMan12
Copy link
Member

@berrange this would be a short-term fix until we can integrate the two bits of handling logic (as per #666 (comment)). If we don't get that long-term fix before 0.7.0, we should merge this (with a UI option).

@samhed
Copy link
Member

samhed commented Nov 18, 2016

@devicenull could you add a UI option for this?

@CendioOssman
Copy link
Member

No longer needed as the bug in the keyboard code should now be fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants