From 268bd5e7fb572bff144a16ab6d2c42a2cf0be3ef Mon Sep 17 00:00:00 2001 From: ScreenTinker Date: Tue, 28 Jul 2026 18:16:32 -0500 Subject: [PATCH] Stop shipping untranslated keys as user-facing text MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Driving the app in a real browser showed a context menu whose only item read "schedule.ctx_new". t() returns the KEY when a string is missing — it never returns undefined — so a missing key renders literally, and the common `t('x') || 'A readable default'` guard is dead code: the key is truthy, the default can never fire, and the pattern hides the problem instead of covering it. Every occurrence of it in the app was doing exactly that. Nineteen strings were affected, most of them predating this work: fifteen in the self-hosted update panel and four in video walls, all of which have been showing raw keys to users. The intended text was recovered from the dead defaults, so the wording is the authors' own, and the defaults are removed rather than left to imply a safety net that does not exist. A test now walks the views for the keys they actually ask for and fails on any that English does not define, and separately rejects the `|| default` pattern. Neither problem is visible to a syntax check, a unit test, or review — only to someone looking at the screen — so the guard is the only thing that keeps them from coming back. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Uaeo9MvzKoyXuN6ZsbhtkL --- frontend/js/i18n/en.js | 35 +++++++++++++ frontend/js/views/admin.js | 40 +++++++-------- frontend/js/views/schedule.js | 18 +++---- frontend/js/views/video-wall.js | 8 +-- server/test/i18n-keys-exist.test.js | 77 +++++++++++++++++++++++++++++ 5 files changed, 145 insertions(+), 33 deletions(-) create mode 100644 server/test/i18n-keys-exist.test.js diff --git a/frontend/js/i18n/en.js b/frontend/js/i18n/en.js index 45b48a3..4a25a7b 100644 --- a/frontend/js/i18n/en.js +++ b/frontend/js/i18n/en.js @@ -1,6 +1,41 @@ // English translations. This file is the source of truth for keys — // every other locale should mirror its keys (or fall back to en). export default { + + // Recovered from dead `t(k) || 'default'` fallbacks: t() returns the key when a string is + // missing, so those defaults never rendered and users saw the raw key instead. + 'admin.check_now': 'Check Now', + 'admin.checking': 'Checking...', + 'admin.copied': 'Copied!', + 'admin.copy': 'Copy', + 'admin.copy_command': 'Copy', + 'admin.latest_version': 'Latest Version', + 'admin.manual_update': 'Manual Update Required', + 'admin.manual_update_desc': 'Run this command on the server:', + 'admin.status': 'Status', + 'admin.up_to_date': 'Up to Date', + 'admin.update_available': 'Update Available', + 'admin.update_failed': 'Update Failed', + 'admin.update_now': 'Update Now', + 'admin.update_success': 'Update Successful', + 'admin.updating': 'Updating...', + 'wall.no_playlist': 'No playlist', + 'wall.playlist': 'Playlist', + 'wall.set_playlist': 'Set Playlist', + 'wall.toast.playlist_updated': 'Playlist updated', + + // Calendar direct-manipulation strings. NOTE: t() returns the KEY when a string is missing, + // never undefined — so `t('x') || 'fallback'` can never fire and would ship the raw key to the + // user. These must exist here; a browser run caught 'schedule.ctx_new' rendering literally. + 'schedule.ctx_new': 'New schedule here…', + 'schedule.ctx_edit': 'Edit…', + 'schedule.ctx_duplicate': 'Duplicate', + 'schedule.ctx_delete': 'Delete', + 'schedule.confirm_series': 'This schedule repeats. Changing it here updates every occurrence. Continue?', + 'schedule.confirm_delete': 'Delete this schedule?', + 'schedule.toast.deleted': 'Schedule deleted', + 'schedule.drag_hint': 'Drag across a time to add a schedule, or right-click for options.', + // Getting-started checklist (components/getting-started.js). Driven by real account state, // not a one-time flag, so it can tell someone what is actually left to do. 'gs.title': 'Get your first screen live', diff --git a/frontend/js/views/admin.js b/frontend/js/views/admin.js index d83cc57..b05b8e3 100644 --- a/frontend/js/views/admin.js +++ b/frontend/js/views/admin.js @@ -409,16 +409,16 @@ async function loadSystem() { const versionComparison = version.latest_version ? `
-
${t('admin.latest_version') || 'Latest Version'}
+
${t('admin.latest_version')}
${esc(version.latest_version)}
-
${t('admin.status') || 'Status'}
-
${version.update_available ? (t('admin.update_available') || 'Update Available') : (t('admin.up_to_date') || 'Up to Date')}
+
${t('admin.status')}
+
${version.update_available ? (t('admin.update_available')) : (t('admin.up_to_date'))}
` : `
-
${t('admin.latest_version') || 'Latest Version'}
-
${t('admin.checking') || 'Checking...'}
+
${t('admin.latest_version')}
+
${t('admin.checking')}
`; el.innerHTML = ` @@ -427,8 +427,8 @@ async function loadSystem() { ${versionComparison}
- - + + ${t('admin.download_db_backup')} ${t('admin.server_status')}
@@ -439,7 +439,7 @@ async function loadSystem() { document.getElementById('checkUpdateBtn')?.addEventListener('click', async () => { const btn = document.getElementById('checkUpdateBtn'); btn.disabled = true; - btn.textContent = t('admin.checking') || 'Checking...'; + btn.textContent = t('admin.checking'); try { const res = await fetch('/api/admin/check-update', { method: 'POST', headers: headers() }); const data = await res.json(); @@ -451,7 +451,7 @@ async function loadSystem() { } catch (err) { showToast(err.message, 'error'); btn.disabled = false; - btn.textContent = t('admin.check_now') || 'Check Now'; + btn.textContent = t('admin.check_now'); } }); @@ -460,7 +460,7 @@ async function loadSystem() { const btn = document.getElementById('triggerUpdateBtn'); const resultEl = document.getElementById('updateResult'); btn.disabled = true; - btn.textContent = t('admin.updating') || 'Updating...'; + btn.textContent = t('admin.updating'); try { const res = await fetch('/api/admin/trigger-update', { method: 'POST', headers: headers() }); const data = await res.json(); @@ -469,8 +469,8 @@ async function loadSystem() { resultEl.innerHTML = `
- ${data.success ? (t('admin.update_success') || 'Update Successful') : (t('admin.update_failed') || 'Update Failed')} - + ${data.success ? (t('admin.update_success')) : (t('admin.update_failed'))} + 
${esc(data.output || '')}
`; @@ -478,7 +478,7 @@ async function loadSystem() { const pre = resultEl.querySelector('pre'); const text = pre ? pre.textContent : ''; if (navigator.clipboard) { - navigator.clipboard.writeText(text).then(() => showToast(t('admin.copied') || 'Copied!', 'success')); + navigator.clipboard.writeText(text).then(() => showToast(t('admin.copied'), 'success')); } else { // Fallback for older browsers const ta = document.createElement('textarea'); @@ -489,7 +489,7 @@ async function loadSystem() { ta.select(); document.execCommand('copy'); document.body.removeChild(ta); - showToast(t('admin.copied') || 'Copied!', 'success'); + showToast(t('admin.copied'), 'success'); } }); } else if (data.instructions) { @@ -497,17 +497,17 @@ async function loadSystem() { resultEl.innerHTML = `
- ${t('admin.manual_update') || 'Manual Update Required'} - + ${t('admin.manual_update')} +
-

${t('admin.manual_update_desc') || 'Run this command on the server:'}

+

${t('admin.manual_update_desc')}

${esc(data.instructions)}
`; document.getElementById('copyCmdBtn')?.addEventListener('click', () => { const pre = resultEl.querySelector('pre'); const text = pre ? pre.textContent : ''; if (navigator.clipboard) { - navigator.clipboard.writeText(text).then(() => showToast(t('admin.copied') || 'Copied!', 'success')); + navigator.clipboard.writeText(text).then(() => showToast(t('admin.copied'), 'success')); } else { const ta = document.createElement('textarea'); ta.value = text; @@ -517,7 +517,7 @@ async function loadSystem() { ta.select(); document.execCommand('copy'); document.body.removeChild(ta); - showToast(t('admin.copied') || 'Copied!', 'success'); + showToast(t('admin.copied'), 'success'); } }); } @@ -525,7 +525,7 @@ async function loadSystem() { showToast(err.message, 'error'); } finally { btn.disabled = false; - btn.textContent = t('admin.update_now') || 'Update Now'; + btn.textContent = t('admin.update_now'); } }); } catch (err) { el.innerHTML = `

${esc(err.message)}

`; } diff --git a/frontend/js/views/schedule.js b/frontend/js/views/schedule.js index 224b53c..b40418a 100644 --- a/frontend/js/views/schedule.js +++ b/frontend/js/views/schedule.js @@ -461,7 +461,7 @@ export async function render(container) { return; } if (editsWholeSeries(st.ev) - && !confirm(t('schedule.confirm_series') || 'This schedule repeats. Changing it here updates every occurrence. Continue?')) { + && !confirm(t('schedule.confirm_series'))) { loadCalendar(); return; } @@ -502,10 +502,10 @@ export async function render(container) { menu.style.cssText = `position:fixed;left:${x}px;top:${y}px;z-index:2000;min-width:170px;background:var(--bg-secondary,#1f2530);` + 'border:1px solid var(--border,#333);border-radius:6px;padding:4px;box-shadow:0 6px 24px rgba(0,0,0,.4);font-size:13px'; const items = ev - ? [[t('schedule.ctx_edit') || 'Edit…', () => editSchedule(ev)], - [t('schedule.ctx_duplicate') || 'Duplicate', () => duplicateSchedule(ev)], - [t('schedule.ctx_delete') || 'Delete', () => deleteSchedule(ev)]] - : [[t('schedule.ctx_new') || 'New schedule here…', () => openCreateAt(dayDate, Math.floor(minutes / 15) * 15, Math.floor(minutes / 15) * 15 + 60)]]; + ? [[t('schedule.ctx_edit'), () => editSchedule(ev)], + [t('schedule.ctx_duplicate'), () => duplicateSchedule(ev)], + [t('schedule.ctx_delete'), () => deleteSchedule(ev)]] + : [[t('schedule.ctx_new'), () => openCreateAt(dayDate, Math.floor(minutes / 15) * 15, Math.floor(minutes / 15) * 15 + 60)]]; items.forEach(([label, fn]) => { const b = document.createElement('div'); b.textContent = label; @@ -535,10 +535,10 @@ export async function render(container) { } async function deleteSchedule(ev) { - if (!confirm(t('schedule.confirm_delete') || 'Delete this schedule?')) return; + if (!confirm(t('schedule.confirm_delete'))) return; try { await API(`/schedules/${ev.id}`, { method: 'DELETE' }); - showToast(t('schedule.toast.deleted') || 'Deleted', 'success'); + showToast(t('schedule.toast.deleted'), 'success'); } catch (err) { showToast(err.message, 'error'); } loadCalendar(); } @@ -598,11 +598,11 @@ export async function render(container) { document.getElementById('deleteScheduleBtn').onclick = async () => { if (!editingId) return; - if (!confirm(t('schedule.confirm_delete') || 'Delete this schedule?')) return; + if (!confirm(t('schedule.confirm_delete'))) return; try { await API(`/schedules/${editingId}`, { method: 'DELETE' }); document.getElementById('scheduleModal').style.display = 'none'; - showToast(t('schedule.toast.deleted') || 'Schedule deleted', 'success'); + showToast(t('schedule.toast.deleted'), 'success'); loadCalendar(); } catch (err) { showToast(err.message, 'error'); diff --git a/frontend/js/views/video-wall.js b/frontend/js/views/video-wall.js index 1049671..fc09e44 100644 --- a/frontend/js/views/video-wall.js +++ b/frontend/js/views/video-wall.js @@ -217,12 +217,12 @@ async function renderWallEditor(container, wallId) { Cols/rows/bezel are used by Auto-arrange. Drag freely on the canvas to override.
-

${t('wall.playlist') || 'Playlist'}

+

${t('wall.playlist')}

- +
@@ -688,7 +688,7 @@ async function renderWallEditor(container, wallId) { try { await API(`/walls/${wallId}`, { method: 'PUT', body: JSON.stringify({ playlist_id: playlistId }) }); wall.playlist_id = playlistId; - showToast(t('wall.toast.playlist_updated') || 'Playlist updated', 'success'); + showToast(t('wall.toast.playlist_updated'), 'success'); } catch (err) { showToast(err.message, 'error'); } }); diff --git a/server/test/i18n-keys-exist.test.js b/server/test/i18n-keys-exist.test.js new file mode 100644 index 0000000..4a04880 --- /dev/null +++ b/server/test/i18n-keys-exist.test.js @@ -0,0 +1,77 @@ +'use strict'; + +// t() returns the KEY ITSELF when a string is missing — `registry[lang]?.[key] ?? fallback[key] ?? key`. +// It never returns undefined. Two consequences, both of which have already bitten: +// +// 1. A missing key ships to the user as raw text. A browser run found a context menu whose only +// item read "schedule.ctx_new". +// 2. `t('x') || 'Some default'` looks like a safety net but is dead code, because the key string +// is truthy. The default can never render, so it hides the missing key instead of covering it. +// +// Neither shows up in a unit test of the logic, or in a syntax check, or in review — only in front +// of a user. So this walks the views for the keys they actually ask for and checks English has them. + +const { test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); + +const FRONTEND = path.join(__dirname, '..', '..', 'frontend', 'js'); +const EN = fs.readFileSync(path.join(FRONTEND, 'i18n', 'en.js'), 'utf8'); + +// Keys defined in en.js, as written: 'some.key': '...' +const defined = new Set([...EN.matchAll(/^\s*'([^']+)'\s*:/gm)].map(m => m[1])); + +function sourceFiles(dir) { + const out = []; + for (const e of fs.readdirSync(dir, { withFileTypes: true })) { + const p = path.join(dir, e.name); + if (e.isDirectory()) { if (e.name !== 'i18n') out.push(...sourceFiles(p)); } + else if (e.name.endsWith('.js')) out.push(p); + } + return out; +} + +// Only literal t('...') calls — a computed key cannot be checked statically, and pretending +// otherwise would produce false failures. +function referencedKeys(src) { + return [...src.matchAll(/\bt\(\s*'([a-z0-9_]+(?:\.[a-z0-9_]+)+)'/gi)].map(m => m[1]); +} + +test('every literal t() key used by the app exists in English', () => { + const missing = []; + for (const file of sourceFiles(FRONTEND)) { + const src = fs.readFileSync(file, 'utf8'); + for (const key of referencedKeys(src)) { + if (!defined.has(key)) missing.push(`${path.relative(FRONTEND, file)}: ${key}`); + } + } + assert.deepEqual(missing, [], + `these render as raw key text to the user:\n ${missing.join('\n ')}`); +}); + +test('no t() call carries a || default, which can never fire', () => { + // The pattern reads as a safety net and is the opposite: it guarantees the missing key is + // silently shipped instead of the readable default. + const offenders = []; + for (const file of sourceFiles(FRONTEND)) { + const src = fs.readFileSync(file, 'utf8'); + for (const m of src.matchAll(/\bt\(\s*'[^']+'\s*(?:,[^)]*)?\)\s*\|\|\s*'/g)) { + const line = src.slice(0, m.index).split('\n').length; + offenders.push(`${path.relative(FRONTEND, file)}:${line}`); + } + } + assert.deepEqual(offenders, [], + `t() never returns falsy, so these defaults are dead:\n ${offenders.join('\n ')}`); +}); + +test('the getting-started checklist has all of its strings', () => { + // Called out separately because it is brand-new copy and entirely user-facing. + for (const k of ['gs.title', 'gs.progress', 'gs.dismiss', + 'gs.device.title', 'gs.device.desc', 'gs.device.cta', + 'gs.content.title', 'gs.content.desc', 'gs.content.cta', + 'gs.playlist.title', 'gs.playlist.desc', 'gs.playlist.cta', + 'gs.assign.title', 'gs.assign.desc', 'gs.assign.cta']) { + assert.ok(defined.has(k), `${k} is missing and would render literally`); + } +});