diff options
| author | tanaka takashi <isazaka@gmail.com> | 2026-09-01 20:38:17 +0900 |
|---|---|---|
| committer | tanaka takashi <isazaka@gmail.com> | 2026-09-01 20:38:17 +0900 |
| commit | bb06e0e58dcb90eceede6f0b05743fdccc8f51af (patch) | |
| tree | aa239db6e1a1d695c651c363702182c24b57c4cd /src/popup/popup.js | |
| parent | 55e581bab9b600e7c070ec596d3109fc897d2d86 (diff) | |
fix: options page never ran due to missing export; unify settings handling
Root cause of the broken options page (audit finding):
- options.js imported CONTEXT_PRESETS from settings-schema.js, but that
module never exported it. A named-import mismatch in ES modules is a
SyntaxError, so the entire options page script silently never executed.
This is why the settings page rendered its static HTML but was dead:
no language options, no model list, no save handler.
Fixes
- options.js: import CONTEXT_PRESETS from shared/constants.js (where it
is defined) instead of settings-schema.js
- options.js: save flow now merges the validated form over the stored
settings (sanitize again) so fields saved by the popup in the meantime
are preserved instead of being dropped
- popup.js: populate language options inside loadSettings and sanitize
settings before every save, matching the options page behavior
- settings-schema.js: add normalizeOllamaHost() and use it in
sanitizeSettings so trailing-slash variants are stored identically
- content.js: consolidate popover state - remove the duplicate outer
lastSourceText and keep regenerate source inside the popover module;
regenerate now calls portClient.translate directly
Test hardening (would have caught the original bug)
- tests/ui-smoke-tests.js: new smoke tests that install a fake browser
API, run the real popup.js / options.js page scripts as ES modules
against representative markup, and assert that language options are
populated, the model select is filled from the mocked model list, and
the stored host is reflected. The runner result elements are preserved
across the sandboxed DOM swap
- test.html: include the smoke test module
Results: 73 passed, 0 failed (71 existing + 2 smoke). dist rebuilt.
Diffstat (limited to 'src/popup/popup.js')
| -rw-r--r-- | src/popup/popup.js | 27 |
1 files changed, 12 insertions, 15 deletions
diff --git a/src/popup/popup.js b/src/popup/popup.js index c07851e..612d0fd 100644 --- a/src/popup/popup.js +++ b/src/popup/popup.js @@ -1,7 +1,7 @@ 'use strict'; import { LANGUAGES, ERROR_CODES, RUNTIME_MESSAGES } from '../shared/constants.js'; -import { defaultSettings } from '../shared/settings-schema.js'; +import { defaultSettings, sanitizeSettings } from '../shared/settings-schema.js'; const $enabled = /** @type {HTMLInputElement} */ (document.getElementById('enabled-toggle')); const $model = /** @type {HTMLSelectElement} */ (document.getElementById('model-select')); @@ -83,21 +83,25 @@ async function loadModels() { async function loadSettings() { const result = await browser.storage.local.get({ settings: null }); - const settings = result.settings || defaultSettings(); + const settings = sanitizeSettings(result.settings || defaultSettings()); $enabled.checked = Boolean(settings.enabled); + for (const lang of LANGUAGES) { + const option = document.createElement('option'); + option.value = lang.code; + option.textContent = lang.label; + $language.append(option); + } $language.value = settings.targetLanguage || 'ja'; fillModels(knownModels, settings.model || ''); - if (settings.model) { - // Preserve the stored model even before the first list load. - fillModels(knownModels.length ? knownModels : [settings.model], settings.model); - } } async function persist(patch) { const result = await browser.storage.local.get({ settings: null }); - const settings = result.settings || defaultSettings(); + const settings = sanitizeSettings(result.settings || defaultSettings()); const merged = { ...settings, ...patch }; - await browser.storage.local.set({ settings: merged }); + // Sanitize before saving so unrelated fields stay valid. + const validated = sanitizeSettings(merged); + await browser.storage.local.set({ settings: validated }); } $enabled.addEventListener('change', () => { @@ -121,13 +125,6 @@ $openOptions.addEventListener('click', () => { browser.runtime.openOptionsPage(); }); -// Init -for (const language of LANGUAGES) { - const option = document.createElement('option'); - option.value = language.code; - option.textContent = language.label; - $language.append(option); -} void (async () => { await loadSettings(); await loadModels(); |
