Clean up AltGraph handling

It doesn't need to be this general as the issue is mostly about
Windows. Also use the same modifier shuffle that RealVNC and
TigerVNC uses to get macOS working well.

core/input/devices.js

@@ -13,6 +13,7 @@ import { isTouchDevice } from '../util/browsers.js'
 import { setCapture, releaseCapture, stopEvent, getPointerEvent } from '../util/events.js';
 import { set_defaults, make_properties } from '../util/properties.js';
 import * as KeyboardUtil from "./util.js";
+import KeyTable from "./keysym.js";
 
 //
 // Keyboard event handler
@@ -23,8 +24,6 @@ const Keyboard = function (defaults) {
                                     // (even if they are happy)
     this._pendingKey = null;        // Key waiting for keypress
 
-    this._modifierState = KeyboardUtil.ModifierSync();
-
     set_defaults(this, defaults, {
         'target': document,
         'focused': true
@@ -39,6 +38,13 @@ const Keyboard = function (defaults) {
     };
 };
 
+function isMac() {
+    return navigator && !!(/mac/i).exec(navigator.platform);
+}
+function isWindows() {
+    return navigator && !!(/win/i).exec(navigator.platform);
+}
+
 Keyboard.prototype = {
     // private methods
 
@@ -50,7 +56,32 @@ Keyboard.prototype = {
         Log.Debug("onKeyEvent " + (down ? "down" : "up") +
                   ", keysym: " + keysym, ", code: " + code);
 
+        // Windows sends CtrlLeft+AltRight when you press
+        // AltGraph, which tends to confuse the hell out of
+        // remote systems. Fake a release of these keys until
+        // there is a way to detect AltGraph properly.
+        var fakeAltGraph = false;
+        if (down && isWindows()) {
+            if ((code !== 'ControlLeft') &&
+                (code !== 'AltRight') &&
+                ('ControlLeft' in this._keyDownList) &&
+                ('AltRight' in this._keyDownList)) {
+                fakeAltGraph = true;
+                this._onKeyEvent(this._keyDownList['AltRight'],
+                                 'AltRight', false);
+                this._onKeyEvent(this._keyDownList['ControlLeft'],
+                                 'ControlLeft', false);
+            }
+        }
+
         this._onKeyEvent(keysym, code, down);
+
+        if (fakeAltGraph) {
+            this._onKeyEvent(this._keyDownList['ControlLeft'],
+                             'ControlLeft', true);
+            this._onKeyEvent(this._keyDownList['AltRight'],
+                             'AltRight', true);
+        }
     },
 
     _getKeyCode: function (e) {
@@ -70,8 +101,6 @@ Keyboard.prototype = {
     _handleKeyDown: function (e) {
         if (!this._focused) { return; }
 
-        this._modifierState.keydown(e);
-
         var code = this._getKeyCode(e);
         var keysym = KeyboardUtil.getKeysym(e);
 
@@ -90,6 +119,27 @@ Keyboard.prototype = {
             return;
         }
 
+        // Alt behaves more like AltGraph on macOS, so shuffle the
+        // keys around a bit to make things more sane for the remote
+        // server. This method is used by RealVNC and TigerVNC (and
+        // possibly others).
+        if (isMac()) {
+            switch (keysym) {
+            case KeyTable.XK_Super_L:
+                keysym = KeyTable.XK_Alt_L;
+                break;
+            case KeyTable.XK_Super_R:
+                keysym = KeyTable.XK_Super_L;
+                break;
+            case KeyTable.XK_Alt_L:
+                keysym = KeyTable.XK_Mode_switch;
+                break;
+            case KeyTable.XK_Alt_R:
+                keysym = KeyTable.XK_ISO_Level3_Shift;
+                break;
+            }
+        }
+
         // Is this key already pressed? If so, then we must use the
         // same keysym or we'll confuse the server
         if (code in this._keyDownList) {
@@ -106,45 +156,9 @@ Keyboard.prototype = {
         this._pendingKey = null;
         stopEvent(e);
 
-        // if a char modifier is pressed, get the keys it consists
-        // of (on Windows, AltGr is equivalent to Ctrl+Alt)
-        var active = this._modifierState.activeCharModifier();
-
-        // If we have a char modifier down, and we're able to
-        // determine a keysym reliably then (a) we know to treat
-        // the modifier as a char modifier, and (b) we'll have to
-        // "escape" the modifier to undo the modifier when sending
-        // the char.
-        if (active) {
-            var isCharModifier = false;
-            for (var i  = 0; i < active.length; ++i) {
-                if (active[i] === keysym) {
-                    isCharModifier = true;
-                }
-            }
-            if (!isCharModifier) {
-                var escape = this._modifierState.activeCharModifier();
-            }
-        }
-
         this._keyDownList[code] = keysym;
 
-        // undo modifiers
-        if (escape) {
-            for (var i = 0; i < escape.length; ++i) {
-                this._sendKeyEvent(escape[i], 'Unidentified', false);
-            }
-        }
-
-        // send the character event
         this._sendKeyEvent(keysym, code, true);
-
-        // redo modifiers
-        if (escape) {
-            for (i = 0; i < escape.length; ++i) {
-                this._sendKeyEvent(escape[i], 'Unidentified', true);
-            }
-        }
     },
 
     // Legacy event for browsers without code/key
@@ -169,27 +183,6 @@ Keyboard.prototype = {
         code = this._pendingKey;
         this._pendingKey = null;
 
-        // if a char modifier is pressed, get the keys it consists
-        // of (on Windows, AltGr is equivalent to Ctrl+Alt)
-        var active = this._modifierState.activeCharModifier();
-
-        // If we have a char modifier down, and we're able to
-        // determine a keysym reliably then (a) we know to treat
-        // the modifier as a char modifier, and (b) we'll have to
-        // "escape" the modifier to undo the modifier when sending
-        // the char.
-        if (active && keysym) {
-            var isCharModifier = false;
-            for (var i  = 0; i < active.length; ++i) {
-                if (active[i] === keysym) {
-                    isCharModifier = true;
-                }
-            }
-            if (!isCharModifier) {
-                var escape = this._modifierState.activeCharModifier();
-            }
-        }
-
         if (!keysym) {
             console.log('keypress with no keysym:', e);
             return;
@@ -197,22 +190,7 @@ Keyboard.prototype = {
 
         this._keyDownList[code] = keysym;
 
-        // undo modifiers
-        if (escape) {
-            for (var i = 0; i < escape.length; ++i) {
-                this._sendKeyEvent(escape[i], 'Unidentified', false);
-            }
-        }
-
-        // send the character event
         this._sendKeyEvent(keysym, code, true);
-
-        // redo modifiers
-        if (escape) {
-            for (i = 0; i < escape.length; ++i) {
-                this._sendKeyEvent(escape[i], 'Unidentified', true);
-            }
-        }
     },
 
     _handleKeyUp: function (e) {
@@ -220,8 +198,6 @@ Keyboard.prototype = {
 
         stopEvent(e);
 
-        this._modifierState.keyup(e);
-
         var code = this._getKeyCode(e);
 
         // Do we really think this key is down?

core/input/util.js

@@ -6,105 +6,6 @@ import fixedkeys from "./fixedkeys.js";
 function isMac() {
     return navigator && !!(/mac/i).exec(navigator.platform);
 }
-function isWindows() {
-    return navigator && !!(/win/i).exec(navigator.platform);
-}
-function isLinux() {
-    return navigator && !!(/linux/i).exec(navigator.platform);
-}
-
-// Return true if the specified char modifier is currently down
-export function hasCharModifier(charModifier, currentModifiers) {
-    if (charModifier.length === 0) { return false; }
-
-    for (var i = 0; i < charModifier.length; ++i) {
-        if (!currentModifiers[charModifier[i]]) {
-            return false;
-        }
-    }
-    return true;
-}
-
-// Helper object tracking modifier key state
-// and generates fake key events to compensate if it gets out of sync
-export function ModifierSync(charModifier) {
-    if (!charModifier) {
-        if (isMac()) {
-            // on Mac, Option (AKA Alt) is used as a char modifier
-            charModifier = [KeyTable.XK_Alt_L];
-        }
-        else if (isWindows()) {
-            // on Windows, Ctrl+Alt is used as a char modifier
-            charModifier = [KeyTable.XK_Alt_L, KeyTable.XK_Control_L];
-        }
-        else if (isLinux()) {
-            // on Linux, ISO Level 3 Shift (AltGr) is used as a char modifier
-            charModifier = [KeyTable.XK_ISO_Level3_Shift];
-        }
-        else {
-            charModifier = [];
-        }
-    }
-
-    var state = {};
-    state[KeyTable.XK_Control_L] = false;
-    state[KeyTable.XK_Alt_L] = false;
-    state[KeyTable.XK_ISO_Level3_Shift] = false;
-    state[KeyTable.XK_Shift_L] = false;
-    state[KeyTable.XK_Meta_L] = false;
-
-    function sync(evt, keysym) {
-        var result = [];
-        function syncKey(keysym) {
-            return {keysym: keysym, type: state[keysym] ? 'keydown' : 'keyup'};
-        }
-
-        if (evt.ctrlKey !== undefined &&
-            evt.ctrlKey !== state[KeyTable.XK_Control_L] && keysym !== KeyTable.XK_Control_L) {
-            state[KeyTable.XK_Control_L] = evt.ctrlKey;
-            result.push(syncKey(KeyTable.XK_Control_L));
-        }
-        if (evt.altKey !== undefined &&
-            evt.altKey !== state[KeyTable.XK_Alt_L] && keysym !== KeyTable.XK_Alt_L) {
-            state[KeyTable.XK_Alt_L] = evt.altKey;
-            result.push(syncKey(KeyTable.XK_Alt_L));
-        }
-        if (evt.altGraphKey !== undefined &&
-            evt.altGraphKey !== state[KeyTable.XK_ISO_Level3_Shift] && keysym !== KeyTable.XK_ISO_Level3_Shift) {
-            state[KeyTable.XK_ISO_Level3_Shift] = evt.altGraphKey;
-            result.push(syncKey(KeyTable.XK_ISO_Level3_Shift));
-        }
-        if (evt.shiftKey !== undefined &&
-            evt.shiftKey !== state[KeyTable.XK_Shift_L] && keysym !== KeyTable.XK_Shift_L) {
-            state[KeyTable.XK_Shift_L] = evt.shiftKey;
-            result.push(syncKey(KeyTable.XK_Shift_L));
-        }
-        if (evt.metaKey !== undefined &&
-            evt.metaKey !== state[KeyTable.XK_Meta_L] && keysym !== KeyTable.XK_Meta_L) {
-            state[KeyTable.XK_Meta_L] = evt.metaKey;
-            result.push(syncKey(KeyTable.XK_Meta_L));
-        }
-        return result;
-    }
-    function syncKeyEvent(evt, down) {
-        var keysym = getKeysym(evt);
-
-        // first, apply the event itself, if relevant
-        if (keysym !== null && state[keysym] !== undefined) {
-            state[keysym] = down;
-        }
-        return sync(evt, keysym);
-    }
-
-    return {
-        // sync on the appropriate keyboard event
-        keydown: function(evt) { return syncKeyEvent(evt, true);},
-        keyup: function(evt) { return syncKeyEvent(evt, false);},
-
-        // if a char modifier is down, return the keys it consists of, otherwise return null
-        activeCharModifier: function() { return hasCharModifier(charModifier, state) ? charModifier : null; }
-    };
-}
 
 // Get 'KeyboardEvent.code', handling legacy browsers
 export function getKeycode(evt){

tests/test.keyboard.js

@@ -146,7 +146,72 @@ describe('Key Event Handling', function() {
         });
     });
 
-    describe('Escape Modifiers', function() {
+    describe('Shuffle modifiers on macOS', function() {
+        var origNavigator;
+        beforeEach(function () {
+            // window.navigator is a protected read-only property in many
+            // environments, so we need to redefine it whilst running these
+            // tests.
+            origNavigator = Object.getOwnPropertyDescriptor(window, "navigator");
+            if (origNavigator === undefined) {
+                // Object.getOwnPropertyDescriptor() doesn't work
+                // properly in any version of IE
+                this.skip();
+            }
+
+            Object.defineProperty(window, "navigator", {value: {}});
+            if (window.navigator.platform !== undefined) {
+                // Object.defineProperty() doesn't work properly in old
+                // versions of Chrome
+                this.skip();
+            }
+
+            window.navigator.platform = "Mac x86_64";
+        });
+        afterEach(function () {
+            Object.defineProperty(window, "navigator", origNavigator);
+        });
+
+        it('should change Alt to AltGraph', function() {
+            var count = 0;
+            var kbd = new Keyboard({
+            onKeyEvent: function(keysym, code, down) {
+                switch (count++) {
+                    case 0:
+                        expect(keysym).to.be.equal(0xFF7E);
+                        expect(code).to.be.equal('AltLeft');
+                        break;
+                    case 1:
+                        expect(keysym).to.be.equal(0xFE03);
+                        expect(code).to.be.equal('AltRight');
+                        break;
+                }
+            }});
+            kbd._handleKeyDown(keyevent('keydown', {code: 'AltLeft', key: 'Alt'}));
+            kbd._handleKeyDown(keyevent('keydown', {code: 'AltRight', key: 'Alt'}));
+            expect(count).to.be.equal(2);
+        });
+        it('should change left Super to Alt', function(done) {
+            var kbd = new Keyboard({
+            onKeyEvent: function(keysym, code, down) {
+                expect(keysym).to.be.equal(0xFFE9);
+                expect(code).to.be.equal('MetaLeft');
+                done();
+            }});
+            kbd._handleKeyDown(keyevent('keydown', {code: 'MetaLeft', key: 'Meta'}));
+        });
+        it('should change right Super to left Super', function(done) {
+            var kbd = new Keyboard({
+            onKeyEvent: function(keysym, code, down) {
+                expect(keysym).to.be.equal(0xFFEB);
+                expect(code).to.be.equal('MetaRight');
+                done();
+            }});
+            kbd._handleKeyDown(keyevent('keydown', {code: 'MetaRight', key: 'Meta'}));
+        });
+    });
+
+    describe('Escape AltGraph on Windows', function() {
         var origNavigator;
         beforeEach(function () {
             // window.navigator is a protected read-only property in many
@@ -172,7 +237,7 @@ describe('Key Event Handling', function() {
             Object.defineProperty(window, "navigator", origNavigator);
         });
 
-        it('should generate fake undo/redo events on press when a char modifier is down', function() {
+        it('should generate fake undo/redo events on press when AltGraph is down', function() {
             var times_called = 0;
             var kbd = new Keyboard({
             onKeyEvent: function(keysym, code, down) {
@@ -183,18 +248,18 @@ describe('Key Event Handling', function() {
                     expect(down).to.be.equal(true);
                     break;
                 case 1:
-                    expect(keysym).to.be.equal(0xFFE9);
+                    expect(keysym).to.be.equal(0xFFEA);
-                    expect(code).to.be.equal('AltLeft');
+                    expect(code).to.be.equal('AltRight');
                     expect(down).to.be.equal(true);
                     break;
                 case 2:
-                    expect(keysym).to.be.equal(0xFFE9);
+                    expect(keysym).to.be.equal(0xFFEA);
-                    expect(code).to.be.equal('Unidentified');
+                    expect(code).to.be.equal('AltRight');
                     expect(down).to.be.equal(false);
                     break;
                 case 3:
                     expect(keysym).to.be.equal(0xFFE3);
-                    expect(code).to.be.equal('Unidentified');
+                    expect(code).to.be.equal('ControlLeft');
                     expect(down).to.be.equal(false);
                     break;
                 case 4:
@@ -203,20 +268,20 @@ describe('Key Event Handling', function() {
                     expect(down).to.be.equal(true);
                     break;
                 case 5:
-                    expect(keysym).to.be.equal(0xFFE9);
+                    expect(keysym).to.be.equal(0xFFE3);
-                    expect(code).to.be.equal('Unidentified');
+                    expect(code).to.be.equal('ControlLeft');
                     expect(down).to.be.equal(true);
                     break;
                 case 6:
-                    expect(keysym).to.be.equal(0xFFE3);
+                    expect(keysym).to.be.equal(0xFFEA);
-                    expect(code).to.be.equal('Unidentified');
+                    expect(code).to.be.equal('AltRight');
                     expect(down).to.be.equal(true);
                     break;
                 }
             }});
             // First the modifier combo
             kbd._handleKeyDown(keyevent('keydown', {code: 'ControlLeft', key: 'Control'}));
-            kbd._handleKeyDown(keyevent('keydown', {code: 'AltLeft', key: 'Alt'}));
+            kbd._handleKeyDown(keyevent('keydown', {code: 'AltRight', key: 'Alt'}));
             // Next a normal character
             kbd._handleKeyDown(keyevent('keydown', {code: 'KeyA', key: 'a'}));
             expect(times_called).to.be.equal(7);
@@ -235,7 +300,7 @@ describe('Key Event Handling', function() {
             }});
             // First the modifier combo
             kbd._handleKeyDown(keyevent('keydown', {code: 'ControlLeft', key: 'Control'}));
-            kbd._handleKeyDown(keyevent('keydown', {code: 'AltLeft', key: 'Alt'}));
+            kbd._handleKeyDown(keyevent('keydown', {code: 'AltRight', key: 'Alt'}));
             // Next a normal character
             kbd._handleKeyDown(keyevent('keydown', {code: 'KeyA', key: 'a'}));
             kbd._handleKeyUp(keyevent('keyup', {code: 'KeyA', key: 'a'}));