From 10cf0da3d26b60575593d7896a9123d7ba7255bf Mon Sep 17 00:00:00 2001
From: mwiegand <martin.wiegand@seibert.group>
Date: Sat, 16 May 2026 20:49:23 +0200
Subject: [PATCH] fix(editor): capture-phase keydown + popup leak + cache
 warmup
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Addresses Critical #1 + Important #2/#3/#4 from the Task 9 code review.

CRITICAL — Tab/Enter were stolen by CodeJar before the popup handler
saw them. CodeJar registers its keydown listener during construction
(line ~159), so it ran first in bubble order: Tab handler
preventDefaulted and inserted 2 spaces, Enter handler preventDefaulted
+ stopPropagation'd (with leading indent), so the popup-accept either
ran on corrupted state or never fired at all. Fix: register the popup
listener with {capture: true} and call stopPropagation on the keys we
own — that way capture phase fires before CodeJar's bubble listener
and the key is fully consumed by the popup while it's visible. Normal
typing (popup hidden) early-returns without stopPropagation, so
CodeJar's tab-indent + enter-preserve-indent still work when there's
no autocomplete to accept.

IMPORTANT — destroy() leaked the popup <ul> into document.body. Each
mount/destroy cycle (e.g. modal close/reopen) left an orphan popup.
Fix: pop.remove() in destroy().

IMPORTANT — async refreshPopup could race in stale renders if the
first keystroke fired the vocab fetch and the second keystroke
captured a different ctx before the fetch resolved. Fix: warm the
cache with a fire-and-forget loadVocab(language) at mount, so the
first user keystroke hits cache. Eliminates the only realistic window
for the race.

