From 3e8bb981974fffee09951cca8939b33309b5e86c Mon Sep 17 00:00:00 2001 From: Paris Date: Wed, 13 Jul 2016 16:35:57 +0300 Subject: [PATCH 1/4] Fix #169 --- src/xterm.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/xterm.js b/src/xterm.js index 64f56fed..7e420312 100644 --- a/src/xterm.js +++ b/src/xterm.js @@ -4953,8 +4953,8 @@ (term.isMac && ev.altKey && !ev.ctrlKey && !ev.metaKey) || (term.isMSWindows && ev.altKey && ev.ctrlKey && !ev.metaKey); - // Don't invoke for arrows, pageDown, home, backspace, etc. - return thirdLevelKey && (!ev.keyCode || ev.keyCode > 47); + // Don't invoke for arrows, pageDown, home, backspace, etc. (on non-keypress events) + return thirdLevelKey && (ev.type != 'keypress' || (!ev.keyCode || ev.keyCode > 47)); } function matchColor(r1, g1, b1) { From 6663a947eec09d257a6f7126ca9e406c0121761d Mon Sep 17 00:00:00 2001 From: Paris Date: Wed, 13 Jul 2016 16:48:48 +0300 Subject: [PATCH 2/4] Separate dummy keyDown and keyPress events in tests --- test/test.js | 54 +++++++++++++++++++++++++++++----------------------- 1 file changed, 30 insertions(+), 24 deletions(-) diff --git a/test/test.js b/test/test.js index 70b5db8e..5059a242 100644 --- a/test/test.js +++ b/test/test.js @@ -72,10 +72,16 @@ describe('xterm.js', function() { }); describe('Third level shift', function() { - var ev = { - preventDefault: function() {}, - stopPropagation: function() {} - }; + var evKeyDown = { + preventDefault: function() {}, + stopPropagation: function() {}, + type: 'keydown' + }, + evKeyPress = { + preventDefault: function() {}, + stopPropagation: function() {}, + type: 'keypress' + }; beforeEach(function() { xterm.handler = function() {}; @@ -90,22 +96,22 @@ describe('xterm.js', function() { it('should not interfere with the alt key on keyDown', function() { assert.equal( - xterm.keyDown(Object.assign({}, ev, { altKey: true, keyCode: 81 })), + xterm.keyDown(Object.assign({}, evKeyDown, { altKey: true, keyCode: 81 })), true ); assert.equal( - xterm.keyDown(Object.assign({}, ev, { altKey: true, keyCode: 192 })), + xterm.keyDown(Object.assign({}, evKeyDown, { altKey: true, keyCode: 192 })), true ); }); it('should interefere with the alt + arrow keys', function() { assert.equal( - xterm.keyDown(Object.assign({}, ev, { altKey: true, keyCode: 37 })), + xterm.keyDown(Object.assign({}, evKeyDown, { altKey: true, keyCode: 37 })), false ); assert.equal( - xterm.keyDown(Object.assign({}, ev, { altKey: true, keyCode: 39 })), + xterm.keyDown(Object.assign({}, evKeyDown, { altKey: true, keyCode: 39 })), false ); }); @@ -122,13 +128,13 @@ describe('xterm.js', function() { if (keys.length === 0) done(); }); - xterm.keyPress(Object.assign({}, ev, { altKey: true, keyCode: 64 })); // @ + xterm.keyPress(Object.assign({}, evKeyPress, { altKey: true, keyCode: 64 })); // @ // Firefox - xterm.keyPress(Object.assign({}, ev, { altKey: true, charCode: 64, keyCode: 0 })); - xterm.keyPress(Object.assign({}, ev, { altKey: true, keyCode: 92 })); // \ - xterm.keyPress(Object.assign({}, ev, { altKey: true, charCode: 92, keyCode: 0 })); - xterm.keyPress(Object.assign({}, ev, { altKey: true, keyCode: 124 })); // | - xterm.keyPress(Object.assign({}, ev, { altKey: true, charCode: 124, keyCode: 0 })); + xterm.keyPress(Object.assign({}, evKeyPress, { altKey: true, charCode: 64, keyCode: 0 })); + xterm.keyPress(Object.assign({}, evKeyPress, { altKey: true, keyCode: 92 })); // \ + xterm.keyPress(Object.assign({}, evKeyPress, { altKey: true, charCode: 92, keyCode: 0 })); + xterm.keyPress(Object.assign({}, evKeyPress, { altKey: true, keyCode: 124 })); // | + xterm.keyPress(Object.assign({}, evKeyPress, { altKey: true, charCode: 124, keyCode: 0 })); }); }); @@ -139,22 +145,22 @@ describe('xterm.js', function() { it('should not interfere with the alt + ctrl key on keyDown', function() { assert.equal( - xterm.keyDown(Object.assign({}, ev, { altKey: true, ctrlKey: true, keyCode: 81 })), + xterm.keyDown(Object.assign({}, evKeyDown, { altKey: true, ctrlKey: true, keyCode: 81 })), true ); assert.equal( - xterm.keyDown(Object.assign({}, ev, { altKey: true, ctrlKey: true, keyCode: 192 })), + xterm.keyDown(Object.assign({}, evKeyDown, { altKey: true, ctrlKey: true, keyCode: 192 })), true ); }); it('should interefere with the alt + ctrl + arrow keys', function() { assert.equal( - xterm.keyDown(Object.assign({}, ev, { altKey: true, ctrlKey: true, keyCode: 37 })), + xterm.keyDown(Object.assign({}, evKeyDown, { altKey: true, ctrlKey: true, keyCode: 37 })), false ); assert.equal( - xterm.keyDown(Object.assign({}, ev, { altKey: true, ctrlKey: true, keyCode: 39 })), + xterm.keyDown(Object.assign({}, evKeyDown, { altKey: true, ctrlKey: true, keyCode: 39 })), false ); }); @@ -172,22 +178,22 @@ describe('xterm.js', function() { }); xterm.keyPress( - Object.assign({}, ev, { altKey: true, ctrlKey: true, keyCode: 64 }) + Object.assign({}, evKeyPress, { altKey: true, ctrlKey: true, keyCode: 64 }) ); // @ xterm.keyPress( - Object.assign({}, ev, { altKey: true, ctrlKey: true, charCode: 64, keyCode: 0 }) + Object.assign({}, evKeyPress, { altKey: true, ctrlKey: true, charCode: 64, keyCode: 0 }) ); xterm.keyPress( - Object.assign({}, ev, { altKey: true, ctrlKey: true, keyCode: 92 }) + Object.assign({}, evKeyPress, { altKey: true, ctrlKey: true, keyCode: 92 }) ); // \ xterm.keyPress( - Object.assign({}, ev, { altKey: true, ctrlKey: true, charCode: 92, keyCode: 0 }) + Object.assign({}, evKeyPress, { altKey: true, ctrlKey: true, charCode: 92, keyCode: 0 }) ); xterm.keyPress( - Object.assign({}, ev, { altKey: true, ctrlKey: true, keyCode: 124 }) + Object.assign({}, evKeyPress, { altKey: true, ctrlKey: true, keyCode: 124 }) ); // | xterm.keyPress( - Object.assign({}, ev, { altKey: true, ctrlKey: true, charCode: 124, keyCode: 0 }) + Object.assign({}, evKeyPress, { altKey: true, ctrlKey: true, charCode: 124, keyCode: 0 }) ); }); }); From 0862fd1f59c986b650ce6a6fa14d946848736205 Mon Sep 17 00:00:00 2001 From: Paris Date: Wed, 13 Jul 2016 17:10:45 +0300 Subject: [PATCH 3/4] Fix thirdLevelKey clause --- src/xterm.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/xterm.js b/src/xterm.js index 7e420312..7bd6e5b4 100644 --- a/src/xterm.js +++ b/src/xterm.js @@ -4954,7 +4954,7 @@ (term.isMSWindows && ev.altKey && ev.ctrlKey && !ev.metaKey); // Don't invoke for arrows, pageDown, home, backspace, etc. (on non-keypress events) - return thirdLevelKey && (ev.type != 'keypress' || (!ev.keyCode || ev.keyCode > 47)); + return thirdLevelKey && ((ev.type != 'keypress') ? (!ev.keyCode || ev.keyCode > 47) : true); } function matchColor(r1, g1, b1) { From 42ec3b492a24b110007a73f5f8ff48a4e4d83737 Mon Sep 17 00:00:00 2001 From: Paris Date: Wed, 13 Jul 2016 17:53:33 +0300 Subject: [PATCH 4/4] Clarify `isThirdLevelShift` --- src/xterm.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/xterm.js b/src/xterm.js index 7bd6e5b4..d88ddf49 100644 --- a/src/xterm.js +++ b/src/xterm.js @@ -4953,8 +4953,12 @@ (term.isMac && ev.altKey && !ev.ctrlKey && !ev.metaKey) || (term.isMSWindows && ev.altKey && ev.ctrlKey && !ev.metaKey); + if (ev.type == 'keypress') { + return thirdLevelKey; + } + // Don't invoke for arrows, pageDown, home, backspace, etc. (on non-keypress events) - return thirdLevelKey && ((ev.type != 'keypress') ? (!ev.keyCode || ev.keyCode > 47) : true); + return thirdLevelKey && (!ev.keyCode || ev.keyCode > 47); } function matchColor(r1, g1, b1) {