From 5fb6dd9bfaf6191393b7809fa0ac11b952a70a23 Mon Sep 17 00:00:00 2001 From: Sam Date: Mon, 13 Feb 2023 09:25:12 +1100 Subject: [PATCH] FIX: account for cursor drift when completing terms (#19660) Previously after uploads completed post raw would drift. If you autocompleted text after the upload stub got replaced it would insert in the wrong position. --- .../discourse/app/lib/autocomplete.js | 117 +++++++--- .../tests/unit/lib/autocomplete-test.js | 210 ++++++++++++++++++ 2 files changed, 292 insertions(+), 35 deletions(-) create mode 100644 app/assets/javascripts/discourse/tests/unit/lib/autocomplete-test.js diff --git a/app/assets/javascripts/discourse/app/lib/autocomplete.js b/app/assets/javascripts/discourse/app/lib/autocomplete.js index 3041ec78500..974856e5c29 100644 --- a/app/assets/javascripts/discourse/app/lib/autocomplete.js +++ b/app/assets/javascripts/discourse/app/lib/autocomplete.js @@ -91,7 +91,6 @@ export default function (options) { let autocompleteOptions = null; let selectedOption = null; let completeStart = null; - let completeEnd = null; let me = this; let div = null; let scrollElement = null; @@ -218,6 +217,8 @@ export default function (options) { } let completeTerm = async function (term) { + let completeEnd = null; + if (term) { if (isInput) { me.val(""); @@ -233,11 +234,31 @@ export default function (options) { if (term) { let text = me.val(); + // After completion is done our position for completeStart may have + // drifted. This can happen if the TEXTAREA changed out-of-band between + // the time autocomplete was first displayed and the time of completion + // Specifically this may happen due to uploads which inject a placeholder + // which is later replaced with a different length string. + let pos = guessCompletePosition({ completeTerm: true }); + + if ( + pos.completeStart !== undefined && + pos.completeEnd !== undefined + ) { + completeStart = pos.completeStart; + completeEnd = pos.completeEnd; + } else { + completeStart = completeEnd = caretPosition(me[0]); + } + + let space = + text.substring(completeEnd + 1, completeEnd + 2) === " " ? "" : " "; + text = text.substring(0, completeStart) + (options.preserveKey ? options.key || "" : "") + term + - " " + + space + text.substring(completeEnd + 1, text.length); me.val(text); @@ -536,7 +557,6 @@ export default function (options) { if (match) { completeStart = cp - match[0].length; - completeEnd = completeStart + match[0].length - 1; let term = match[0].substring(1, match[0].length); updateAutoComplete(dataSource(term, options)); } @@ -550,7 +570,7 @@ export default function (options) { checkTriggerRule() && (!prevChar || allowedLettersRegex.test(prevChar)) ) { - completeStart = completeEnd = cp - 1; + completeStart = cp - 1; updateAutoComplete(dataSource("", options)); } } @@ -560,8 +580,55 @@ export default function (options) { } } + function guessCompletePosition(opts) { + let prev, stopFound, term; + let prevIsGood = true; + let element = me[0]; + let backSpace = opts && opts.backSpace; + let completeTermOption = opts && opts.completeTerm; + + let caretPos = caretPosition(element); + + if (backSpace) { + caretPos -= 1; + } + + let start = null; + let end = null; + + let initialCaretPos = caretPos; + + while (prevIsGood && caretPos >= 0) { + caretPos -= 1; + prev = element.value[caretPos]; + + stopFound = prev === options.key; + + if (stopFound) { + prev = element.value[caretPos - 1]; + + if ( + checkTriggerRule({ backSpace }) && + (prev === undefined || allowedLettersRegex.test(prev)) + ) { + start = caretPos; + term = element.value.substring(caretPos + 1, initialCaretPos); + end = caretPos + term.length; + + break; + } + } + prevIsGood = !allowedLettersRegex.test(prev); + if (completeTermOption) { + prevIsGood ||= prev === " "; + } + } + + return { completeStart: start, completeEnd: end, term }; + } + function handleKeyDown(e) { - let c, i, initial, prev, prevIsGood, stopFound, term, total, userToComplete; + let i, term, total, userToComplete; let cp; if (e.ctrlKey || e.altKey || e.metaKey) { @@ -596,35 +663,12 @@ export default function (options) { } if (completeStart === null && e.which === keys.backSpace && options.key) { - c = caretPosition(me[0]); - c -= 1; - initial = c; - prevIsGood = true; + let position = guessCompletePosition({ backSpace: true }); + completeStart = position.completeStart; - while (prevIsGood && c >= 0) { - c -= 1; - prev = me[0].value[c]; - stopFound = prev === options.key; - - if (stopFound) { - prev = me[0].value[c - 1]; - - if ( - checkTriggerRule({ backSpace: true }) && - (!prev || allowedLettersRegex.test(prev)) - ) { - completeStart = c; - term = me[0].value.substring(c + 1, initial); - - if (!completeEnd) { - completeEnd = c + term.length; - } - - updateAutoComplete(dataSource(term, options)); - return true; - } - } - prevIsGood = /[a-zA-Z\.-]/.test(prev); + if (position.completeEnd) { + updateAutoComplete(dataSource(position.term, options)); + return true; } } @@ -685,6 +729,11 @@ export default function (options) { e.preventDefault(); return false; case keys.downArrow: + if (!autocompleteOptions) { + closeAutocomplete(); + return true; + } + total = autocompleteOptions.length; selectedOption = selectedOption + 1; if (selectedOption >= total) { @@ -700,7 +749,6 @@ export default function (options) { case keys.backSpace: autocompleteOptions = null; cp--; - completeEnd = cp; if (cp < 0) { closeAutocomplete(); @@ -724,7 +772,6 @@ export default function (options) { return true; default: autocompleteOptions = null; - completeEnd = cp; return true; } } diff --git a/app/assets/javascripts/discourse/tests/unit/lib/autocomplete-test.js b/app/assets/javascripts/discourse/tests/unit/lib/autocomplete-test.js new file mode 100644 index 00000000000..acfa61dfb2a --- /dev/null +++ b/app/assets/javascripts/discourse/tests/unit/lib/autocomplete-test.js @@ -0,0 +1,210 @@ +import { module, test } from "qunit"; +import autocomplete from "discourse/lib/autocomplete"; +import { compile } from "handlebars"; + +module("Unit | Utility | autocomplete", function (hooks) { + let elements = []; + + function textArea(value) { + let element = document.createElement("TEXTAREA"); + element.value = value; + document.getElementById("ember-testing").appendChild(element); + elements.push(element); + return element; + } + + function cleanup() { + elements.forEach((e) => { + e.remove(); + autocomplete.call($(e), { cancel: true }); + autocomplete.call($(e), "destroy"); + }); + elements = []; + } + + hooks.afterEach(function () { + cleanup(); + }); + + function simulateKey(element, key) { + let keyCode = key.charCodeAt(0); + + let bubbled = false; + let trackBubble = function () { + bubbled = true; + }; + + element.addEventListener("keydown", trackBubble); + + let keyboardEvent = new KeyboardEvent("keydown", { + key, + keyCode, + which: keyCode, + }); + + element.dispatchEvent(keyboardEvent); + + element.removeEventListener("keydown", trackBubble); + + if (bubbled) { + let pos = element.selectionStart; + let value = element.value; + // backspace + if (key === "\b") { + element.value = value.slice(0, pos - 1) + value.slice(pos); + element.selectionStart = pos - 1; + element.selectionEnd = pos - 1; + } else { + element.value = value.slice(0, pos) + key + value.slice(pos); + element.selectionStart = pos + 1; + element.selectionEnd = pos + 1; + } + } + + element.dispatchEvent( + new KeyboardEvent("keyup", { key, keyCode, which: keyCode }) + ); + } + + test("Autocomplete can account for cursor drift correctly", function (assert) { + let element = textArea(""); + let $element = $(element); + + autocomplete.call($element, { + key: "@", + dataSource: (term) => + ["test1", "test2"].filter((word) => word.includes(term)), + template: compile(`
+ +
`), + }); + + simulateKey(element, "@"); + simulateKey(element, "\r"); + + assert.strictEqual(element.value, "@test1 "); + assert.strictEqual(element.selectionStart, 7); + assert.strictEqual(element.selectionEnd, 7); + + simulateKey(element, "@"); + simulateKey(element, "2"); + simulateKey(element, "\r"); + + assert.strictEqual(element.value, "@test1 @test2 "); + assert.strictEqual(element.selectionStart, 14); + assert.strictEqual(element.selectionEnd, 14); + + element.selectionStart = 6; + element.selectionEnd = 6; + + simulateKey(element, "\b"); + simulateKey(element, "\b"); + simulateKey(element, "\r"); + + assert.strictEqual(element.value, "@test1 @test2 "); + assert.strictEqual(element.selectionStart, 7); + assert.strictEqual(element.selectionEnd, 7); + + // lets see that deleting last space triggers autocomplete + element.selectionStart = element.value.length; + element.selectionEnd = element.value.length; + simulateKey(element, "\b"); + let list = document.querySelectorAll("#ac-testing ul li"); + assert.strictEqual(list.length, 1); + + simulateKey(element, "\b"); + list = document.querySelectorAll("#ac-testing ul li"); + assert.strictEqual(list.length, 2); + + // close autocomplete + simulateKey(element, "\r"); + + // does not trigger by mistake at the start + element.value = "test"; + element.selectionStart = element.value.length; + element.selectionEnd = element.value.length; + + simulateKey(element, "\b"); + list = document.querySelectorAll("#ac-testing ul li"); + assert.strictEqual(list.length, 0); + }); + + test("Autocomplete can handle spaces", function (assert) { + let element = textArea(""); + let $element = $(element); + + autocomplete.call($element, { + key: "@", + dataSource: (term) => + [ + { username: "jd", name: "jane dale" }, + { username: "jb", name: "jack black" }, + ] + .filter((user) => { + return user.username.includes(term) || user.name.includes(term); + }) + .map((user) => user.username), + template: compile(`
+ +
`), + }); + + simulateKey(element, "@"); + simulateKey(element, "j"); + simulateKey(element, "a"); + simulateKey(element, "n"); + simulateKey(element, "e"); + simulateKey(element, " "); + simulateKey(element, "d"); + simulateKey(element, "\r"); + + assert.strictEqual(element.value, "@jd "); + }); + + test("Autocomplete can render on @", function (assert) { + let element = textArea("@"); + let $element = $(element); + + autocomplete.call($element, { + key: "@", + dataSource: () => ["test1", "test2"], + template: compile(`
+ +
`), + }); + + element.dispatchEvent(new KeyboardEvent("keydown", { key: "@" })); + element.dispatchEvent(new KeyboardEvent("keyup", { key: "@" })); + + let list = document.querySelectorAll("#ac-testing ul li"); + assert.strictEqual(2, list.length); + + let selected = document.querySelectorAll("#ac-testing ul li a.selected"); + assert.strictEqual(1, selected.length); + assert.strictEqual("test1", selected[0].innerText); + }); +});