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.

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]);
+      let position = guessCompletePosition({ backSpace: true });
-      c -= 1;
+      completeStart = position.completeStart;
-      initial = c;
-      prevIsGood = true;
 
-      while (prevIsGood && c >= 0) {
+      if (position.completeEnd) {
-        c -= 1;
+        updateAutoComplete(dataSource(position.term, options));
-        prev = me[0].value[c];
+        return true;
-        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);
       }
     }
 
@@ -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;
       }
     }

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(`<div id='ac-testing' class='autocomplete ac-test'>
+  <ul>
+    {{#each options as |option|}}
+      <li>
+        <a href>
+          {{option}}
+        </a>
+      </li>
+    {{/each}}
+  </ul>
+</div>`),
+    });
+
+    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(`<div id='ac-testing' class='autocomplete ac-test'>
+  <ul>
+    {{#each options as |option|}}
+      <li>
+        <a href>
+          {{option}}
+        </a>
+      </li>
+    {{/each}}
+  </ul>
+</div>`),
+    });
+
+    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(`<div id='ac-testing' class='autocomplete ac-test'>
+  <ul>
+    {{#each options as |option|}}
+      <li>
+        <a href>
+          {{option}}
+        </a>
+      </li>
+    {{/each}}
+  </ul>
+</div>`),
+    });
+
+    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);
+  });
+});