feat: add selection popup, accounts, and custom styles - #1
Open
yattdev wants to merge 3 commits into
Open
Conversation
Selecting text now pops up a ✍ bubble immediately instead of requiring a right-click, styles are user-editable/addable instead of hard-coded, and proofreading routes through whichever connected account (Google, OpenAI, xAI, Ollama, or Chrome Built-in AI) the user has activated. Along the way, fixes several bugs found while wiring this up: Replace silently no-oping after a popup click moved focus, narrow editable-field detection, missing input events on contenteditable replace, off-screen result panels near viewport edges, missing guards on the Try Again path, a context menu that never picked up style changes, and API keys that were sitting in synced (not local) storage. Adds a small Vitest suite (test/) for the new storage/account/style logic; no build step is required to load the extension itself.
…njection Found by driving content/selection.js and content/content.js in a real headless Chrome over CDP — this surface had no automated coverage, and all four bugs below were invisible to the lib/-only unit suite. - Text selected inside <input>/<textarea> lives in the field's own UA shadow tree, so getSelection().isCollapsed is true for it even though toString() returns the text. evaluateSelection() bailed on that check before ever showing a bubble or capturing the target, which meant the bubble never appeared in editable fields and captureTarget's 'input' branch was unreachable dead code — silently breaking Replace, the bug this change set set out to fix. Read the field's own selection instead. - Every mouseup re-ran evaluateSelection(), which called showBubble() unconditionally; showBubble() tears the host down and rebuilds it with the picker closed. The picker therefore slammed shut ~50ms after the click that opened it and a style pill could never be reached. Reuse the existing bubble when the selection is unchanged. - The popup on/off toggle and per-site disable list skipped the target capture, not just the bubble. The context menu stays available when the bubble is off, so its Replace action silently no-opped while still reporting "Text replaced". Capture unconditionally; gate only the bubble, and report the real outcome. - A custom style's icon was interpolated into innerHTML unescaped (the label beside it was escaped), so an icon of <img src=x onerror=...> executed in the content script's world on every result panel. - The disabled-sites box is free text and users paste URLs; the shipped matcher never parsed them, so "https://example.com" blocked nothing. Mirror the URL normalization already unit-tested in lib/settings.js. Adds test/e2e/ (16 cases, dependency-free CDP client, ephemeral ports) and test/boundaries.test.js (9 cases). npm test 48/48, npm run test:e2e 16/16.
1. Scope proofread messages to the originating frame — chrome.tabs.sendMessage had no frameId, and content_scripts declares all_frames:true, so every send broadcast to every frame in the tab. A page with an embedded iframe rendered one duplicate result panel per frame. runProofread now threads info.frameId (context menu) / sender.frameId (message-based entry points) through to every sendMessage call. 2. Serialize styleOrder read-modify-write in lib/styles.js. Concurrent addStyle() calls each read the same stale styleOrder and the last write won, orphaning the others' pf_style_* records. Adds an in-process async lock around every styleOrder mutation and combines addStyle's two writes into one atomic set() — fixes same-context races (e.g. Promise.all from one Options tab); cross-tab/cross-device races still need a compare-and-set write, noted in the code comment. 3. Give the Gemini API-key fallback an actual UI and retry path. The error message already told users to add a fallback key but no field for it existed anywhere. Adds accounts.js storage for it (shared cred_gemini local key), an options.js field on the Google account row, and an automatic retry in llm-providers.js gemini() when the OAuth call is rejected with 401/403. 4. Restore the per-account model override UI dropped from v1. connectAccount already accepted a model, but nothing in the options page could set one after connecting. Adds updateAccountModel() and a Model field on every non-builtin account row. 5. Coalesce concurrent rebuildContextMenu() calls. addStyle's two writes (now one, per #2) each fired storage.onChanged, running two overlapping removeAll()+create() passes and throwing duplicate-id runtime.lastError noise. rebuildContextMenu() now serializes into a single in-flight run followed by at most one coalesced follow-up. 6. Fix README: built-in styles can be disabled but not deleted (AC25) — the Features list said "deleted" too. 7. Single source of truth for default models. lib/llm-providers.js had its own hardcoded per-provider defaults (gpt-4o, grok-3, ...) that could drift from PROVIDER_REGISTRY.defaultModel. Adds resolveAccountModel() in accounts.js as the one place that resolves an account's effective model; llm-providers.js no longer guesses. npm test 77/77, npm run test:e2e 16/16 (unchanged, content scripts weren't touched by this round).
yattdev
marked this pull request as ready for review
August 6, 2026 03:49
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
Selecting text now pops up a ✍ bubble immediately instead of requiring a right-click, proofreading routes through whichever account (Chrome Built-in AI, Google, OpenAI, xAI, or Ollama) the user has activated, and the 8 built-in proofread styles are now user-editable with support for adding custom ones.
Changes
content/selection.jswatches for text selections (debouncedselectionchange/mouseup/keyup), gated by a global on/off toggle, a minimum-character threshold, and a per-site disable list, and shows a small ✍ bubble that expands into a style picker on click. The right-click context menu is preserved and stays in sync with the same style list.lib/accounts.jsadds a provider-keyed account model (one per provider, 5 max — Chrome Built-in AI, Google Gemini, OpenAI, xAI, Ollama). Exactly one account is active at a time; every proofread (bubble, context menu, Try Again) routes through it via a singlerunProofread()entry point in the service worker. API keys live inchrome.storage.local, neverchrome.storage.sync.lib/styles.jsmoves the 8 built-in prompts out of a frozen literal into per-itemchrome.storage.syncrecords (avoids the 8KB per-item quota), with full CRUD — enable/disable, reorder, edit, reset-to-default (built-ins), add/delete (custom) — exposed from a rewritten Options page.search/email/url/tel); missinginputevents on contenteditable replace; off-screen result panels near viewport edges; an unguarded Try Again path that threw raw API errors instead of a friendly message; a context menu that never picked up style changes without a reinstall; a legacy plaintext API key left behind in synced storage after migration; selection inside<input>/<textarea>not being detected (lives in the field's own UA shadow tree); the bubble's picker slamming shut ~50ms after opening; a stored style icon being interpolated intoinnerHTMLunescaped (XSS via a custom style); and the disabled-sites list not accepting pasted full URLs.lib/settings.jsadds an idempotent v1→v2 migration that seeds the built-in styles and converts an existing flat{provider, apiKey, ...}blob into a connected, active account, then removes the legacy plaintext keys from sync.Motivation
Per the task: proofreading previously required a right-click every time, account linking was scaffolded but non-functional (missing OAuth scopes/host permissions, no real account model), and the proofread styles were hard-coded with no way for users to customize them.
Testing
npm test— 48/48 passing (Vitest). Covers the storage/account/style logic inlib/againsttest/chrome-mock.js, an in-memorychrome.storage.sync/.localshim that enforces the real 8KB-per-item and ~100KB total sync quotas, plus boundary/escaping cases.npm run test:e2e— 16/16 passing. Drivescontent/selection.jsandcontent/content.jsin a real headless Chrome over the DevTools Protocol (realSelection, real shadow DOM, real synthesized mouse input), since this sandbox's Chrome build refuses--load-extension.Breaking Changes
None for the extension's behavior. Existing users' flat v1 settings (
provider/apiKey/etc.) are migrated automatically and idempotently into the new account model on first load after upgrading.