crabapples
/
noVNC
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Bugfix/vnc 130 ime race condition (#137) 

* VNC-130 Added IME support for composition updates, and optimized key event handling
This commit is contained in:
quickiwiki 2025-07-18 14:37:13 +05:00 committed by GitHub
parent 87186d1bed
commit 009bd4726d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 116 additions and 143 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
app/ui.js
View File

@ -657,17 +657,12 @@ const UI = {
* ------v------*/ * ------v------*/
// Ignore clicks that are propogated from child elements in sub panels // Ignore clicks that are propogated from child elements in sub panels
isControlPanelItemClick(e) { isControlPanelItemClick(e) {
if (!(e && e.target && e.target.classList && e.target.parentNode && if (!e?.target?.classList || !e?.target?.parentNode)
(
e.target.classList.contains('noVNC_button') && e.target.parentNode.id !== 'noVNC_modifiers' ||
e.target.classList.contains('noVNC_button_div') ||
e.target.classList.contains('noVNC_heading')
)
)) {
return false; return false;
}
return true; return e.target.classList.contains('noVNC_button') && e.target.parentNode?.id !== 'noVNC_modifiers' ||
e.target.classList.contains('noVNC_button_div') ||
e.target.classList.contains('noVNC_heading');
}, },
// Disable/enable controls depending on connection state // Disable/enable controls depending on connection state
@ -1417,6 +1412,7 @@ const UI = {
* ------v------*/ * ------v------*/
connect(event, password) { connect(event, password) {
Log.Debug("UI.connect");
// Ignore when rfb already exists // Ignore when rfb already exists
if (typeof UI.rfb !== 'undefined') { if (typeof UI.rfb !== 'undefined') {
@ -1711,6 +1707,7 @@ const UI = {
//receive message from parent window //receive message from parent window
receiveMessage(event) { receiveMessage(event) {
if (event.data && event.data.action) { if (event.data && event.data.action) {
Log.Debug("Received message from parent window: " + event.data.action);
switch (event.data.action) { switch (event.data.action) {
case 'clipboardsnd': case 'clipboardsnd':
if (UI.rfb && UI.rfb.clipboardUp) { if (UI.rfb && UI.rfb.clipboardUp) {

232
core/input/keyboard.js
View File

@ -11,12 +11,12 @@ import KeyTable from "./keysym.js";
import keysyms from "./keysymdef.js"; import keysyms from "./keysymdef.js";
import imekeys from "./imekeys.js"; import imekeys from "./imekeys.js";
import * as browser from "../util/browser.js"; import * as browser from "../util/browser.js";
import { isChromiumBased } from '../util/browser.js';
// //
// Keyboard event handler // Keyboard event handler
// //
const thresholdTime = 16;
export default class Keyboard { export default class Keyboard {
constructor(screenInput, touchInput) { constructor(screenInput, touchInput) {
this._screenInput = screenInput; this._screenInput = screenInput;
@ -26,22 +26,25 @@ export default class Keyboard {
// (even if they are happy) // (even if they are happy)
this._altGrArmed = false; // Windows AltGr detection this._altGrArmed = false; // Windows AltGr detection
this._rfbKeyQueue = [];
this._lastSendTime = 0;
// keep these here so we can refer to them later // keep these here so we can refer to them later
this._eventHandlers = { this._eventHandlers = {
'keyup': this._handleKeyUp.bind(this), 'keyup': this._handleKeyUp.bind(this),
'keydown': this._handleKeyDown.bind(this), 'keydown': this._handleKeyDown.bind(this),
'blur': this._allKeysUp.bind(this), 'blur': this._allKeysUp.bind(this),
'compositionstart' : this._handleCompositionStart.bind(this), 'compositionstart': this._handleCompositionStart.bind(this),
'compositionend' : this._handleCompositionEnd.bind(this), 'compositionend': this._handleCompositionEnd.bind(this),
'input' : this._handleInput.bind(this) 'compositionupdate': this._handleCompositionUpdate.bind(this),
'input': this._handleInput.bind(this)
}; };
// ===== EVENT HANDLERS ===== // ===== EVENT HANDLERS =====
this.onkeyevent = () => {}; // Handler for key press/release this.onkeyevent = () => {}; // Handler for key press/release
this._enableIME = false; this._enableIME = false;
this._imeHold = false; this._imeStarted = false;
this._imeInProgress = false;
this._lastKeyboardInput = null; this._lastKeyboardInput = null;
this._defaultKeyboardInputLen = 100; this._defaultKeyboardInputLen = 100;
this._keyboardInputReset(); this._keyboardInputReset();
@ -67,9 +70,8 @@ export default class Keyboard {
// tool and the browser only recieves some of the key down events, but not the key up events. This leaves the server // tool and the browser only recieves some of the key down events, but not the key up events. This leaves the server
// out of sync, with cetain keys stuck down. This attempts to discover and fix these occurances in a OS nuetral way // out of sync, with cetain keys stuck down. This attempts to discover and fix these occurances in a OS nuetral way
if (event) { if (event) {
for (const [key, value] of Object.entries(this._keyDownList)) { for (const [key, value] of Object.entries(this._keyDownList)) {
switch(key) { switch (key) {
case "ControlLeft": case "ControlLeft":
case "ControlRight": case "ControlRight":
if (!event.ctrlKey) { if (!event.ctrlKey) {
@ -103,6 +105,29 @@ export default class Keyboard {
} }
} }
_scheduleRfbKeySend() {
if (this._rfbKeyQueue.length === 0) return;
const process = (timestamp) => {
const elapsed = timestamp - this._lastSendTime;
if (elapsed > thresholdTime) {
while (this._rfbKeyQueue.length > 0) {
const event = this._rfbKeyQueue.shift();
Log.Debug("onkeyevent " + (event.down ? "down" : "up") +
", keysym: " + event.keysym, ", code: " + event.code);
this.onkeyevent(event.keysym, event.code, event.down);
}
this._lastSendTime = timestamp;
}
if (this._rfbKeyQueue.length > 0) {
requestAnimationFrame(process);
}
};
requestAnimationFrame(process);
}
_sendKeyEvent(keysym, code, down) { _sendKeyEvent(keysym, code, down) {
if (down) { if (down) {
this._keyDownList[code] = keysym; this._keyDownList[code] = keysym;
@ -114,9 +139,13 @@ export default class Keyboard {
delete this._keyDownList[code]; delete this._keyDownList[code];
} }
Log.Debug("onkeyevent " + (down ? "down" : "up") + this._rfbKeyQueue.push({keysym: keysym, code: code, down: down});
", keysym: " + keysym, ", code: " + code); this._scheduleRfbKeySend();
this.onkeyevent(keysym, code, down); }
_sendKeyStroke(keySym, code) {
this._sendKeyEvent(keySym, code, true);
this._sendKeyEvent(keySym, code, false);
} }
_getKeyCode(e) { _getKeyCode(e) {
@ -137,11 +166,11 @@ export default class Keyboard {
// is not layout independent, so it is as bad as using keyCode // is not layout independent, so it is as bad as using keyCode
if (e.keyIdentifier) { if (e.keyIdentifier) {
// Non-character key? // Non-character key?
if (e.keyIdentifier.substr(0, 2) !== 'U+') { if (!e.keyIdentifier.startsWith('U+')) {
return e.keyIdentifier; return e.keyIdentifier;
} }
const codepoint = parseInt(e.keyIdentifier.substr(2), 16); const codepoint = parseInt(e.keyIdentifier.substring(2), 16);
const char = String.fromCharCode(codepoint).toUpperCase(); const char = String.fromCharCode(codepoint).toUpperCase();
return 'Platform' + char.charCodeAt(); return 'Platform' + char.charCodeAt();
@ -151,19 +180,48 @@ export default class Keyboard {
} }
_handleCompositionStart(e) { _handleCompositionStart(e) {
Log.Debug("composition started"); Log.Debug("Composition started: " + e.data);
if (this._enableIME) { this._imeStarted = true;
this._imeHold = true; this._lastKeyboardInput = "";
this._imeInProgress = true;
} }
_handleCompositionUpdate(e) {
Log.Debug("Composition update: " + e.data);
const oldValue = this._lastKeyboardInput;
const newValue = e.data;
let diffStart = 0;
if (this._imeStarted) {
this._sendKeyStroke(keysyms.lookup(newValue.charCodeAt(0)), 'Unidentified');
this._imeStarted = false;
} else {
//find position where difference starts
for (let i = 0; i < Math.min(oldValue.length, newValue.length); i++) {
if (newValue.charAt(i) !== oldValue.charAt(i)) {
break;
}
diffStart++;
}
//send backspaces if needed
Log.Debug("Backspace diffStart: " + diffStart);
Log.Debug("Old value: " + oldValue + " Old value length: " + oldValue.length + " New value: " + newValue);
for (let bs = oldValue.length - diffStart; bs > 0; bs--) {
this._sendKeyStroke(KeyTable.XK_BackSpace, "Backspace");
}
//send new keys
for (let i = diffStart; i < newValue.length; i++) {
this._sendKeyStroke(keysyms.lookup(newValue.charCodeAt(i)), 'Unidentified');
}
}
this._lastKeyboardInput = newValue;
//this._touchInput.focus();
} }
_handleCompositionEnd(e) { _handleCompositionEnd(e) {
Log.Debug("Composition ended"); Log.Debug("Composition ended: " + e.data);
if (this._enableIME) { this._imeInProgress = false; } this._touchInput.value = '';
if (isChromiumBased()) {
this._imeHold = false;
}
} }
_handleInput(e) { _handleInput(e) {
@ -171,114 +229,36 @@ export default class Keyboard {
//IME events will make this happen, for example //IME events will make this happen, for example
//IME changes can back out old characters and replace, thus send differential if IME //IME changes can back out old characters and replace, thus send differential if IME
//otherwise send new characters //otherwise send new characters
if (this._enableIME && this._imeHold) { Log.Debug("Current buffer: " + this._touchInput.value + " Input: " + e.data + " isComposing: " + e.isComposing + " input.type: " + e.inputType);
Log.Debug("IME input change, sending differential"); if (!e.isComposing && e.inputType !== "insertCompositionText") {
if (!this._imeInProgress) {
this._imeHold = false; //Firefox fires compisitionend before last input change
}
const oldValue = this._lastKeyboardInput;
const newValue = e.target.value;
let diff_start = 0;
//find position where difference starts
for (let i = 0; i < Math.min(oldValue.length, newValue.length); i++) {
if (newValue.charAt(i) != oldValue.charAt(i)) {
break;
}
diff_start++;
}
//send backspaces if needed
for (let bs = oldValue.length - diff_start; bs > 0; bs--) {
this._sendKeyEvent(KeyTable.XK_BackSpace, "Backspace", true);
this._sendKeyEvent(KeyTable.XK_BackSpace, "Backspace", false);
}
//send new keys
for (let i = diff_start; i < newValue.length; i++) {
this._sendKeyEvent(keysyms.lookup(newValue.charCodeAt(i)), 'Unidentified', true);
this._sendKeyEvent(keysyms.lookup(newValue.charCodeAt(i)), 'Unidentified', false);
}
this._lastKeyboardInput = newValue;
} else {
Log.Debug("Non-IME input change, sending new characters"); Log.Debug("Non-IME input change, sending new characters");
const newValue = e.target.value; const newValue = e.data;
if (!this._lastKeyboardInput) { for (let i = 0; i < newValue.length; i++) {
this._keyboardInputReset(); this._sendKeyStroke(keysyms.lookup(newValue.charCodeAt(i)), 'Unidentified');
} }
const oldValue = this._lastKeyboardInput; this._touchInput.value = '';
let newLen;
try {
// Try to check caret position since whitespace at the end
// will not be considered by value.length in some browsers
newLen = Math.max(e.target.selectionStart, newValue.length);
} catch (err) {
// selectionStart is undefined in Google Chrome
newLen = newValue.length;
}
const oldLen = oldValue.length;
let inputs = newLen - oldLen;
let backspaces = inputs < 0 ? -inputs : 0;
// Compare the old string with the new to account for
// text-corrections or other input that modify existing text
for (let i = 0; i < Math.min(oldLen, newLen); i++) {
if (newValue.charAt(i) != oldValue.charAt(i)) {
inputs = newLen - i;
backspaces = oldLen - i;
break;
}
}
// Send the key events
for (let i = 0; i < backspaces; i++) {
this._sendKeyEvent(KeyTable.XK_BackSpace, "Backspace", true);
this._sendKeyEvent(KeyTable.XK_BackSpace, "Backspace", false);
}
for (let i = newLen - inputs; i < newLen; i++) {
this._sendKeyEvent(keysyms.lookup(newValue.charCodeAt(i)), 'Unidentified', true);
this._sendKeyEvent(keysyms.lookup(newValue.charCodeAt(i)), 'Unidentified', false);
}
// Control the text content length in the keyboardinput element
if (newLen > 2 * this._defaultKeyboardInputLen) {
this._keyboardInputReset();
} else if (newLen < 1) {
// There always have to be some text in the keyboardinput
// element with which backspace can interact.
this._keyboardInputReset();
// This sometimes causes the keyboard to disappear for a second
// but it is required for the android keyboard to recognize that
// text has been added to the field
e.target.blur();
// This has to be ran outside of the input handler in order to work
setTimeout(e.target.focus.bind(e.target), 0);
} else {
this._lastKeyboardInput = newValue;
}
} }
} }
_keyboardInputReset() { _keyboardInputReset() {
this._touchInput.value = new Array(this._defaultKeyboardInputLen).join("_"); this._touchInput.value = "";
this._lastKeyboardInput = this._touchInput.value; this._lastKeyboardInput = this._touchInput.value;
} }
_handleKeyDown(e) { _handleKeyDown(e) {
Log.Debug("Key Down: " + e.keyCode + " isComposing: " + e.isComposing);
if (e.isComposing || e.keyCode === 229) {
//skip event if IME related
Log.Debug("Skipping keydown, IME interaction, keycode: " + e.keyCode);
return;
}
const code = this._getKeyCode(e); const code = this._getKeyCode(e);
let keysym = KeyboardUtil.getKeysym(e); let keysym = KeyboardUtil.getKeysym(e);
this.clearKeysDown(e); this.clearKeysDown(e);
Log.Debug("Key Down: " + e.keyCode + " code: " + code + " keysym: " + keysym);
if (this._isIMEInteraction(e)) {
//skip event if IME related
Log.Debug("Skipping keydown, IME interaction, code: " + code + " keysym: " + keysym + " keycode: " + e.keyCode);
return;
}
// Windows doesn't have a proper AltGr, but handles it using // Windows doesn't have a proper AltGr, but handles it using
// fake Ctrl+Alt. However the remote end might not be Windows, // fake Ctrl+Alt. However the remote end might not be Windows,
@ -401,13 +381,13 @@ export default class Keyboard {
} }
_handleKeyUp(e) { _handleKeyUp(e) {
const code = this._getKeyCode(e); Log.Debug("Key Up: " + e.keyCode + " Buffer: " + this._touchInput.value);
if (e.isComposing || e.keyCode === 229) {
if (this._isIMEInteraction(e)) {
//skip IME related events //skip IME related events
Log.Debug("Skipping keyup, IME interaction, code: " + code + " keycode: " + e.keyCode); Log.Debug("Skipping keyup, IME interaction, keycode: " + e.keyCode);
return; return;
} }
const code = this._getKeyCode(e);
stopEvent(e); stopEvent(e);
// We can't get a release in the middle of an AltGr sequence, so // We can't get a release in the middle of an AltGr sequence, so
@ -459,10 +439,10 @@ export default class Keyboard {
_isIMEInteraction(e) { _isIMEInteraction(e) {
//input must come from touchinput (textarea) and ime must be enabled //input must come from touchinput (textarea) and ime must be enabled
if (e.target != this._touchInput || !this._enableIME) { return false; } if (e.target !== this._touchInput || !this._enableIME) { return false; }
//keyCode of 229 is IME composition //keyCode of 229 is IME composition
if (e.keyCode == 229) { if (e.keyCode === 229) {
return true; return true;
} }
@ -471,11 +451,7 @@ export default class Keyboard {
//we can't do that with none character keys though //we can't do that with none character keys though
//Firefox does not seem to fire key events for IME interaction but Chrome does //Firefox does not seem to fire key events for IME interaction but Chrome does
//TODO: potentially skip this for Firefox browsers, needs more testing with different IME types //TODO: potentially skip this for Firefox browsers, needs more testing with different IME types
if (e.keyCode in imekeys) { return e.keyCode in imekeys;
return true;
}
return false;
} }
// ===== PUBLIC METHODS ===== // ===== PUBLIC METHODS =====
@ -498,13 +474,13 @@ export default class Keyboard {
grab() { grab() {
//Log.Debug(">> Keyboard.grab"); //Log.Debug(">> Keyboard.grab");
this._screenInput.addEventListener('keydown', this._eventHandlers.keydown); this._screenInput.addEventListener('keydown', this._eventHandlers.keydown);
this._screenInput.addEventListener('keyup', this._eventHandlers.keyup); this._screenInput.addEventListener('keyup', this._eventHandlers.keyup);
this._touchInput.addEventListener('keydown', this._eventHandlers.keydown); this._touchInput.addEventListener('keydown', this._eventHandlers.keydown);
this._touchInput.addEventListener('keyup', this._eventHandlers.keyup); this._touchInput.addEventListener('keyup', this._eventHandlers.keyup);
this._touchInput.addEventListener('compositionstart', this._eventHandlers.compositionstart); this._touchInput.addEventListener('compositionstart', this._eventHandlers.compositionstart);
this._touchInput.addEventListener('compositionupdate', this._eventHandlers.compositionupdate);
this._touchInput.addEventListener('compositionend', this._eventHandlers.compositionend); this._touchInput.addEventListener('compositionend', this._eventHandlers.compositionend);
this._touchInput.addEventListener('input', this._eventHandlers.input); this._touchInput.addEventListener('input', this._eventHandlers.input);
@ -516,13 +492,13 @@ export default class Keyboard {
ungrab() { ungrab() {
//Log.Debug(">> Keyboard.ungrab"); //Log.Debug(">> Keyboard.ungrab");
this._screenInput.removeEventListener('keydown', this._eventHandlers.keydown); this._screenInput.removeEventListener('keydown', this._eventHandlers.keydown);
this._screenInput.removeEventListener('keyup', this._eventHandlers.keyup); this._screenInput.removeEventListener('keyup', this._eventHandlers.keyup);
this._touchInput.removeEventListener('keydown', this._eventHandlers.keydown); this._touchInput.removeEventListener('keydown', this._eventHandlers.keydown);
this._touchInput.removeEventListener('keyup', this._eventHandlers.keyup); this._touchInput.removeEventListener('keyup', this._eventHandlers.keyup);
this._touchInput.removeEventListener('compositionstart', this._eventHandlers.compositionstart); this._touchInput.removeEventListener('compositionstart', this._eventHandlers.compositionstart);
this._touchInput.removeEventListener('compositionupdate', this._eventHandlers.compositionupdate);
this._touchInput.removeEventListener('compositionend', this._eventHandlers.compositionend); this._touchInput.removeEventListener('compositionend', this._eventHandlers.compositionend);
this._touchInput.removeEventListener('input', this._eventHandlers.input); this._touchInput.removeEventListener('input', this._eventHandlers.input);

2
core/input/keysymdef.js
View File

@ -682,7 +682,7 @@ export default {
return keysym; return keysym;
} }
// General mapping as final fallback // General mapping as the final fallback
return 0x01000000 | u; return 0x01000000 | u;
}, },
}; };

2
core/rfb.js
View File

@ -399,6 +399,7 @@ export default class RFB extends EventTargetMixin {
get viewOnly() { return this._viewOnly; } get viewOnly() { return this._viewOnly; }
set viewOnly(viewOnly) { set viewOnly(viewOnly) {
Log.Debug("Setting viewOnly to " + viewOnly);
this._viewOnly = viewOnly; this._viewOnly = viewOnly;
if (this._rfbConnectionState === "connecting" || if (this._rfbConnectionState === "connecting" ||
@ -986,7 +987,6 @@ export default class RFB extends EventTargetMixin {
if (!keysym) { if (!keysym) {
return; return;
} }
Log.Info("Sending keysym (" + (down ? "down" : "up") + "): " + keysym);
if (this._isPrimaryDisplay) { if (this._isPrimaryDisplay) {
RFB.messages.keyEvent(this._sock, keysym, down ? 1 : 0); RFB.messages.keyEvent(this._sock, keysym, down ? 1 : 0);
} else { } else {