From 97b60e64223f0242e3cd658e14fe4c151261300e Mon Sep 17 00:00:00 2001 From: Martin Meyerhoff Date: Sat, 11 Oct 2014 14:49:06 +0200 Subject: [PATCH] Fix buggy autocomplete behaviour for non-US keyboards This change moves the code for actually entering data from the keydown handler to the keypress handler, which can reliably catch the character entered (rather than the key pressed). This is my second attempt at https://meta.discourse.org/t/typing-shows-on-non-us-keyboard-layouts/20449 without doing dangerous refactoring. This does not fix the issue reported in https://meta.discourse.org/t/overly-aggresive-emoji-autocomplete/20691/7 as that is a different bug. --- .../discourse/lib/autocomplete.js.es6 | 112 +++++++----------- 1 file changed, 46 insertions(+), 66 deletions(-) diff --git a/app/assets/javascripts/discourse/lib/autocomplete.js.es6 b/app/assets/javascripts/discourse/lib/autocomplete.js.es6 index 6c61513379f..3a7e2380f44 100644 --- a/app/assets/javascripts/discourse/lib/autocomplete.js.es6 +++ b/app/assets/javascripts/discourse/lib/autocomplete.js.es6 @@ -6,44 +6,32 @@ export var CANCELLED_STATUS = "__CANCELLED"; -var shiftMap = []; -shiftMap[192] = "~"; -shiftMap[49] = "!"; -shiftMap[50] = "@"; -shiftMap[51] = "#"; -shiftMap[52] = "$"; -shiftMap[53] = "%"; -shiftMap[54] = "^"; -shiftMap[55] = "&"; -shiftMap[56] = "*"; -shiftMap[57] = "("; -shiftMap[48] = ")"; -shiftMap[109] = "_"; -shiftMap[107] = "+"; -shiftMap[219] = "{"; -shiftMap[221] = "}"; -shiftMap[220] = "|"; -shiftMap[59] = ":"; -shiftMap[222] = "\""; -shiftMap[188] = "<"; -shiftMap[190] = ">"; -shiftMap[191] = "?"; -shiftMap[32] = " "; +var keys = { + backSpace: 8, + tab: 9, + enter: 13, + shift: 16, + ctrl: 17, + alt: 18, + esc: 27, + space: 32, + leftWindows: 91, + rightWindows: 92, + pageUp: 33, + pageDown: 34, + end: 35, + home: 36, + leftArrow: 37, + upArrow: 38, + rightArrow: 39, + downArrow: 40, + insert: 45, + deleteKey: 46, + zero: 48, + a: 65, + z: 90, +}; -function mapKeyPressToActualCharacter(isShiftKey, characterCode) { - if ( characterCode === 27 || characterCode === 8 || characterCode === 9 || characterCode === 20 || characterCode === 16 || characterCode === 17 || characterCode === 91 || characterCode === 13 || characterCode === 92 || characterCode === 18 ) { return false; } - - // Lookup non-letter keypress while holding shift - if (isShiftKey && ( characterCode < 65 || characterCode > 90 )) { - return shiftMap[characterCode]; - } - - var stringValue = String.fromCharCode(characterCode); - if ( !isShiftKey ) { - stringValue = stringValue.toLowerCase(); - } - return stringValue; -} export default function(options) { var autocompletePlugin = this; @@ -271,17 +259,22 @@ export default function(options) { }); $(this).keypress(function(e) { + var caretPosition = Discourse.Utilities.caretPosition(me[0]), + term; if (!options.key) return; // keep hunting backwards till you hit a the @ key if (e.which === options.key.charCodeAt(0)) { - var caretPosition = Discourse.Utilities.caretPosition(me[0]); var prevChar = me.val().charAt(caretPosition - 1); if (!prevChar || /\s/.test(prevChar)) { completeStart = completeEnd = caretPosition; updateAutoComplete(options.dataSource("")); } + } else if ((completeStart !== null) && (e.charCode !== 0)) { + term = me.val().substring(completeStart + (options.key ? 1 : 0), caretPosition); + term += String.fromCharCode(e.charCode); + updateAutoComplete(options.dataSource(term)); } }); @@ -313,8 +306,8 @@ export default function(options) { if (!options.key) { completeStart = 0; } - if (e.which === 16) return; - if ((completeStart === null) && e.which === 8 && options.key) { + if (e.which === keys.shift) return; + if ((completeStart === null) && e.which === keys.backSpace && options.key) { c = Discourse.Utilities.caretPosition(me[0]); next = me[0].value[c]; c -= 1; @@ -339,7 +332,7 @@ export default function(options) { } // ESC - if (e.which === 27) { + if (e.which === keys.escape) { if (completeStart !== null) { closeAutocomplete(); return false; @@ -358,9 +351,9 @@ export default function(options) { // Keyboard codes! So 80's. switch (e.which) { - case 13: - case 39: - case 9: + case keys.enter: + case keys.rightArrow: + case keys.tab: if (!autocompleteOptions) return true; if (selectedOption >= 0 && (userToComplete = autocompleteOptions[selectedOption])) { completeTerm(userToComplete); @@ -370,14 +363,14 @@ export default function(options) { } e.stopImmediatePropagation(); return false; - case 38: + case keys.upArrow: selectedOption = selectedOption - 1; if (selectedOption < 0) { selectedOption = 0; } markSelected(); return false; - case 40: + case keys.downArrow: total = autocompleteOptions.length; selectedOption = selectedOption + 1; if (selectedOption >= total) { @@ -388,12 +381,10 @@ export default function(options) { } markSelected(); return false; - default: - // otherwise they're typing - let's search for it! + case keys.backSpace: completeEnd = caretPosition; - if (e.which === 8) { - caretPosition--; - } + caretPosition--; + if (caretPosition < 0) { closeAutocomplete(); if (isInput) { @@ -404,25 +395,14 @@ export default function(options) { } return true; } + term = me.val().substring(completeStart + (options.key ? 1 : 0), caretPosition); - if (e.which >= 48 && e.which <= 90) { - term += mapKeyPressToActualCharacter(e.shiftKey, e.which); - } else if (e.which === 187) { - term += "+"; - } else if (e.which === 189) { - term += (e.shiftKey) ? "_" : "-"; - } else if (e.which === 220) { - term += (e.shiftKey) ? "|" : "]"; - } else if (e.which === 222) { - term += (e.shiftKey) ? "\"" : "'"; - } else if (!e.which) { - /* fake event */ - } else if (e.which !== 8) { - term += ","; - } updateAutoComplete(options.dataSource(term)); return true; + default: + completeEnd = caretPosition; + return true; } } });