summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authortanaka takashi <isazaka@gmail.com>2026-09-01 20:38:17 +0900
committertanaka takashi <isazaka@gmail.com>2026-09-01 20:38:17 +0900
commitbb06e0e58dcb90eceede6f0b05743fdccc8f51af (patch)
treeaa239db6e1a1d695c651c363702182c24b57c4cd
parent55e581bab9b600e7c070ec596d3109fc897d2d86 (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.
-rw-r--r--src/content/content.js13
-rw-r--r--src/options/options.html2
-rw-r--r--src/options/options.js10
-rw-r--r--src/popup/popup.css16
-rw-r--r--src/popup/popup.html4
-rw-r--r--src/popup/popup.js27
-rw-r--r--src/shared/settings-schema.js14
-rw-r--r--tests/test.html1
-rw-r--r--tests/ui-smoke-tests.js156
9 files changed, 199 insertions, 44 deletions
diff --git a/src/content/content.js b/src/content/content.js
index e1e6823..fa5f416 100644
--- a/src/content/content.js
+++ b/src/content/content.js
@@ -56,8 +56,6 @@
let currentSelectionRect = null;
/** @type {string|null} id of the request the popover currently shows */
let activeRequestId = null;
- /** @type {string|null} source text of the last request (for regenerate) */
- let lastSourceText = null;
// ---------------------------------------------------------------------
// 2. Selection controller
@@ -364,7 +362,7 @@
});
regenerateButton.addEventListener('click', () => {
if (localLastSourceText) {
- requestTranslation(localLastSourceText);
+ portClient.translate(localLastSourceText);
}
});
}
@@ -551,7 +549,6 @@
const p = connect();
const requestId = generateRequestId();
activeRequestId = requestId;
- lastSourceText = text;
popover.beginRequest(text, currentSelectionRect || { left: 16, top: 16, bottom: 40, right: 356 });
p.postMessage({ type: CONTENT_TO_BG.TRANSLATE, requestId, text });
},
@@ -587,14 +584,6 @@
portClient.translate(selection.text);
}
- /**
- * Request a translation for an arbitrary source text (regenerate).
- * @param {string} sourceText
- */
- function requestTranslation(sourceText) {
- portClient.translate(sourceText);
- }
-
// ---------------------------------------------------------------------
// Enabled state (storage.local, EN-01/EN-02)
// ---------------------------------------------------------------------
diff --git a/src/options/options.html b/src/options/options.html
index b60e66b..7390c71 100644
--- a/src/options/options.html
+++ b/src/options/options.html
@@ -99,6 +99,6 @@
<p class="dev-link"><a href="../../tests/test.html" target="_blank" rel="noreferrer">開発者向け: テストを実行</a></p>
</main>
- <script src="options.js"></script>
+ <script type="module" src="options.js"></script>
</body>
</html> \ No newline at end of file
diff --git a/src/options/options.js b/src/options/options.js
index 606291b..b4fc349 100644
--- a/src/options/options.js
+++ b/src/options/options.js
@@ -1,10 +1,9 @@
'use strict';
-import { LANGUAGES, ERROR_CODES, RUNTIME_MESSAGES } from '../shared/constants.js';
+import { LANGUAGES, ERROR_CODES, RUNTIME_MESSAGES, CONTEXT_PRESETS } from '../shared/constants.js';
import {
defaultSettings,
sanitizeSettings,
- CONTEXT_PRESETS,
isValidOllamaHost
} from '../shared/settings-schema.js';
@@ -152,7 +151,6 @@ function applySettings(settings) {
input.value = value === null || value === undefined ? '' : String(value);
}
$stop.value = (settings.advanced.stop || []).join('\n');
- fillModels(knownModels, settings.model);
}
async function loadSettings() {
@@ -230,7 +228,11 @@ $form.addEventListener('submit', (event) => {
}
void (async () => {
- await browser.storage.local.set({ settings: validated });
+ // Merge over the stored settings so fields not present in this form
+ // (e.g. saved by the popup meanwhile) are preserved.
+ const result = await browser.storage.local.get({ settings: null });
+ const merged = { ...(result.settings || defaultSettings()), ...validated };
+ await browser.storage.local.set({ settings: sanitizeSettings(merged) });
if (warnings.length > 0) {
$saveStatus.textContent = `保存しました(修正された項目: ${warnings.join(', ')})`;
} else {
diff --git a/src/popup/popup.css b/src/popup/popup.css
index 8ed312a..964454d 100644
--- a/src/popup/popup.css
+++ b/src/popup/popup.css
@@ -20,17 +20,15 @@ h1 {
margin-bottom: 10px;
}
-.toggle-label {
- font-weight: 500;
-}
-
-.toggle-label + input[type='checkbox'] {
- margin-left: 8px;
+.toggle-row {
+ flex-direction: row;
+ align-items: center;
+ justify-content: space-between;
}
-.row .toggle-label {
- display: inline-block;
- margin-bottom: 4px;
+.toggle-label {
+ font-weight: 500;
+ margin: 0;
}
label {
diff --git a/src/popup/popup.html b/src/popup/popup.html
index 92099a7..e51475f 100644
--- a/src/popup/popup.html
+++ b/src/popup/popup.html
@@ -8,7 +8,7 @@
<main>
<h1>Ollama Translations</h1>
- <section class="row">
+ <section class="row toggle-row">
<label class="toggle-label" for="enabled-toggle">翻訳機能</label>
<input type="checkbox" id="enabled-toggle" role="switch">
</section>
@@ -29,6 +29,6 @@
<button id="open-options" type="button">詳細設定を開く</button>
</main>
- <script src="popup.js"></script>
+ <script type="module" src="popup.js"></script>
</body>
</html> \ No newline at end of file
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();
diff --git a/src/shared/settings-schema.js b/src/shared/settings-schema.js
index fad7a5f..057812d 100644
--- a/src/shared/settings-schema.js
+++ b/src/shared/settings-schema.js
@@ -33,6 +33,7 @@ export function isValidOllamaHost(value) {
}
if (url.protocol !== 'http:') return false;
if (url.hostname !== 'localhost' && url.hostname !== '127.0.0.1') return false;
+ // Only the root path ("http://host/" or "http://host") is allowed.
if (url.pathname !== '/' && url.pathname !== '') return false;
if (url.search !== '' || url.hash !== '') return false;
if (url.username !== '' || url.password !== '') return false;
@@ -45,6 +46,17 @@ export function isValidOllamaHost(value) {
}
/**
+ * Normalize a validated host for storage: strip the trailing slash so
+ * "http://localhost:11434/" and "http://localhost:11434" are stored
+ * identically.
+ * @param {string} host already validated host
+ * @returns {string}
+ */
+export function normalizeOllamaHost(host) {
+ return host.replace(/\/+$/, '') || 'http://localhost';
+}
+
+/**
* @param {*} value
* @returns {boolean}
*/
@@ -189,7 +201,7 @@ export function sanitizeSettings(stored) {
if (isValidOllamaHost(stored.ollamaHost)) {
// Normalize: strip trailing slash.
- result.ollamaHost = stored.ollamaHost.replace(/\/+$/, '') || DEFAULT_OLLAMA_HOST;
+ result.ollamaHost = normalizeOllamaHost(stored.ollamaHost) || DEFAULT_OLLAMA_HOST;
}
if (typeof stored.model === 'string' && stored.model.length <= 256) {
diff --git a/tests/test.html b/tests/test.html
index 7983b17..ddacace 100644
--- a/tests/test.html
+++ b/tests/test.html
@@ -25,5 +25,6 @@
<script src="runner.js"></script>
<script src="mocks.js"></script>
<script type="module" src="all-tests.js"></script>
+ <script type="module" src="ui-smoke-tests.js"></script>
</body>
</html> \ No newline at end of file
diff --git a/tests/ui-smoke-tests.js b/tests/ui-smoke-tests.js
new file mode 100644
index 0000000..84fe4c4
--- /dev/null
+++ b/tests/ui-smoke-tests.js
@@ -0,0 +1,156 @@
+/**
+ * UI smoke tests: load the real page scripts (popup.js / options.js) as ES
+ * modules against a mocked browser API and verify that they initialize the
+ * DOM. This catches import/export mismatches that pure unit tests miss
+ * (e.g. a named import that the module does not export -> SyntaxError and
+ * the whole page script never runs).
+ */
+
+import { LANGUAGES } from '../src/shared/constants.js';
+
+const { describe, it, assertTrue, assertEqual } = window.TestRunner;
+
+/**
+ * Install a fake `browser` global before importing page scripts.
+ */
+function installFakeBrowser() {
+ const data = new Map();
+ const listeners = [];
+ const local = {
+ async get(defaults) {
+ const result = {};
+ for (const key of Object.keys(defaults)) {
+ result[key] = data.has(key) ? data.get(key) : defaults[key];
+ }
+ return result;
+ },
+ async set(obj) {
+ const changes = {};
+ for (const [key, value] of Object.entries(obj)) {
+ changes[key] = { oldValue: data.get(key), newValue: value };
+ data.set(key, value);
+ }
+ for (const listener of listeners) listener(changes, 'local');
+ }
+ };
+ globalThis.browser = {
+ storage: {
+ local,
+ onChanged: {
+ addListener(fn) {
+ listeners.push(fn);
+ }
+ }
+ },
+ runtime: {
+ /** @returns {Promise<{ok: boolean, models?: string[]}>} */
+ sendMessage: async () => ({ ok: true, models: ['fake-model'] }),
+ openOptionsPage: () => {}
+ }
+ };
+ return globalThis.browser;
+}
+
+/**
+ * Run an ES module script by importing it with a cache-busting query.
+ * The page's markup is added inside a sandbox container; the script runs,
+ * async init settles, assertions are collected via a probe callback, and
+ * finally the sandbox is removed and the runner's own elements restored.
+ * @param {string} scriptPath
+ * @param {string} markup
+ * @param {() => *} probe assertions to run while the sandbox is attached
+ */
+async function runPageScript(scriptPath, markup, probe) {
+ installFakeBrowser();
+ // Preserve the runner's result elements across the DOM swap.
+ const results = document.getElementById('results');
+ const summary = document.getElementById('summary');
+ const resultsParent = results ? results.parentNode : null;
+ const summaryParent = summary ? summary.parentNode : null;
+ const resultsNext = results ? results.nextSibling : null;
+ const summaryNext = summary ? summary.nextSibling : null;
+
+ const sandbox = document.createElement('div');
+ sandbox.id = 'smoke-sandbox';
+ sandbox.innerHTML = markup;
+ document.body.append(sandbox);
+
+ try {
+ // Import with cache-buster so repeated runs re-execute the script.
+ await import(`${scriptPath}?smoke=${Date.now()}-${Math.random().toString(36).slice(2)}`);
+ // Let async init settle.
+ await new Promise((resolve) => setTimeout(resolve, 50));
+ return probe();
+ } finally {
+ sandbox.remove();
+ if (results && resultsParent) resultsParent.insertBefore(results, resultsNext);
+ if (summary && summaryParent) summaryParent.insertBefore(summary, summaryNext);
+ // Reset the fake browser so later tests use the real API if needed.
+ delete globalThis.browser;
+ }
+}
+
+describe('popup page script smoke', () => {
+ it('initializes language options and model select from markup', async () => {
+ const r = await runPageScript('../src/popup/popup.js', `
+ <input type="checkbox" id="enabled-toggle">
+ <select id="model-select"></select>
+ <button id="reload-models"></button>
+ <p id="model-status" hidden></p>
+ <select id="language-select"></select>
+ <button id="open-options"></button>
+ `, () => ({
+ langOptions: document.getElementById('language-select').options.length,
+ langValue: document.getElementById('language-select').value,
+ modelOptions: document.getElementById('model-select').options.length,
+ firstModel: document.getElementById('model-select').options[0]?.textContent ?? null
+ }));
+ assertEqual(r.langOptions, LANGUAGES.length);
+ assertEqual(r.langValue, 'ja'); // default target language
+ assertTrue(r.modelOptions > 0, 'model select should be populated');
+ assertEqual(r.firstModel, 'fake-model');
+ });
+});
+
+describe('options page script smoke', () => {
+ it('initializes language options and loads models into the select', async () => {
+ const r = await runPageScript('../src/options/options.js', `
+ <form id="options-form">
+ <input type="checkbox" id="enabled">
+ <input type="text" id="ollama-host">
+ <select id="model"></select>
+ <button id="reload-models" type="button"></button>
+ <p id="model-status" hidden></p>
+ <select id="target-language"></select>
+ <select id="thinking">
+ <option value="off">off</option><option value="on">on</option>
+ <option value="low">low</option><option value="medium">medium</option>
+ <option value="high">high</option>
+ </select>
+ <input type="range" id="temperature" min="0" max="2" step="0.1">
+ <output id="temperature-value"></output>
+ <input type="range" id="context-length" min="0" max="5" step="1">
+ <output id="context-length-value"></output>
+ <input type="radio" name="seed-mode" value="random">
+ <input type="radio" name="seed-mode" value="fixed">
+ <div id="seed-value-row"></div>
+ <input type="number" id="seed-value">
+ <textarea id="user-system-prompt"></textarea>
+ <div id="advanced-fields"></div>
+ <textarea id="stop"></textarea>
+ <span id="save-status"></span>
+ </form>
+ `, () => ({
+ langOptions: document.getElementById('target-language').options.length,
+ langValue: document.getElementById('target-language').value,
+ modelOptions: document.getElementById('model').options.length,
+ firstModel: document.getElementById('model').options[0]?.textContent ?? null,
+ host: document.getElementById('ollama-host').value
+ }));
+ assertEqual(r.langOptions, LANGUAGES.length);
+ assertEqual(r.langValue, 'ja');
+ assertTrue(r.modelOptions > 0, 'model select should be populated by loadModels');
+ assertEqual(r.firstModel, 'fake-model');
+ assertEqual(r.host, 'http://localhost:11434');
+ });
+}); \ No newline at end of file