IMPORTANT — acceptCompletion's Range.setStart could throw
IndexSizeError on pathological state (caret inside a tokenized span
where the fragment isn't fully upstream). Fix: try/catch the entire
DOM mutation block, log + dismiss on failure. Plus an inline comment
documenting the single-text-node invariant the current grammars hold.

Plan source updated for the capture-phase fix (most important for
future regeneration); the other fixes are smaller and only mirrored
into the actual code.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 .../plans/2026-05-16-textarea-code-editor.md  | 19 +++++-
 l4d2web/l4d2web/static/js/editor.js           | 63 ++++++++++++++-----
 2 files changed, 67 insertions(+), 15 deletions(-)

diff --git a/docs/superpowers/plans/2026-05-16-textarea-code-editor.md b/docs/superpowers/plans/2026-05-16-textarea-code-editor.md
index 82697ac..25e2b56 100644
--- a/docs/superpowers/plans/2026-05-16-textarea-code-editor.md
+++ b/docs/superpowers/plans/2026-05-16-textarea-code-editor.md
@@ -1065,26 +1065,43 @@ code.addEventListener("blur", function () {
   setTimeout(hidePopup, 100);
 });
 
+// IMPORTANT: capture-phase listener + stopPropagation.
+//
+// CodeJar registers its own keydown handler on this same element during
+// construction (line ~159 above). Its Tab handler unconditionally
+// preventDefaults and inserts 2 spaces; its Enter handler does the same
+// (+ stopPropagation when there's leading indent). Bubble-phase order is
+// registration order — CodeJar would consume Tab/Enter before our popup
+// handler ever sees them.
+//
+// Capture-phase listener fires first. Calling stopPropagation here
+// prevents CodeJar's bubble handler from running for the keys we own
+// while the popup is visible. Normal typing (popup hidden) early-returns
+// without stopPropagation so CodeJar's other behavior is preserved.
 code.addEventListener("keydown", function (e) {
   if (!popup || popup.style.display === "none" || !popupItems.length) return;
   if (e.key === "ArrowDown") {
     popupActive = (popupActive + 1) % Math.min(popupItems.length, 8);
     renderPopup(popup, popupItems, popupActive);
     e.preventDefault();
+    e.stopPropagation();
   } else if (e.key === "ArrowUp") {
     popupActive =
       (popupActive - 1 + Math.min(popupItems.length, 8)) %
       Math.min(popupItems.length, 8);
     renderPopup(popup, popupItems, popupActive);
     e.preventDefault();
+    e.stopPropagation();
   } else if (e.key === "Tab" || e.key === "Enter") {
     acceptCompletion(popupItems[popupActive]);
     e.preventDefault();
+    e.stopPropagation();
   } else if (e.key === "Escape") {
     hidePopup();
     e.preventDefault();
+    e.stopPropagation();
   }
-});
+}, true);
 ```
 
 - [ ] **Step 4: Manual smoke**
diff --git a/l4d2web/l4d2web/static/js/editor.js b/l4d2web/l4d2web/static/js/editor.js
index cadf868..68b6a12 100644
--- a/l4d2web/l4d2web/static/js/editor.js
+++ b/l4d2web/l4d2web/static/js/editor.js
@@ -169,6 +169,11 @@
 
     attachOnUpdate(jar);
 
+    // Warm the vocab cache so the first keystroke doesn't race a network
+    // fetch. Fire-and-forget; loadVocab already caches the empty result
+    // on failure so this is idempotent.
+    if (language !== "plain") loadVocab(language);
+
     let popup = null;
     let popupItems = [];
     let popupActive = 0;
@@ -201,19 +206,30 @@
         return;
       }
       // Replace the trailing word fragment with the chosen identifier.
-      const sel = window.getSelection();
-      const range = sel.getRangeAt(0);
-      range.setStart(range.endContainer, range.endOffset - ctx.fragment.length);
-      range.deleteContents();
-      range.insertNode(document.createTextNode(entry.name));
-      range.collapse(false);
-      sel.removeAllRanges();
-      sel.addRange(range);
-      // Force CodeJar to re-highlight + emit onUpdate.
-      // Use instance.jar (not bare `jar` closure) for consistency with
-      // setLanguage's tear-down-and-remount — the jar reference can be
-      // swapped at runtime.
-      instance.jar.updateCode(instance.jar.toString());
+      // Assumes the fragment lives in one text node — true for the current
+      // srccfg and bash grammars (identifiers tokenize as a single \b…\b
+      // chunk). If a future grammar splits identifiers across nodes,
+      // setStart(endContainer, endOffset - fragment.length) will land
+      // inside the wrong node and the deleteContents will corrupt text.
+      // The try/catch below converts IndexSizeError or any other DOM
+      // exception into a graceful popup-dismiss rather than a broken caret.
+      try {
+        const sel = window.getSelection();
+        const range = sel.getRangeAt(0);
+        range.setStart(range.endContainer, range.endOffset - ctx.fragment.length);
+        range.deleteContents();
+        range.insertNode(document.createTextNode(entry.name));
+        range.collapse(false);
+        sel.removeAllRanges();
+        sel.addRange(range);
+        // Force CodeJar to re-highlight + emit onUpdate.
+        // Use instance.jar (not bare `jar` closure) for consistency with
+        // setLanguage's tear-down-and-remount — the jar reference can be
+        // swapped at runtime.
+        instance.jar.updateCode(instance.jar.toString());
+      } catch (err) {
+        console.warn("[editor] acceptCompletion failed; dismissing popup", err);
+      }
       hidePopup();
     }
 
@@ -251,26 +267,44 @@
       setTimeout(hidePopup, 100);
     });
 
+    // IMPORTANT: capture-phase listener.
+    //
+    // CodeJar registers its own keydown handler on this same element during
+    // construction (window.CodeJar at line ~159 above), and it uses
+    // preventDefault + insert-tab-spaces on Tab and preventDefault +
+    // stopPropagation on Enter (with leading indent). Bubble-phase order =
+    // registration order: CodeJar runs first and consumes those keys before
+    // our popup handler ever sees them.
+    //
+    // Capture phase fires before bubble. By calling stopPropagation here,
+    // we prevent CodeJar's bubble handler from running for the keys we own
+    // while the popup is visible. Normal typing (popup hidden) still flows
+    // through to CodeJar unchanged because we early-return without
+    // stopPropagation in that case.
     code.addEventListener("keydown", function (e) {
       if (!popup || popup.style.display === "none" || !popupItems.length) return;
       if (e.key === "ArrowDown") {
         popupActive = (popupActive + 1) % Math.min(popupItems.length, 8);
         renderPopup(popup, popupItems, popupActive);
         e.preventDefault();
+        e.stopPropagation();
       } else if (e.key === "ArrowUp") {
         popupActive =
           (popupActive - 1 + Math.min(popupItems.length, 8)) %
           Math.min(popupItems.length, 8);
         renderPopup(popup, popupItems, popupActive);
         e.preventDefault();
+        e.stopPropagation();
       } else if (e.key === "Tab" || e.key === "Enter") {
         acceptCompletion(popupItems[popupActive]);
         e.preventDefault();
+        e.stopPropagation();
       } else if (e.key === "Escape") {
         hidePopup();
         e.preventDefault();
+        e.stopPropagation();
       }
-    });
+    }, true);
 
     const instance = {
       textarea,
@@ -312,6 +346,7 @@
       },
       destroy: function () {
         instance.jar.destroy();
+        if (popup) popup.remove();
         shell.remove();
         textarea.style.display = "";
         delete textarea._codeEditor;