From c9983b5f4b1b4efa534e0de96f74e1276199856c Mon Sep 17 00:00:00 2001 From: heznpc Date: Mon, 11 May 2026 01:12:30 +0900 Subject: [PATCH] =?UTF-8?q?chore:=20v3.5.13=20=E2=80=94=20quality=20pass?= =?UTF-8?q?=20(sidebar=20split,=20type=20checking,=20i18n=20CI,=20prod=20l?= =?UTF-8?q?og=20strip,=20protected-terms=20hardening)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Multi-front cleanup from a 2026-05-10 review covering the items the recent hotfix train (v3.5.6 → v3.5.12) repeatedly touched: long single-purpose files, missing static type signal, no enforced i18n drift check, and a few production-realistic edge cases in shared helpers. Refactor — `src/content/sidebar-chat.js` (1224 → 815 lines, –33%) split into: * `chat-render.js` (formatResponse / applyInline / sanitizeHtml) * `chat-history.js` (IDB store + history sub-panel) * Sub-panel state (savedChatHTML, history/flashcard flags) hoisted to `_sb._chat.state` so the modules share one source of truth. * Manifest content_scripts.js order updated; format-response.test.js follows the functions to chat-render.js. Added: * `tsconfig.json` (allowJs + checkJs, strict:false) for IDE / `tsc --noEmit`. * `src/lib/_sb-typedef.js` — JSDoc-only contract for `window._sb` and the new `_chat` / ProtectedTermsApi / GeminiBlockApi sub-namespaces. * `scripts/check-i18n-keys.js` + CI step + `npm run check:i18n` — validates _locales/ key parity and constants.js label-dict shape across the 11 premium languages (handles flat, lang-outer, and section-outer shapes; skips language-agnostic lookups like SKILLBRIDGE_MODEL_LABELS). * `docs/E2E_PLAN.md` — working spec for the deferred Playwright suite (6 priority targets) so the next pass doesn't restart from zero. Fixed: * `restoreProtectedTerms` defends against null/undefined/non-string input, empty-string wrong-forms (would have inserted between every char via `replaceAll('', x)`), and self-mapping entries. * Background SW + content-script handlers now warn on misrouted `action`/`type` discriminators (catches the v3.5.6 cache-cleanup class of bug at first occurrence in dev instead of silent fallthrough). Build: * `build-bundle.js` esbuild gains `pure: ['console.debug','console.info']` so production bundles strip dev-only logs (verified 0 occurrences in dist/bundled). `console.warn`/`error` preserved on purpose. Tests: 343/343 (+7 new, +6 in protected-terms covering the new edge cases). --- .github/workflows/ci.yml | 3 + CHANGELOG.md | 27 ++ docs/E2E_PLAN.md | 91 +++++++ manifest.json | 4 +- package.json | 3 +- scripts/build-bundle.js | 8 + scripts/check-i18n-keys.js | 187 +++++++++++++ src/background/background.js | 28 ++ src/content/chat-history.js | 301 +++++++++++++++++++++ src/content/chat-render.js | 195 ++++++++++++++ src/content/content.js | 12 + src/content/sidebar-chat.js | 485 +++------------------------------- src/lib/_sb-typedef.js | 90 +++++++ src/lib/protected-terms.js | 30 ++- tests/format-response.test.js | 11 +- tests/protected-terms.test.js | 44 +++ tsconfig.json | 21 ++ 17 files changed, 1083 insertions(+), 457 deletions(-) create mode 100644 docs/E2E_PLAN.md create mode 100644 scripts/check-i18n-keys.js create mode 100644 src/content/chat-history.js create mode 100644 src/content/chat-render.js create mode 100644 src/lib/_sb-typedef.js create mode 100644 tsconfig.json diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 59dff98..df12e41 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -39,6 +39,9 @@ jobs: - name: Check glossary consistency run: node scripts/check-glossary.js + - name: Check i18n key coverage + run: node scripts/check-i18n-keys.js + - name: Check shared constants sync run: node scripts/check-bg-sync.js diff --git a/CHANGELOG.md b/CHANGELOG.md index cc077ca..7a2d0e8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,33 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ## [Unreleased] +## [3.5.13] - 2026-05-11 + +### Refactor +- Split `src/content/sidebar-chat.js` (1224 → 815 lines, –33%) into three modules with a shared `window._sb._chat` namespace: + - `src/content/chat-render.js` (new) — `formatResponse`, `applyInline`, `sanitizeHtml`. Pure DOM-free markdown→HTML + the trusted-structure HTML sanitizer used by both the live chat bubble and the history detail view. + - `src/content/chat-history.js` (new) — IndexedDB conversation store, history sub-panel, detail view. Owns `chat-history` IDB store + the panel UI; calls back into sidebar-chat through `_sb._chat.{closeSubPanel,formatResponse,sanitizeHtml}`. + - `src/content/sidebar-chat.js` keeps panel infrastructure (sidebar inject, chat input, sub-panel state machinery, focus trap, flashcards, PDF export, send-chat stream). + - Sub-panel state (`savedChatHTML`, `historyPanelOpen`, `flashcardPanelOpen`) hoisted to `_sb._chat.state` so history and flashcard modules share one source of truth instead of duplicating local flags. + - `manifest.json` `content_scripts.js` order updated to load `chat-render.js` before `sidebar-chat.js` (which now exposes the `_sb._chat` panel helpers) and `chat-history.js` after (which consumes them). + - `tests/format-response.test.js` follows `formatResponse`/`applyInline` to its new home in `chat-render.js`. + +### Added +- `src/lib/_sb-typedef.js` — JSDoc-only contract for the `window._sb` shared namespace and the `_chat`/`ProtectedTermsApi`/`GeminiBlockApi` sub-namespaces. Not loaded at runtime (no manifest entry); picked up by IDEs and `tsc --noEmit` via the new `tsconfig.json`. +- `tsconfig.json` — `allowJs` + `checkJs` for IDE/local type checking against existing JSDoc. `strict: false` so the rollout doesn't surface a wave of pre-existing nullability warnings; tighten incrementally as files migrate. +- `scripts/check-i18n-keys.js` + `npm run check:i18n` — validates (1) every locale under `_locales/` matches the English `messages.json` key set so Chrome doesn't silently fall back, and (2) every `*_LABELS` / `*_GREETINGS` / `*_PLACEHOLDERS` dictionary in `src/lib/constants.js` is shape-consistent across the 11 premium languages, both for flat `{ en, ko, … }` maps and section-outer `{ key: { en, ko, … } }` maps. Wired into the CI `validate` job. +- `docs/E2E_PLAN.md` — working spec for the deferred Playwright suite. Records the 6 priority coverage targets (golden translation, SPA mid-stream, cache-cleanup alarm, stream-cancel, protected-terms, panel switch) so the next person who picks it up doesn't restart from zero. + +### Fixed +- `restoreProtectedTerms(text)` now defends against three production-realistic edge cases that previously corrupted output or crashed: `null`/`undefined`/non-string input (returned safe fallback instead of throwing on `.includes`); empty-string wrong-forms in the dictionary (`String.prototype.replaceAll('', x)` would have inserted the correct form between every char); and self-mapping entries where a wrong-form equals its correct form (silent no-op cycle bloating the hot loop on long pages). +- Background SW + content-script handlers gain a `_logMisroutedMessage` defensive log: if a `{ action: ... }`-shaped message reaches the background (or a `{ type: ... }` reaches a content script) it warns loudly with the discriminator. Catches the v3.5.6 cache-cleanup class of bug at first occurrence in dev rather than silently falling through "Unknown action". + +### Build +- `scripts/build-bundle.js` esbuild now passes `pure: ['console.debug', 'console.info']` to the content + background bundles. With `minify: true` already on, those calls get tree-shaken from production output (verified 0 occurrences in `dist/bundled/*.bundle.js`). `console.warn` / `console.error` deliberately preserved so real degradation/errors still reach DevTools. + +### Tests +- `tests/protected-terms.test.js` (+6 tests): null/undefined/non-string input safety, idempotence (`f(f(x)) == f(x)`), empty-wrong-form skip, self-mapping skip, non-string-array-element skip. Total now 24 tests against the protected-terms helper. + ## [3.5.12] - 2026-05-11 ### Fixed diff --git a/docs/E2E_PLAN.md b/docs/E2E_PLAN.md new file mode 100644 index 0000000..9906b97 --- /dev/null +++ b/docs/E2E_PLAN.md @@ -0,0 +1,91 @@ +# E2E Test Plan (Deferred) + +Status: **planned**, not yet implemented. Tracked from the v3.5.13 quality +pass on 2026-05-10. The current Jest suite covers pure functions and message +shapes well, but never exercises the real load order / inter-module wiring +(content.js → banners.js → chat-render.js → sidebar-chat.js → chat-history.js, +plus the page-bridge / background trip). + +This file is the working spec for what to add when we get there. Update it +when scope changes; do not let it bit-rot silently. + +## Why we need this + +The recent v3.5.6 → v3.5.12 hotfix train surfaced a recurring class of bug +that no unit test could catch: + +- v3.5.6 cache-cleanup `action` vs `type` discriminator mismatch +- v3.5.7 translator + bridge race during SPA navigation +- v3.5.8 `fetchWithRetry` 4xx fail-fast bug +- v3.5.9 sidebar stream cancel / IDB resilience +- v3.5.10 YouTube subtitle timer leak + +All of these required a real extension load + a real Skilljar-shaped page to +reproduce. We've been catching them in production. Playwright + a minimal +fixture page would close that gap. + +## Stack + +- **Playwright** (`@playwright/test`) — Chromium + Firefox launchers, native + extension loading via `--load-extension=` / `--disable-extensions-except`. +- **Local fixture pages** under `tests/e2e/fixtures/` — static HTML that + mimics the Skilljar DOM shape (lesson body, quiz form, header, video + container) without depending on a live skilljar.com URL. +- **Network stubbing** via Playwright `route()` for: + - `https://translate.googleapis.com/*` — return canned translations + - `https://api.github.com/*` — return canned latest-release JSON + - `https://api.puter.com/*` (or whatever the page-bridge fetches) — return + canned Gemini stream chunks + +## Coverage targets (priority order) + +1. **Golden page translation** — load fixture, set language=ko in popup, + assert visible body text becomes Korean and the cache hit path works on + second visit. +2. **SPA navigation mid-translation** — start translating, push a new URL via + `history.pushState`, assert no stale translation lands in the new page + (verifies `_langGeneration` invariant). +3. **Cache cleanup alarm** — fast-forward Chrome `alarms` to fire + `cache-cleanup` after a stale entry is in IDB, assert it's purged. +4. **Stream cancel on sidebar close** — open chat, start a (stubbed) stream, + close sidebar, assert `AbortController.signal.aborted === true` and no + half-saved IDB entry. +5. **Protected Terms restoration** — fixture page with "클로드" in body, + target=Korean, assert it ends as "Claude" after the verify step. +6. **History panel + flashcard panel switch** — open history, then open + flashcards from chat without restoring chat first; assert we don't blow + away `savedChatHTML` (this was a near-miss bug during the v3.5.13 split). + +## File layout (when implemented) + +``` +tests/e2e/ + fixtures/ + skilljar-lesson.html + skilljar-quiz.html + youtube-embed.html + helpers/ + extension.ts — launch helper (loads dist/firefox into Playwright) + network-stubs.ts — GT / GitHub / Puter route handlers + golden-translation.spec.ts + spa-navigation.spec.ts + cache-cleanup.spec.ts + stream-cancel.spec.ts + protected-terms.spec.ts + panel-switch.spec.ts +playwright.config.ts +``` + +Add `e2e:install` (`playwright install chromium`) and `e2e` (`playwright test`) +scripts to `package.json`. Wire into a separate CI job — keep the fast `test` +job under 30 s by not running E2E on every push. + +## Open questions + +- Do we ship a real Skilljar lesson HTML snapshot, or hand-craft a minimal + fixture? Real snapshot catches more selector regressions; minimal fixture + is cheaper to maintain. +- Service worker lifecycle in Playwright — Chromium suspends it; we may need + `chrome.runtime.getBackgroundPage()` (deprecated in MV3) replacement via + Playwright's `serviceWorkers()` API. +- Firefox MV3 differences — separate spec file or per-test guards? diff --git a/manifest.json b/manifest.json index e2fcb25..3e344c8 100644 --- a/manifest.json +++ b/manifest.json @@ -2,7 +2,7 @@ "manifest_version": 3, "name": "__MSG_extName__", "description": "__MSG_extDescription__", - "version": "3.5.12", + "version": "3.5.13", "minimum_chrome_version": "120", "author": "SkillBridge Contributors", "homepage_url": "https://github.com/heznpc/skillbridge", @@ -28,7 +28,7 @@ "content_scripts": [ { "matches": ["https://*.skilljar.com/*"], - "js": ["src/lib/browser-polyfill.js", "src/lib/selectors.js", "src/lib/constants.js", "src/lib/translator.js", "src/lib/youtube-subtitles.js", "src/lib/protected-terms.js", "src/lib/gemini-block.js", "src/content/content.js", "src/content/banners.js", "src/content/code-comments.js", "src/content/header-controls.js", "src/content/text-selection.js", "src/content/sidebar-chat.js", "src/content/keyboard-shortcuts.js"], + "js": ["src/lib/browser-polyfill.js", "src/lib/selectors.js", "src/lib/constants.js", "src/lib/translator.js", "src/lib/youtube-subtitles.js", "src/lib/protected-terms.js", "src/lib/gemini-block.js", "src/content/content.js", "src/content/banners.js", "src/content/code-comments.js", "src/content/header-controls.js", "src/content/text-selection.js", "src/content/chat-render.js", "src/content/sidebar-chat.js", "src/content/chat-history.js", "src/content/keyboard-shortcuts.js"], "css": ["src/content/content.css"], "run_at": "document_idle" } diff --git a/package.json b/package.json index 5ba2fd2..884958d 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "skillbridge", - "version": "3.5.12", + "version": "3.5.13", "private": true, "scripts": { "test": "jest --verbose", @@ -14,6 +14,7 @@ "check:selectors": "node scripts/check-selectors.js", "check:dicts": "node scripts/check-dicts.js", "check:sync": "node scripts/check-bg-sync.js", + "check:i18n": "node scripts/check-i18n-keys.js", "docs": "node scripts/generate-docs.js", "lint": "eslint src/ tests/ scripts/", "lint:fix": "eslint src/ tests/ scripts/ --fix", diff --git a/scripts/build-bundle.js b/scripts/build-bundle.js index 503f44d..0ee21e7 100644 --- a/scripts/build-bundle.js +++ b/scripts/build-bundle.js @@ -23,6 +23,12 @@ async function build() { const contentEntryPath = path.join(DIST, '_content-entry.js'); fs.writeFileSync(contentEntryPath, contentEntry); + // `pure: ['console.debug', 'console.info']` lets minify drop those calls + // entirely from the production bundle (their return values are unused, so + // marking them pure tree-shakes the call-sites). `console.warn`/`error` are + // preserved on purpose so real degradation/errors still reach DevTools. + const PROD_PURE = ['console.debug', 'console.info']; + await esbuild.build({ entryPoints: [contentEntryPath], outfile: path.join(DIST, 'content.bundle.js'), @@ -30,6 +36,7 @@ async function build() { minify: true, target: ['chrome120'], format: 'iife', + pure: PROD_PURE, }); // Bundle background service worker @@ -40,6 +47,7 @@ async function build() { minify: true, target: ['chrome120'], format: 'iife', + pure: PROD_PURE, }); // Bundle CSS diff --git a/scripts/check-i18n-keys.js b/scripts/check-i18n-keys.js new file mode 100644 index 0000000..da0a2c3 --- /dev/null +++ b/scripts/check-i18n-keys.js @@ -0,0 +1,187 @@ +#!/usr/bin/env node +/** + * SkillBridge — i18n Key Coverage Check + * + * Two checks: + * (1) `_locales//messages.json` files all share the same key set as the + * English baseline. Chrome rejects an extension if `default_locale` keys + * are missing in other locales used at install time, and divergence + * silently falls back to English without warning. + * (2) The label dictionaries declared in `src/lib/constants.js` + * (POPUP_LABELS, A11Y_LABELS, etc.) are object-shaped and every language + * sub-object exposes the same key set, so nested labels never quietly + * drop a sub-key on translation. + * + * Exit code 1 on hard mismatches; warnings are non-fatal. + * + * Usage: node scripts/check-i18n-keys.js + */ + +const fs = require('fs'); +const path = require('path'); + +const ROOT = path.resolve(__dirname, '..'); +const LOCALES_DIR = path.join(ROOT, '_locales'); +const CONSTANTS_FILE = path.join(ROOT, 'src', 'lib', 'constants.js'); + +let errors = 0; +let warnings = 0; + +const log = { + pass: (m) => console.log(' ✓', m), + warn: (m) => { + warnings++; + console.warn(' ⚠', m); + }, + fail: (m) => { + errors++; + console.error(' ✗', m); + }, +}; + +// ==================== CHECK 1: _locales coverage ==================== + +console.log('\n--- Check 1: _locales//messages.json key coverage ---'); +const baselineLocale = 'en'; +const baselinePath = path.join(LOCALES_DIR, baselineLocale, 'messages.json'); +if (!fs.existsSync(baselinePath)) { + log.fail(`Baseline locale missing: ${baselineLocale}/messages.json`); +} else { + const baseline = JSON.parse(fs.readFileSync(baselinePath, 'utf8')); + const baselineKeys = new Set(Object.keys(baseline)); + const locales = fs.readdirSync(LOCALES_DIR).filter((d) => fs.statSync(path.join(LOCALES_DIR, d)).isDirectory()); + + for (const lang of locales) { + if (lang === baselineLocale) continue; + const file = path.join(LOCALES_DIR, lang, 'messages.json'); + if (!fs.existsSync(file)) { + log.fail(`Missing messages.json for locale: ${lang}`); + continue; + } + let data; + try { + data = JSON.parse(fs.readFileSync(file, 'utf8')); + } catch (e) { + log.fail(`Invalid JSON in ${lang}/messages.json: ${e.message}`); + continue; + } + const keys = new Set(Object.keys(data)); + const missing = [...baselineKeys].filter((k) => !keys.has(k)); + const extra = [...keys].filter((k) => !baselineKeys.has(k)); + if (missing.length || extra.length) { + const parts = []; + if (missing.length) parts.push(`missing ${missing.join(', ')}`); + if (extra.length) parts.push(`extra ${extra.join(', ')}`); + log.fail(`${lang}: ${parts.join('; ')}`); + } + } + if (errors === 0) log.pass(`All ${locales.length} locales match ${baselineLocale} key set`); +} + +// ==================== CHECK 2: constants.js label dict shape ==================== + +console.log('\n--- Check 2: constants.js label dictionaries ---'); + +// constants.js references selectors.js' globals at the top, so load both into +// the same Function scope before extracting the LABEL dicts. +const SELECTORS_FILE = path.join(ROOT, 'src', 'lib', 'selectors.js'); +const selectorsSrc = fs.readFileSync(SELECTORS_FILE, 'utf8'); +const constantsSrc = fs.readFileSync(CONSTANTS_FILE, 'utf8'); + +function matchExports(src) { + const names = []; + const re = /^const ([A-Z_][A-Z0-9_]*(?:LABELS|GREETINGS|PLACEHOLDERS|UI|QUESTIONS|DESCRIPTIONS))\s*=/gm; + let m; + while ((m = re.exec(src)) !== null) names.push(m[1]); + return names; +} + +let dicts; +try { + const names = matchExports(constantsSrc); + // Both source files reference `window` at the top; provide a stub so the + // top-level `if (typeof window !== 'undefined')` guards don't trip. + const runner = new Function('window', `${selectorsSrc}\n${constantsSrc}\nreturn { ${names.join(', ')} };`); + dicts = runner({}); +} catch (e) { + log.fail(`Failed to load constants.js: ${e.message}`); + printSummary(); + process.exit(errors > 0 ? 1 : 0); +} + +const expectedLangs = new Set(['en', 'ko', 'ja', 'zh-CN', 'zh-TW', 'es', 'fr', 'de', 'pt-BR', 'ru', 'vi']); + +/** + * Validate a flat lang map: { en: 'x', ko: 'x', ... }. + * @returns {boolean} true if at least one expected lang is present. + */ +function looksLikeLangMap(obj) { + if (!obj || typeof obj !== 'object') return false; + return Object.keys(obj).some((k) => expectedLangs.has(k)); +} + +function checkLangCoverage(label, dict) { + const missing = [...expectedLangs].filter((l) => !(l in dict)); + if (missing.length) log.warn(`${label}: missing language(s) ${missing.join(', ')}`); +} + +function checkLangOuter(name, dict) { + // { en: object|string, ko: object|string, ... } + checkLangCoverage(name, dict); + const enValue = dict.en; + if (enValue && typeof enValue === 'object' && !Array.isArray(enValue)) { + const baseline = new Set(Object.keys(enValue)); + for (const lang of Object.keys(dict)) { + const v = dict[lang]; + if (!v || typeof v !== 'object' || Array.isArray(v)) { + log.fail(`${name}.${lang}: expected object (matching .en), got ${typeof v}`); + continue; + } + const sub = new Set(Object.keys(v)); + const miss = [...baseline].filter((k) => !sub.has(k)); + const extra = [...sub].filter((k) => !baseline.has(k)); + if (miss.length) log.fail(`${name}.${lang}: missing sub-keys ${miss.join(', ')}`); + if (extra.length) log.warn(`${name}.${lang}: extra sub-keys ${extra.join(', ')}`); + } + } +} + +function checkSectionOuter(name, dict) { + // { sectionA: { en, ko, ... }, sectionB: {...}, ... } + // Skip if no value is an object (e.g. SKILLBRIDGE_MODEL_LABELS is a flat + // language-agnostic lookup, not an i18n dict). + const hasObjectValues = Object.values(dict).some((v) => v && typeof v === 'object'); + if (!hasObjectValues) return; + for (const [section, langMap] of Object.entries(dict)) { + if (!langMap || typeof langMap !== 'object') continue; + if (!looksLikeLangMap(langMap)) { + // Nested deeper — recurse one level + for (const [sub, deeper] of Object.entries(langMap)) { + if (looksLikeLangMap(deeper)) checkLangCoverage(`${name}.${section}.${sub}`, deeper); + } + continue; + } + checkLangCoverage(`${name}.${section}`, langMap); + } +} + +for (const [name, dict] of Object.entries(dicts)) { + if (!dict || typeof dict !== 'object') { + log.warn(`${name}: not an object, skipping`); + continue; + } + if (looksLikeLangMap(dict)) { + checkLangOuter(name, dict); + } else { + checkSectionOuter(name, dict); + } +} + +if (errors === 0) log.pass(`All label dictionaries shape-consistent`); + +printSummary(); +process.exit(errors > 0 ? 1 : 0); + +function printSummary() { + console.log(`\n${errors} error(s), ${warnings} warning(s)`); +} diff --git a/src/background/background.js b/src/background/background.js index 211c0d7..7ed083f 100644 --- a/src/background/background.js +++ b/src/background/background.js @@ -214,6 +214,30 @@ chrome.runtime.onInstalled.addListener((details) => { } }); +// ==================== MESSAGE DISPATCH CONVENTION ==================== +// +// All cross-context messages use ONE of two discriminator fields: +// +// { type: 'SCREAMING_SNAKE' } — addressed to the background worker +// (FETCH_URL, GOOGLE_TRANSLATE, ...) +// { action: 'camelCase' } — addressed to a content script +// (cacheCleanup, setLanguage, toggleSidebar, ...) +// +// Mixing the two (action→bg or type→content) was the v3.5.6 cache-cleanup +// bug. The `__messageDispatchSanityCheck` below catches a recurrence in dev +// builds where the wrong discriminator reaches the wrong handler. + +function _logMisroutedMessage(msg) { + if (msg && typeof msg === 'object' && 'action' in msg && !('type' in msg)) { + // Got an `action`-shaped message at the background — almost certainly a + // copy-paste from the popup→content path. Real bg messages use `type`. + console.warn( + '[SkillBridge BG] Unhandled `action`-shaped message — should this go to a content script instead?', + msg.action, + ); + } +} + // Message handlers chrome.runtime.onMessage.addListener((msg, sender, sendResponse) => { // Verify sender is this extension @@ -305,6 +329,10 @@ chrome.runtime.onMessage.addListener((msg, sender, sendResponse) => { .catch((err) => sendResponse({ ok: false, error: err.message })); return true; } + + // Fell through every `msg.type === ...` branch — surface anything that + // looks like a misrouted content-script message instead of swallowing it. + _logMisroutedMessage(msg); }); // Badge to show active language diff --git a/src/content/chat-history.js b/src/content/chat-history.js new file mode 100644 index 0000000..c77314c --- /dev/null +++ b/src/content/chat-history.js @@ -0,0 +1,301 @@ +/** + * SkillBridge — Tutor Conversation History (IndexedDB + panel UI) + * + * Owns the `chat-history` IDB store, the history sub-panel, and the detail + * view. Talks back into sidebar-chat.js through `_sb._chat` for sub-panel + * state machinery (savedChatHTML, closeSubPanel, bindChatInputEvents) and + * to chat-render.js for sanitizeHtml / formatResponse. + * + * Loaded after content.js + chat-render.js + sidebar-chat.js (which sets up + * `_sb._chat`). Toggling the panel is invoked from sidebar-chat.js's + * "history" button click handler via `_sb._chat.toggleHistoryPanel`. + */ + +(function () { + 'use strict'; + + const sb = window._sb; + if (!sb) { + console.warn('[SkillBridge] chat-history: _sb not ready'); + return; + } + // chat-render.js + sidebar-chat.js must have loaded first. + if (!sb._chat || !sb._chat.sanitizeHtml || !sb._chat.formatResponse) { + console.warn('[SkillBridge] chat-history: _sb._chat not ready (chat-render.js missing?)'); + return; + } + + let historyDb = null; + + function openHistoryDb() { + return new Promise((resolve, reject) => { + if (historyDb) return resolve(historyDb); + const req = indexedDB.open(HISTORY_DB_NAME, 1); + req.onupgradeneeded = (e) => { + const db = e.target.result; + if (!db.objectStoreNames.contains(HISTORY_STORE)) { + const store = db.createObjectStore(HISTORY_STORE, { keyPath: 'id', autoIncrement: true }); + store.createIndex('timestamp', 'timestamp'); + store.createIndex('chapter', 'chapter'); + } + }; + req.onsuccess = (e) => { + historyDb = e.target.result; + // If another tab (or a future extension update) bumps the schema, + // close this connection so it doesn't block the upgrade. Without + // this, every subsequent transaction() throws InvalidStateError. + historyDb.onversionchange = () => { + historyDb.close(); + historyDb = null; + }; + resolve(historyDb); + }; + req.onerror = () => reject(req.error); + }); + } + + async function saveConversation(question, answer, lang) { + try { + const db = await openHistoryDb(); + const chapter = document.querySelector('h1')?.textContent?.trim() || 'Unknown'; + const entry = { + question, + answer, + lang, + chapter, + timestamp: Date.now(), + url: location.href, + }; + const ok = await _addHistoryEntry(db, entry); + if (!ok) { + // First add hit the IDB quota — prune oldest 20 then retry once. + // If the prune freed less than expected (some deletes silently + // failed under the cursor) the retry can still fail; we then + // double the prune count once before giving up. Two-shot retry + // is bounded so a sticky quota can't infinite-loop. + const pruned = await pruneOldHistory(db, 20); + let okAfterPrune = await _addHistoryEntry(db, entry); + if (!okAfterPrune && pruned > 0) { + await pruneOldHistory(db, 40); + okAfterPrune = await _addHistoryEntry(db, entry); + } + if (!okAfterPrune) { + console.warn('[SkillBridge] Chat history save failed after prune+retry — quota may be stuck'); + } + } + } catch (e) { + console.warn('[SkillBridge] Failed to save conversation:', e); + } + } + + function _addHistoryEntry(db, entry) { + return new Promise((resolve) => { + const tx = db.transaction(HISTORY_STORE, 'readwrite'); + const req = tx.objectStore(HISTORY_STORE).add(entry); + req.onsuccess = () => resolve(true); + req.onerror = (e) => { + const isQuota = e.target.error?.name === 'QuotaExceededError'; + if (isQuota) { + console.warn('[SkillBridge] Chat history quota exceeded — will prune and retry'); + } else { + console.warn('[SkillBridge] Chat history add failed:', e.target.error?.name); + } + resolve(false); + }; + }); + } + + function pruneOldHistory(db, target = 20) { + return new Promise((resolve) => { + const tx = db.transaction(HISTORY_STORE, 'readwrite'); + const store = tx.objectStore(HISTORY_STORE); + const idx = store.index('timestamp'); + const req = idx.openCursor(); + let deleted = 0; + req.onsuccess = (e) => { + const cursor = e.target.result; + if (cursor && deleted < target) { + // Track per-delete failures — without this the count claims + // success even when the runtime silently aborted some deletes. + const delReq = cursor.delete(); + delReq.onsuccess = () => { + deleted++; + }; + delReq.onerror = () => { + console.warn('[SkillBridge] history prune: delete failed at cursor', cursor.primaryKey); + }; + cursor.continue(); + } + }; + // Resolve with the actually-deleted count when the whole transaction + // commits (or fails), so the caller knows whether to escalate. + tx.oncomplete = () => resolve(deleted); + tx.onerror = () => resolve(deleted); + tx.onabort = () => resolve(deleted); + }); + } + + async function getConversations(limit = SKILLBRIDGE_LIMITS.HISTORY) { + try { + const db = await openHistoryDb(); + return new Promise((resolve) => { + const tx = db.transaction(HISTORY_STORE, 'readonly'); + const idx = tx.objectStore(HISTORY_STORE).index('timestamp'); + const results = []; + const req = idx.openCursor(null, 'prev'); + req.onsuccess = (e) => { + const cursor = e.target.result; + if (cursor && results.length < limit) { + results.push(cursor.value); + cursor.continue(); + } else { + resolve(results); + } + }; + req.onerror = () => resolve([]); + }); + } catch { + return []; + } + } + + async function clearAllHistory() { + try { + const db = await openHistoryDb(); + const tx = db.transaction(HISTORY_STORE, 'readwrite'); + tx.objectStore(HISTORY_STORE).clear(); + tx.oncomplete = () => { + const listEl = document.getElementById('si18n-history-list'); + if (listEl) { + listEl.innerHTML = `
${sb.t(HISTORY_LABELS.historyCleared)}
`; + } + }; + } catch (e) { + console.warn('[SkillBridge] Failed to clear history:', e); + } + } + + function toggleHistoryPanel() { + const chatPanel = document.getElementById('si18n-panel-chat'); + if (!chatPanel) return; + + const state = sb._chat.state; + + if (state.historyPanelOpen) { + sb._chat.closeSubPanel(); + return; + } + if (state.flashcardPanelOpen) sb._chat.closeSubPanel(); + + state.historyPanelOpen = true; + state.savedChatHTML = chatPanel.innerHTML; + chatPanel.innerHTML = ` +
+ + ${sb.t(HISTORY_LABELS.title)} + +
+
+
${sb.t(HISTORY_LABELS.loading)}
+
+ `; + + document.getElementById('si18n-history-back')?.addEventListener('click', sb._chat.closeSubPanel); + document.getElementById('si18n-history-clear')?.addEventListener('click', () => { + if (confirm(sb.t(HISTORY_LABELS.clearHistory) + '?')) clearAllHistory(); + }); + loadHistoryList(); + } + + async function loadHistoryList() { + const listEl = document.getElementById('si18n-history-list'); + if (!listEl) return; + + const conversations = await getConversations(); + if (conversations.length === 0) { + listEl.innerHTML = `
${sb.t(HISTORY_LABELS.empty)}
`; + return; + } + + const grouped = {}; + for (const conv of conversations) { + const ch = conv.chapter || 'Other'; + if (!grouped[ch]) grouped[ch] = []; + grouped[ch].push(conv); + } + + let html = ''; + for (const [chapter, convs] of Object.entries(grouped)) { + html += `
${sb.escapeHtml(chapter)}
`; + for (const conv of convs) { + const preview = + conv.question.length > SKILLBRIDGE_LIMITS.HISTORY_PREVIEW + ? conv.question.slice(0, SKILLBRIDGE_LIMITS.HISTORY_PREVIEW) + '…' + : conv.question; + const time = new Date(conv.timestamp).toLocaleDateString(undefined, { + month: 'short', + day: 'numeric', + hour: '2-digit', + minute: '2-digit', + }); + html += ` +
+
${sb.escapeHtml(preview)}
+
${time}
+
+ `; + } + } + listEl.innerHTML = sb._chat.sanitizeHtml(html); + + // Event delegation instead of per-item listeners + listEl.addEventListener('click', (e) => { + const item = e.target.closest('.si18n-history-item'); + if (item) showConversationDetail(item.dataset.id); + }); + } + + async function showConversationDetail(id) { + try { + const db = await openHistoryDb(); + const tx = db.transaction(HISTORY_STORE, 'readonly'); + const req = tx.objectStore(HISTORY_STORE).get(Number(id)); + req.onsuccess = () => { + const conv = req.result; + if (!conv) return; + const listEl = document.getElementById('si18n-history-list'); + if (!listEl) return; + const time = conv.timestamp ? new Date(conv.timestamp).toLocaleString() : ''; + const chapter = conv.chapter ? sb.escapeHtml(conv.chapter) : ''; + let metaHtml = ''; + if (chapter || time) { + metaHtml = `
`; + if (chapter) metaHtml += `${chapter}`; + if (time) metaHtml += `${time}`; + metaHtml += `
`; + } + listEl.innerHTML = sb._chat.sanitizeHtml(` +
+ ${metaHtml} +
+
${sb.escapeHtml(conv.question)}
+
+
+
${sb._chat.formatResponse(conv.answer)}
+
+
+ `); + }; + } catch (e) { + console.warn('[SkillBridge] Failed to load conversation:', e); + } + } + + // Expose for sidebar-chat.js (history button binding) and chat-render.js + // tests. saveConversation runs after every successful Gemini response. + sb._chat.saveConversation = saveConversation; + sb._chat.toggleHistoryPanel = toggleHistoryPanel; +})(); diff --git a/src/content/chat-render.js b/src/content/chat-render.js new file mode 100644 index 0000000..7ccbe2e --- /dev/null +++ b/src/content/chat-render.js @@ -0,0 +1,195 @@ +/** + * SkillBridge — Chat response rendering helpers. + * + * Pure (DOM-free for the markdown half) functions used by the sidebar chat, + * conversation-history detail view, and any other surface that needs to + * convert Gemini's markdown into safe HTML. + * + * Loaded after content.js (which constructs `_sb`) and before + * sidebar-chat.js / chat-history.js — both consume these via `sb._chat`. + * + * Exports (on `window._sb._chat`): + * - formatResponse(text) → safe HTML string + * - applyInline(escapedText) → HTML with bold/italic/code spans applied + * - sanitizeHtml(html) → string stripped to the chat-allowlist of tags/attrs + */ + +(function () { + 'use strict'; + + const sb = window._sb; + if (!sb) { + console.warn('[SkillBridge] chat-render: _sb not ready'); + return; + } + + /** + * Convert Gemini-style markdown into HTML. + * Input is fully HTML-escaped first so any markdown captured groups can + * be inserted without re-escaping (see {@link applyInline}). + * @param {string} text + * @returns {string} + */ + function formatResponse(text) { + const escaped = sb.escapeHtml(text); + + // Ensure markdown block elements start on new lines + // (avoid lookbehind for wider browser compatibility) + const normalized = escaped + .replace(/([^\n#])(#{2,3}\s)/g, '$1\n$2') + .replace(/([^\n])(-\s)/g, '$1\n$2') + .replace(/([^\n])(\d+[.)]\s)/g, '$1\n$2'); + + const lines = normalized.split('\n'); + const out = []; + let listBuf = []; + let listOrdered = false; + let paraBuf = []; + + const flushList = () => { + if (!listBuf.length) return; + const tag = listOrdered ? 'ol' : 'ul'; + out.push(`<${tag}>${listBuf.map((t) => `
  • ${applyInline(t)}
    • `).join('')}`); + listBuf = []; + }; + const flushPara = () => { + if (!paraBuf.length) return; + out.push(`

    ${applyInline(paraBuf.join('
    '))}

    `); + paraBuf = []; + }; + + for (const line of lines) { + const trimmed = line.trim(); + if (!trimmed) { + flushList(); + flushPara(); + continue; + } + const hMatch = trimmed.match(/^(#{2,3})\s+(.+)/); + if (hMatch) { + flushList(); + flushPara(); + out.push(`

    ${applyInline(hMatch[2])}

    `); + continue; + } + const ulMatch = trimmed.match(/^[-*]\s+(.*)/); + if (ulMatch) { + if (listBuf.length && listOrdered) flushList(); + listOrdered = false; + flushPara(); + listBuf.push(ulMatch[1]); + continue; + } + const olMatch = trimmed.match(/^\d+[.)]\s+(.*)/); + if (olMatch) { + if (listBuf.length && !listOrdered) flushList(); + listOrdered = true; + flushPara(); + listBuf.push(olMatch[1]); + continue; + } + flushList(); + paraBuf.push(trimmed); + } + flushList(); + flushPara(); + return out.join(''); + } + + function applyInline(text) { + // Input is already HTML-escaped by formatResponse — do NOT re-escape captured groups + return text + .replace(/\*\*(.*?)\*\*/g, (_, g) => '' + g + '') + .replace(/\*(.*?)\*/g, (_, g) => '' + g + '') + .replace(/`(.*?)`/g, (_, g) => '' + g + ''); + } + + /** + * Strip dangerous tags and attributes from trusted-structure HTML. + * Keeps only the tags used by our own formatResponse / history rendering. + * + * Allowlist note: `style` was previously allowed but enables CSS exfil + * (`background:url(attacker)`) and clickjack overlays via attacker-influenced + * content. Use class-based styling instead. + * + * @param {string} html + * @returns {string} + */ + function sanitizeHtml(html) { + const ALLOWED_TAGS = new Set([ + 'div', + 'span', + 'p', + 'h3', + 'ul', + 'ol', + 'li', + 'strong', + 'em', + 'code', + 'br', + 'button', + 'svg', + 'polyline', + 'path', + 'circle', + ]); + const ALLOWED_ATTRS = new Set([ + 'class', + 'id', + 'data-id', + 'data-question', + 'title', + 'aria-label', + 'role', + // SVG presentational attributes + 'width', + 'height', + 'viewBox', + 'fill', + 'stroke', + 'stroke-width', + 'stroke-linecap', + 'stroke-linejoin', + 'cx', + 'cy', + 'r', + 'd', + 'points', + ]); + const parser = new DOMParser(); + const doc = parser.parseFromString(html, 'text/html'); + + function walk(node) { + const children = Array.from(node.childNodes); + for (const child of children) { + if (child.nodeType === Node.ELEMENT_NODE) { + const tag = child.tagName.toLowerCase(); + if (!ALLOWED_TAGS.has(tag)) { + child.remove(); + continue; + } + // Strip disallowed attributes (including event handlers) + for (const attr of Array.from(child.attributes)) { + if (!ALLOWED_ATTRS.has(attr.name) || attr.name.startsWith('on')) { + child.removeAttribute(attr.name); + } + } + walk(child); + } + } + } + + walk(doc.body); + return doc.body.innerHTML; + } + + // Reserve the sub-namespace; sidebar-chat.js will fill in its half. + sb._chat = sb._chat || {}; + sb._chat.formatResponse = formatResponse; + sb._chat.applyInline = applyInline; + sb._chat.sanitizeHtml = sanitizeHtml; + + // Back-compat: existing callers in sidebar-chat.js use the old global name. + sb.formatResponse = formatResponse; +})(); diff --git a/src/content/content.js b/src/content/content.js index 45e10e2..09312f6 100644 --- a/src/content/content.js +++ b/src/content/content.js @@ -315,6 +315,18 @@ chrome.runtime.onMessage.addListener(handleMessage); function handleMessage(request, sender, sendResponse) { + // Catch the inverse of background.js' guard: if a `type`-shaped message + // (intended for the bg worker) somehow reached the content script, we + // would otherwise silently fall through to "Unknown action" and the + // sender just sees a generic failure. Warn loudly in dev so the + // misroute is obvious. + if (request && typeof request === 'object' && 'type' in request && !('action' in request)) { + console.warn( + '[SkillBridge] Content received `type`-shaped message — should this go to background?', + request.type, + ); + } + if (!isReady && request.action === 'translatePage') { pendingActions.push({ request, sendResponse }); sendResponse({ success: true, queued: true }); diff --git a/src/content/sidebar-chat.js b/src/content/sidebar-chat.js index 775ce3f..6d6e6f0 100644 --- a/src/content/sidebar-chat.js +++ b/src/content/sidebar-chat.js @@ -12,10 +12,19 @@ return; } - let historyDb = null; - let historyPanelOpen = false; + // Sub-panel state shared with chat-history.js (and, eventually, + // chat-flashcards.js when that gets extracted). Kept on `sb._chat.state` + // so any module loaded after content.js can read/mutate the same object + // without us re-introducing a tangle of ad-hoc globals. + sb._chat = sb._chat || {}; + sb._chat.state = sb._chat.state || { + savedChatHTML: null, + historyPanelOpen: false, + flashcardPanelOpen: false, + }; + const _state = sb._chat.state; + let scrollRAF = null; - let savedChatHTML = null; let isSending = false; let _rawSectionsCache = null; // Cached raw JSON for section-specific flashcards let _rawSectionsLang = null; // Language of the cached data @@ -145,7 +154,7 @@ function bindSidebarEvents() { document.getElementById('si18n-close')?.addEventListener('click', toggleSidebar); - document.getElementById('si18n-history-btn')?.addEventListener('click', toggleHistoryPanel); + document.getElementById('si18n-history-btn')?.addEventListener('click', () => sb._chat.toggleHistoryPanel?.()); document.getElementById('si18n-fc-btn')?.addEventListener('click', toggleFlashcardPanel); document.getElementById('si18n-pdf-btn')?.addEventListener('click', exportLessonPDF); bindChatInputEvents(); @@ -311,7 +320,7 @@ } } if (bubble) { - bubble.innerHTML = formatResponse(fullText); + bubble.innerHTML = sb._chat.formatResponse(fullText); scrollToBottom(messages); } }, @@ -321,7 +330,7 @@ if (bubble && !signal.aborted) { bubble.classList.remove('si18n-streaming-cursor'); const answerText = lastStreamedText?.trim() || bubble.textContent?.trim() || ''; - if (answerText) saveConversation(text, answerText, sb.currentLang); + if (answerText) sb._chat.saveConversation?.(text, answerText, sb.currentLang); } } catch (err) { // AbortError is expected when the user navigates away mid-stream. @@ -355,450 +364,27 @@ scrollToBottom(messages); } - // ============================================================ - // MARKDOWN RESPONSE FORMATTING - // ============================================================ - - function formatResponse(text) { - const escaped = sb.escapeHtml(text); - - // Ensure markdown block elements start on new lines - // (avoid lookbehind for wider browser compatibility) - const normalized = escaped - .replace(/([^\n#])(#{2,3}\s)/g, '$1\n$2') - .replace(/([^\n])(-\s)/g, '$1\n$2') - .replace(/([^\n])(\d+[.)]\s)/g, '$1\n$2'); - - const lines = normalized.split('\n'); - const out = []; - let listBuf = []; - let listOrdered = false; - let paraBuf = []; - - const flushList = () => { - if (!listBuf.length) return; - const tag = listOrdered ? 'ol' : 'ul'; - out.push(`<${tag}>${listBuf.map((t) => `
  • ${applyInline(t)}
    • `).join('')}`); - listBuf = []; - }; - const flushPara = () => { - if (!paraBuf.length) return; - out.push(`

    ${applyInline(paraBuf.join('
    '))}

    `); - paraBuf = []; - }; - - for (const line of lines) { - const trimmed = line.trim(); - if (!trimmed) { - flushList(); - flushPara(); - continue; - } - const hMatch = trimmed.match(/^(#{2,3})\s+(.+)/); - if (hMatch) { - flushList(); - flushPara(); - out.push(`

    ${applyInline(hMatch[2])}

    `); - continue; - } - const ulMatch = trimmed.match(/^[-*]\s+(.*)/); - if (ulMatch) { - if (listBuf.length && listOrdered) flushList(); - listOrdered = false; - flushPara(); - listBuf.push(ulMatch[1]); - continue; - } - const olMatch = trimmed.match(/^\d+[.)]\s+(.*)/); - if (olMatch) { - if (listBuf.length && !listOrdered) flushList(); - listOrdered = true; - flushPara(); - listBuf.push(olMatch[1]); - continue; - } - flushList(); - paraBuf.push(trimmed); - } - flushList(); - flushPara(); - return out.join(''); - } - - function applyInline(text) { - // Input is already HTML-escaped by formatResponse — do NOT re-escape captured groups - return text - .replace(/\*\*(.*?)\*\*/g, (_, g) => '' + g + '') - .replace(/\*(.*?)\*/g, (_, g) => '' + g + '') - .replace(/`(.*?)`/g, (_, g) => '' + g + ''); - } - - // ============================================================ - // SIMPLE HTML SANITIZER (no external dependency) - // ============================================================ - - /** - * Strip dangerous tags and attributes from trusted-structure HTML. - * Keeps only the tags used by our own formatResponse / history rendering. - */ - function sanitizeHtml(html) { - const ALLOWED_TAGS = new Set([ - 'div', - 'span', - 'p', - 'h3', - 'ul', - 'ol', - 'li', - 'strong', - 'em', - 'code', - 'br', - 'button', - 'svg', - 'polyline', - 'path', - 'circle', - ]); - const ALLOWED_ATTRS = new Set([ - 'class', - 'id', - 'data-id', - 'data-question', - // `style` was previously allowed but enables CSS exfil - // (`background:url(attacker)`) and clickjack overlays via attacker- - // influenced content. Use class-based styling instead. - 'title', - 'aria-label', - 'role', - // SVG presentational attributes - 'width', - 'height', - 'viewBox', - 'fill', - 'stroke', - 'stroke-width', - 'stroke-linecap', - 'stroke-linejoin', - 'cx', - 'cy', - 'r', - 'd', - 'points', - ]); - const parser = new DOMParser(); - const doc = parser.parseFromString(html, 'text/html'); - - function walk(node) { - const children = Array.from(node.childNodes); - for (const child of children) { - if (child.nodeType === Node.ELEMENT_NODE) { - const tag = child.tagName.toLowerCase(); - if (!ALLOWED_TAGS.has(tag)) { - child.remove(); - continue; - } - // Strip disallowed attributes (including event handlers) - for (const attr of Array.from(child.attributes)) { - if (!ALLOWED_ATTRS.has(attr.name) || attr.name.startsWith('on')) { - child.removeAttribute(attr.name); - } - } - walk(child); - } - } - } - - walk(doc.body); - return doc.body.innerHTML; - } + // Markdown rendering + sanitizer were extracted to chat-render.js. + // IndexedDB history (saveConversation, toggleHistoryPanel, …) was extracted + // to chat-history.js. Both attach their public surface onto `sb._chat`. // ============================================================ - // TUTOR CONVERSATION HISTORY (IndexedDB) + // SUB-PANEL STATE MACHINERY (shared with chat-history.js / flashcards) // ============================================================ - function openHistoryDb() { - return new Promise((resolve, reject) => { - if (historyDb) return resolve(historyDb); - const req = indexedDB.open(HISTORY_DB_NAME, 1); - req.onupgradeneeded = (e) => { - const db = e.target.result; - if (!db.objectStoreNames.contains(HISTORY_STORE)) { - const store = db.createObjectStore(HISTORY_STORE, { keyPath: 'id', autoIncrement: true }); - store.createIndex('timestamp', 'timestamp'); - store.createIndex('chapter', 'chapter'); - } - }; - req.onsuccess = (e) => { - historyDb = e.target.result; - // If another tab (or a future extension update) bumps the schema, - // close this connection so it doesn't block the upgrade. Without - // this, every subsequent transaction() throws InvalidStateError. - historyDb.onversionchange = () => { - historyDb.close(); - historyDb = null; - }; - resolve(historyDb); - }; - req.onerror = () => reject(req.error); - }); - } - - async function saveConversation(question, answer, lang) { - try { - const db = await openHistoryDb(); - const chapter = document.querySelector('h1')?.textContent?.trim() || 'Unknown'; - const entry = { - question, - answer, - lang, - chapter, - timestamp: Date.now(), - url: location.href, - }; - const ok = await _addHistoryEntry(db, entry); - if (!ok) { - // First add hit the IDB quota — prune oldest 20 then retry once. - // If the prune freed less than expected (some deletes silently - // failed under the cursor) the retry can still fail; we then - // double the prune count once before giving up. Two-shot retry - // is bounded so a sticky quota can't infinite-loop. - const pruned = await pruneOldHistory(db, 20); - let okAfterPrune = await _addHistoryEntry(db, entry); - if (!okAfterPrune && pruned > 0) { - await pruneOldHistory(db, 40); - okAfterPrune = await _addHistoryEntry(db, entry); - } - if (!okAfterPrune) { - console.warn('[SkillBridge] Chat history save failed after prune+retry — quota may be stuck'); - } - } - } catch (e) { - console.warn('[SkillBridge] Failed to save conversation:', e); - } - } - - function _addHistoryEntry(db, entry) { - return new Promise((resolve) => { - const tx = db.transaction(HISTORY_STORE, 'readwrite'); - const req = tx.objectStore(HISTORY_STORE).add(entry); - req.onsuccess = () => resolve(true); - req.onerror = (e) => { - const isQuota = e.target.error?.name === 'QuotaExceededError'; - if (isQuota) { - console.warn('[SkillBridge] Chat history quota exceeded — will prune and retry'); - } else { - console.warn('[SkillBridge] Chat history add failed:', e.target.error?.name); - } - resolve(false); - }; - }); - } - - function pruneOldHistory(db, target = 20) { - return new Promise((resolve) => { - const tx = db.transaction(HISTORY_STORE, 'readwrite'); - const store = tx.objectStore(HISTORY_STORE); - const idx = store.index('timestamp'); - const req = idx.openCursor(); - let deleted = 0; - req.onsuccess = (e) => { - const cursor = e.target.result; - if (cursor && deleted < target) { - // Track per-delete failures — without this the count claims - // success even when the runtime silently aborted some deletes. - const delReq = cursor.delete(); - delReq.onsuccess = () => { - deleted++; - }; - delReq.onerror = () => { - console.warn('[SkillBridge] history prune: delete failed at cursor', cursor.primaryKey); - }; - cursor.continue(); - } - }; - // Resolve with the actually-deleted count when the whole transaction - // commits (or fails), so the caller knows whether to escalate. - tx.oncomplete = () => resolve(deleted); - tx.onerror = () => resolve(deleted); - tx.onabort = () => resolve(deleted); - }); - } - - async function getConversations(limit = SKILLBRIDGE_LIMITS.HISTORY) { - try { - const db = await openHistoryDb(); - return new Promise((resolve) => { - const tx = db.transaction(HISTORY_STORE, 'readonly'); - const idx = tx.objectStore(HISTORY_STORE).index('timestamp'); - const results = []; - const req = idx.openCursor(null, 'prev'); - req.onsuccess = (e) => { - const cursor = e.target.result; - if (cursor && results.length < limit) { - results.push(cursor.value); - cursor.continue(); - } else { - resolve(results); - } - }; - req.onerror = () => resolve([]); - }); - } catch { - return []; - } - } - - async function clearAllHistory() { - try { - const db = await openHistoryDb(); - const tx = db.transaction(HISTORY_STORE, 'readwrite'); - tx.objectStore(HISTORY_STORE).clear(); - tx.oncomplete = () => { - const listEl = document.getElementById('si18n-history-list'); - if (listEl) { - listEl.innerHTML = `
    ${sb.t(HISTORY_LABELS.historyCleared)}
    `; - } - }; - } catch (e) { - console.warn('[SkillBridge] Failed to clear history:', e); - } - } - - function toggleHistoryPanel() { - const chatPanel = document.getElementById('si18n-panel-chat'); - if (!chatPanel) return; - - if (historyPanelOpen) { - closeSubPanel(); - return; - } - if (flashcardPanelOpen) closeSubPanel(); - - historyPanelOpen = true; - savedChatHTML = chatPanel.innerHTML; - chatPanel.innerHTML = ` -
    - - ${sb.t(HISTORY_LABELS.title)} - -
    -
    -
    ${sb.t(HISTORY_LABELS.loading)}
    -
    - `; - - document.getElementById('si18n-history-back')?.addEventListener('click', closeHistoryPanel); - document.getElementById('si18n-history-clear')?.addEventListener('click', () => { - if (confirm(sb.t(HISTORY_LABELS.clearHistory) + '?')) clearAllHistory(); - }); - loadHistoryList(); - } - function closeSubPanel() { const chatPanel = document.getElementById('si18n-panel-chat'); - if (!chatPanel || !savedChatHTML) return; + if (!chatPanel || !_state.savedChatHTML) return; // The chat bubble that was streaming is about to be replaced — abort // the stream so its onChunk callback doesn't write into a detached node. cancelActiveStream(); - chatPanel.innerHTML = savedChatHTML; - savedChatHTML = null; - historyPanelOpen = false; - flashcardPanelOpen = false; + chatPanel.innerHTML = _state.savedChatHTML; + _state.savedChatHTML = null; + _state.historyPanelOpen = false; + _state.flashcardPanelOpen = false; bindChatInputEvents(); } - function closeHistoryPanel() { - closeSubPanel(); - } - - async function loadHistoryList() { - const listEl = document.getElementById('si18n-history-list'); - if (!listEl) return; - - const conversations = await getConversations(); - if (conversations.length === 0) { - listEl.innerHTML = `
    ${sb.t(HISTORY_LABELS.empty)}
    `; - return; - } - - const grouped = {}; - for (const conv of conversations) { - const ch = conv.chapter || 'Other'; - if (!grouped[ch]) grouped[ch] = []; - grouped[ch].push(conv); - } - - let html = ''; - for (const [chapter, convs] of Object.entries(grouped)) { - html += `
    ${sb.escapeHtml(chapter)}
    `; - for (const conv of convs) { - const preview = - conv.question.length > SKILLBRIDGE_LIMITS.HISTORY_PREVIEW - ? conv.question.slice(0, SKILLBRIDGE_LIMITS.HISTORY_PREVIEW) + '\u2026' - : conv.question; - const time = new Date(conv.timestamp).toLocaleDateString(undefined, { - month: 'short', - day: 'numeric', - hour: '2-digit', - minute: '2-digit', - }); - html += ` -
    -
    ${sb.escapeHtml(preview)}
    -
    ${time}
    -
    - `; - } - } - listEl.innerHTML = sanitizeHtml(html); - - // Event delegation instead of per-item listeners - listEl.addEventListener('click', (e) => { - const item = e.target.closest('.si18n-history-item'); - if (item) showConversationDetail(item.dataset.id); - }); - } - - async function showConversationDetail(id) { - try { - const db = await openHistoryDb(); - const tx = db.transaction(HISTORY_STORE, 'readonly'); - const req = tx.objectStore(HISTORY_STORE).get(Number(id)); - req.onsuccess = () => { - const conv = req.result; - if (!conv) return; - const listEl = document.getElementById('si18n-history-list'); - if (!listEl) return; - const time = conv.timestamp ? new Date(conv.timestamp).toLocaleString() : ''; - const chapter = conv.chapter ? sb.escapeHtml(conv.chapter) : ''; - let metaHtml = ''; - if (chapter || time) { - metaHtml = `
    `; - if (chapter) metaHtml += `${chapter}`; - if (time) metaHtml += `${time}`; - metaHtml += `
    `; - } - listEl.innerHTML = sanitizeHtml(` -
    - ${metaHtml} -
    -
    ${sb.escapeHtml(conv.question)}
    -
    -
    -
    ${formatResponse(conv.answer)}
    -
    -
    - `); - }; - } catch (e) { - console.warn('[SkillBridge] Failed to load conversation:', e); - } - } - // ============================================================ // SIDEBAR TOGGLE // ============================================================ @@ -878,7 +464,6 @@ // FLASHCARD MODE (Vocabulary Cards for Exam Prep) // ============================================================ - let flashcardPanelOpen = false; let flashcardCards = []; let flashcardIndex = 0; let flashcardBoxes = {}; @@ -888,15 +473,16 @@ const chatPanel = document.getElementById('si18n-panel-chat'); if (!chatPanel) return; - if (flashcardPanelOpen) { + if (_state.flashcardPanelOpen) { closeFlashcardPanel(); return; } - // Close history if open - if (historyPanelOpen) closeHistoryPanel(); + // Close history if open — they share `savedChatHTML`, so closing first + // restores the chat panel before we save it again. + if (_state.historyPanelOpen) closeSubPanel(); - flashcardPanelOpen = true; - savedChatHTML = chatPanel.innerHTML; + _state.flashcardPanelOpen = true; + _state.savedChatHTML = chatPanel.innerHTML; flashcardCards = loadFlashcardsForCourse(); flashcardIndex = 0; @@ -969,7 +555,7 @@ _rawSectionsCache = data; _rawSectionsLang = lang; // Re-run with warm cache and update panel - if (flashcardPanelOpen) { + if (_state.flashcardPanelOpen) { flashcardCards = loadFlashcardsForCourse(); flashcardIndex = 0; refreshFlashcard(); @@ -1213,12 +799,17 @@ }, 500); } - // Export to shared namespace + // Export to shared namespace. + // `formatResponse` is now provided by chat-render.js (loaded earlier in + // the manifest order); we deliberately do NOT re-assign it here. sb.injectSidebar = injectSidebar; sb.injectFloatingButton = injectFloatingButton; sb.toggleSidebar = toggleSidebar; sb.updateLocalizedLabels = updateLocalizedLabels; - sb.formatResponse = formatResponse; sb.toggleFlashcardPanel = toggleFlashcardPanel; sb.cancelActiveStream = cancelActiveStream; + // Surface for chat-history.js / chat-flashcards.js / SPA route handlers. + sb._chat.closeSubPanel = closeSubPanel; + sb._chat.bindChatInputEvents = bindChatInputEvents; + sb._chat.cancelActiveStream = cancelActiveStream; })(); diff --git a/src/lib/_sb-typedef.js b/src/lib/_sb-typedef.js new file mode 100644 index 0000000..29c62b1 --- /dev/null +++ b/src/lib/_sb-typedef.js @@ -0,0 +1,90 @@ +/** + * SkillBridge — `_sb` Namespace Contract + * + * Pure JSDoc type definitions for the shared namespace that content scripts + * mount on `window._sb`. Only loaded by IDEs / `tsc --noEmit` for static + * analysis (see tsconfig.json) — there is no runtime export here. The file + * is intentionally NOT in manifest.content_scripts.js. + * + * Convention: + * - `content.js` constructs the base `_sb` object and exposes shared state + * accessors (currentLang, sidebarVisible, translator, etc.). + * - Each extracted content module attaches its own methods to `_sb` + * (banners.js, header-controls.js, sidebar-chat.js, ...). + * - All cross-module calls go through `_sb.foo?.()` so a module that + * hasn't loaded yet (or got intentionally dropped from the manifest) + * never throws. + * + * @see src/content/content.js — owner of the `_sb` object + * @see src/content/{banners,header-controls,sidebar-chat,text-selection,code-comments,keyboard-shortcuts}.js + */ + +/** + * @typedef {Object} SbState + * @property {string} currentLang — active target language ISO code + * @property {boolean} sidebarVisible + * @property {SkilljarTranslator|null} translator + * @property {boolean} isExamPage — true when `detectExamPage()` matched at init + * @property {Map} originalTexts + * @property {Map} translatedTexts + * @property {Map} originalComments + * @property {number} gtGeneration — bumped each switchLanguage; use to drop stale GT results + * @property {boolean} isOffline + * + * @property {(map: Record, lang?: string) => string} t + * @property {(text: string) => string} escapeHtml + * @property {(text: string) => boolean} isLikelyEnglish + * @property {(newLang: string, opts?: { onDone?: () => void }) => Promise} switchLanguage + * @property {() => { url: string; title: string; lang: string; lessonText?: string }} getPageContext + * + * --- banners.js --- + * @property {?() => void} showOfflineBanner + * @property {?() => void} hideOfflineBanner + * @property {?() => void} showExamBanner + * @property {?() => void} showTranslationProgress + * @property {?(percent: number, label?: string) => void} updateTranslationProgress + * @property {?() => void} hideTranslationProgress + * + * --- header-controls.js --- + * @property {?() => void} injectDarkModeToggle + * @property {?() => void} toggleDarkMode + * @property {?() => void} injectHeaderLanguageSelect + * @property {?() => string|null} detectBrowserLanguage + * @property {?(detected?: string|null) => void} showWelcomeBanner + * + * --- text-selection.js --- + * @property {?() => void} initAskTutorButton + * + * --- keyboard-shortcuts.js --- + * @property {?() => void} toggleShortcutsHelp + * + * --- code-comments.js --- + * @property {?(targetLang: string) => Promise} translateCodeComments + * + * --- sidebar-chat.js (and any extracted chat-* modules) --- + * @property {?() => void} injectSidebar + * @property {?() => void} injectFloatingButton + * @property {?() => void} toggleSidebar + * @property {?() => void} updateLocalizedLabels + * @property {?(text: string) => string} formatResponse + * @property {?() => void} toggleFlashcardPanel + * @property {?() => void} cancelActiveStream + */ + +/** + * @typedef {Object} ProtectedTermsApi + * @property {(targetLang: string, translator: SkilljarTranslator) => void} buildProtectedTermsMap + * @property {(text: string) => string} restoreProtectedTerms + * @property {() => void} resetProtectedTerms + * @property {() => string} getKeepEnglishTerms + */ + +/** + * @typedef {Object} GeminiBlockApi + * @property {(text: string) => string} escapeHtml + * @property {(text: string) => string} sanitizeFromGemini + */ + +// This file is JSDoc-only — no runtime code. The empty `void 0` keeps it a +// valid module body for IDEs that need an expression. +void 0; diff --git a/src/lib/protected-terms.js b/src/lib/protected-terms.js index 29a92bd..5ffd1df 100644 --- a/src/lib/protected-terms.js +++ b/src/lib/protected-terms.js @@ -28,8 +28,18 @@ const map = {}; const protectedEntries = translator.getProtectedTerms?.() || {}; for (const [correct, wrongForms] of Object.entries(protectedEntries)) { - if (Array.isArray(wrongForms)) { - for (const wrong of wrongForms) map[wrong] = correct; + if (!Array.isArray(wrongForms)) continue; + for (const wrong of wrongForms) { + // Skip nullish/empty/non-string forms — `String.prototype.replaceAll` + // on an empty needle inserts the correct form between every char, + // which silently corrupts every translation. The glossary checker + // also flags these, but defending here keeps a stale dictionary + // from blowing up in production. + if (typeof wrong !== 'string' || wrong.length === 0) continue; + // Self-mapping (correct → correct) is a no-op; skip to avoid + // wasted iterations on long pages. + if (wrong === correct) continue; + map[wrong] = correct; } } _protectedTermsSorted = Object.entries(map).sort((a, b) => b[0].length - a[0].length); @@ -39,10 +49,24 @@ /** * Fix mistranslated protected terms in the given text. - * @param {string} text + * + * Known limitation — CJK substring corruption: a Hangul/Hanzi/Kana wrong-form + * that happens to be a prefix of a legitimate longer word will still be + * replaced (e.g. wrong-form "기술" ("skill") inside "기술자" ("technician") + * yields "skill자"). The fix lives in the per-language dictionary itself: + * add the longer compound as its own entry (mapping to its correct form) + * and the longer-first sort below will match it before the shorter prefix. + * See `src/data/.json` `_protected` section. + * + * @param {string|null|undefined} text * @returns {string} */ function restoreProtectedTerms(text) { + // Defensive: callers occasionally pass `null` (e.g. when a Gemini stream + // aborts mid-flight), and the previous implementation would throw + // "Cannot read .includes of null" instead of returning a safe fallback. + if (text == null) return ''; + if (typeof text !== 'string') return text; if (_protectedTermsSorted.length === 0) return text; let result = text; for (const [wrong, correct] of _protectedTermsSorted) { diff --git a/tests/format-response.test.js b/tests/format-response.test.js index 357a1d3..9d32bc6 100644 --- a/tests/format-response.test.js +++ b/tests/format-response.test.js @@ -1,6 +1,9 @@ /** * Unit tests for chat response formatting (markdown → HTML). - * Extracts formatResponse and applyInline directly from sidebar-chat.js source. + * Extracts formatResponse and applyInline from chat-render.js (where the + * functions live since the v3.5.13 sidebar-chat.js split). The test follows + * the production code to its new home rather than re-implementing it, so + * production bugs can't pass green. */ /* global describe, test, expect */ @@ -13,9 +16,9 @@ const geminiBlockSrc = fs.readFileSync(path.join(__dirname, '..', 'src', 'lib', const escapeHtmlBody = geminiBlockSrc.match(/function escapeHtml\(text\)\s*\{([\s\S]*?)\n {2}\}/); const escapeHtml = new Function('text', escapeHtmlBody[1]); -// --- Extract formatResponse + applyInline from sidebar-chat.js --- -const sidebarSrc = fs.readFileSync(path.join(__dirname, '..', 'src', 'content', 'sidebar-chat.js'), 'utf8'); -const fmtBlock = sidebarSrc.match( +// --- Extract formatResponse + applyInline from chat-render.js --- +const renderSrc = fs.readFileSync(path.join(__dirname, '..', 'src', 'content', 'chat-render.js'), 'utf8'); +const fmtBlock = renderSrc.match( /function formatResponse\(text\)\s*\{[\s\S]*?\n {2}\}\n\n {2}function applyInline[\s\S]*?\n {2}\}/, ); const { formatResponse, applyInline } = new Function('sb', `${fmtBlock[0]}\n return { formatResponse, applyInline };`)( diff --git a/tests/protected-terms.test.js b/tests/protected-terms.test.js index 71ce226..e950ddd 100644 --- a/tests/protected-terms.test.js +++ b/tests/protected-terms.test.js @@ -131,6 +131,50 @@ describe('Protected Terms System (real production code)', () => { test('replaces all occurrences of the same wrong form', () => { expect(restoreProtectedTerms('클로드 클로드 클로드')).toBe('Claude Claude Claude'); }); + + test('is idempotent — applying twice equals applying once', () => { + const once = restoreProtectedTerms('클로드 코드를 사용하여 기술을 만듭니다'); + const twice = restoreProtectedTerms(once); + expect(twice).toBe(once); + }); + + test('returns empty string for null input (does not throw)', () => { + // Real callers pass `null` when a Gemini stream aborts mid-flight; + // the previous implementation crashed with `Cannot read .includes of null`. + expect(restoreProtectedTerms(null)).toBe(''); + }); + + test('returns empty string for undefined input (does not throw)', () => { + expect(restoreProtectedTerms(undefined)).toBe(''); + }); + + test('passes non-string input through unchanged', () => { + // Defensive — a number sneaking in shouldn't crash. Returning the input + // makes the corruption obvious upstream rather than masking it as "". + expect(restoreProtectedTerms(42)).toBe(42); + }); + }); + + describe('hardening — empty / self-mapping wrong forms', () => { + test('skips empty-string wrong forms (would corrupt every char)', () => { + // String.prototype.replaceAll('', x) inserts x between every char. + // Production must filter empties before they reach the replace step. + buildProtectedTermsMap('ko', fakeTranslator({ Claude: ['', '클로드'] })); + expect(restoreProtectedTerms('클로드 hi')).toBe('Claude hi'); + }); + + test('skips wrong forms that equal their correct form (no-op cycle)', () => { + // Self-mapping like { Claude: ['Claude'] } would do String.replaceAll + // work for nothing on every pass; ensuring it's filtered keeps the hot + // path tight as glossaries grow. + buildProtectedTermsMap('ko', fakeTranslator({ Claude: ['Claude', '클로드'] })); + expect(restoreProtectedTerms('Claude와 클로드')).toBe('Claude와 Claude'); + }); + + test('skips non-string wrong forms inside the array (defensive)', () => { + buildProtectedTermsMap('ko', fakeTranslator({ Claude: [null, undefined, 42, '클로드'] })); + expect(restoreProtectedTerms('클로드')).toBe('Claude'); + }); }); describe('getKeepEnglishTerms', () => { diff --git a/tsconfig.json b/tsconfig.json new file mode 100644 index 0000000..c9e356a --- /dev/null +++ b/tsconfig.json @@ -0,0 +1,21 @@ +{ + "compilerOptions": { + "target": "ES2022", + "module": "ES2022", + "moduleResolution": "node", + "lib": ["ES2022", "DOM", "DOM.Iterable", "WebWorker"], + "allowJs": true, + "checkJs": true, + "noEmit": true, + "strict": false, + "noImplicitAny": false, + "strictNullChecks": false, + "alwaysStrict": false, + "noImplicitThis": false, + "skipLibCheck": true, + "resolveJsonModule": true, + "types": [] + }, + "include": ["src/**/*.js"], + "exclude": ["node_modules", "dist", "src/bridge/puter.js"] +}