From 17bccc1ade341e2f9d7495059fa07cee3248f5db Mon Sep 17 00:00:00 2001
From: golem <golem@zavage.net>
Date: Sat, 1 Aug 2026 23:21:45 -0600
Subject: [PATCH] fix: derive the model maxlength and document the frame-focus
 cases

maxlength was hard-coded to 200 in the markup while MAX_MODEL_CHARS lives
in lib/defaults.js; lowering the constant would have left the input
accepting values mergeSettings() silently discards. It is now set from the
constant at render time.

Also records the hasFocus() gate's four manual frame cases in AGENTS.md,
including the findbar trade-off, and notes that fitToBudget()'s budget
guarantee assumes mergeSettings-validated input.
---
 AGENTS.md            | 19 +++++++++++++++----
 lib/build-url.js     |  3 +++
 options/options.html |  4 +++-
 options/options.js   |  7 ++++++-
 4 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/AGENTS.md b/AGENTS.md
index 38ca2ae..5cd9fae 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -50,10 +50,21 @@ After `npm test`, use `npm run dev` and confirm:
   in both light and dark themes and at a narrow window width, and values
   persist across a restart.
 - The right-click **Ask ChatGPT** item appears only when text is selected.
-- `Alt+Shift+G` acts on the selection, including a selection inside an iframe,
-  and opens a blank chat when nothing is selected or on a restricted page such
-  as `about:config`. Press the key to test this — `commands.getAll()` reports a
-  shortcut as registered even when Firefox has silently overridden it.
+- `Alt+Shift+G` acts on the selection and opens a blank chat when nothing is
+  selected or on a restricted page such as `about:config`. Press the key to test
+  this — `commands.getAll()` reports a shortcut as registered even when Firefox
+  has silently overridden it. The injected snippet is gated on
+  `document.hasFocus()`, so cover all four frame cases:
+  - Select in the top frame → the top-frame text arrives.
+  - Select inside an iframe → the iframe text arrives.
+  - Select in the top frame, *then* select inside an iframe → the iframe text
+    arrives. `hasFocus()` is true for a focused frame and all its ancestors, so
+    if the stale top-frame text arrives instead, exclude ancestors by also
+    checking that `document.activeElement` is not the frame element.
+  - Press `Ctrl+F`, find a term, then press the shortcut without clicking back
+    into the page → a blank chat is expected. Firefox leaves focus in the
+    findbar, so no content frame reports focus. This is a deliberate trade: a
+    stale cross-origin frame silently seeding the prompt was the worse failure.
 
 Inspect ZIP contents whenever packaging rules or assets change; `test/` and
 `docs/` must never appear, not even as empty directory entries.
diff --git a/lib/build-url.js b/lib/build-url.js
index 10e3e08..398828b 100644
--- a/lib/build-url.js
+++ b/lib/build-url.js
@@ -56,6 +56,9 @@ function lastWhitespaceIndex(text) {
 }
 
 // Returns the longest prefix of `prompt` whose finished URL fits the budget.
+// The guarantee assumes `settings` came through mergeSettings(): only `q` is
+// ever trimmed, so a model longer than MAX_MODEL_CHARS could blow the budget on
+// its own and leave no room for `q` at all. Every real caller goes via load().
 // Slicing happens by CODE POINT on the decoded string, so a surrogate pair can
 // never be split and a percent-escape can never be severed (encoding happens
 // afterwards, via URLSearchParams).
diff --git a/options/options.html b/options/options.html
index 8920222..2ebd7cd 100644
--- a/options/options.html
+++ b/options/options.html
@@ -9,7 +9,9 @@
 <form id="settings" autocomplete="off">
   <div class="field">
     <label for="model">Model <span class="tag">experimental</span></label>
-    <input type="text" id="model" placeholder="auto" spellcheck="false" maxlength="200" />
+    <!-- maxlength is set from MAX_MODEL_CHARS in options.js, so the input and
+         mergeSettings() can never disagree about the bound. -->
+    <input type="text" id="model" placeholder="auto" spellcheck="false" />
     <p class="hint">
       ChatGPT may ignore this and use your account default. Leave blank to
       always use your account default.
diff --git a/options/options.js b/options/options.js
index cc1ef03..371a192 100644
--- a/options/options.js
+++ b/options/options.js
@@ -1,4 +1,4 @@
-import { DEFAULT_SETTINGS } from "../lib/defaults.js";
+import { DEFAULT_SETTINGS, MAX_MODEL_CHARS } from "../lib/defaults.js";
 import { load, save } from "../lib/settings.js";
 
 const SAVE_DEBOUNCE_MS = 400;
@@ -59,6 +59,11 @@ function scheduleSave() {
   saveTimer = setTimeout(() => persist(collect()), SAVE_DEBOUNCE_MS);
 }
 
+// Derived rather than hard-coded in the markup: if the two ever disagreed, the
+// input would accept a model that mergeSettings() then silently discards,
+// blanking the setting with no explanation.
+field("model").maxLength = MAX_MODEL_CHARS;
+
 // Module scripts are deferred, so the DOM is already parsed here.
 render(await load());