mirror of
https://github.com/screentinker/screentinker.git
synced 2026-08-13 13:53:12 -06:00
Stop shipping untranslated keys as user-facing text
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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uaeo9MvzKoyXuN6ZsbhtkL
This commit is contained in:
ScreenTinker
parent
68dd1b3e05
commit
268bd5e7fb
|
|
@ -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',
|
||||
|
|
|
|||
|
|
@ -409,16 +409,16 @@ async function loadSystem() {
|
|||
|
||||
const versionComparison = version.latest_version
|
||||
? `<div class="info-card">
|
||||
<div class="info-card-label">${t('admin.latest_version') || 'Latest Version'}</div>
|
||||
<div class="info-card-label">${t('admin.latest_version')}</div>
|
||||
<div class="info-card-value small">${esc(version.latest_version)}</div>
|
||||
</div>
|
||||
<div class="info-card">
|
||||
<div class="info-card-label">${t('admin.status') || 'Status'}</div>
|
||||
<div class="info-card-value small" style="color:${version.update_available ? 'var(--warning)' : 'var(--success)'}">${version.update_available ? (t('admin.update_available') || 'Update Available') : (t('admin.up_to_date') || 'Up to Date')}</div>
|
||||
<div class="info-card-label">${t('admin.status')}</div>
|
||||
<div class="info-card-value small" style="color:${version.update_available ? 'var(--warning)' : 'var(--success)'}">${version.update_available ? (t('admin.update_available')) : (t('admin.up_to_date'))}</div>
|
||||
</div>`
|
||||
: `<div class="info-card">
|
||||
<div class="info-card-label">${t('admin.latest_version') || 'Latest Version'}</div>
|
||||
<div class="info-card-value small" style="color:var(--text-muted)">${t('admin.checking') || 'Checking...'}</div>
|
||||
<div class="info-card-label">${t('admin.latest_version')}</div>
|
||||
<div class="info-card-value small" style="color:var(--text-muted)">${t('admin.checking')}</div>
|
||||
</div>`;
|
||||
|
||||
el.innerHTML = `
|
||||
|
|
@ -427,8 +427,8 @@ async function loadSystem() {
|
|||
${versionComparison}
|
||||
</div>
|
||||
<div style="display:flex;gap:8px;margin-top:16px;flex-wrap:wrap">
|
||||
<button class="btn btn-secondary btn-sm" id="checkUpdateBtn">${t('admin.check_now') || 'Check Now'}</button>
|
||||
<button class="btn btn-primary btn-sm" id="triggerUpdateBtn"${!version.update_available ? ' style="display:none"' : ''}>${t('admin.update_now') || 'Update Now'}</button>
|
||||
<button class="btn btn-secondary btn-sm" id="checkUpdateBtn">${t('admin.check_now')}</button>
|
||||
<button class="btn btn-primary btn-sm" id="triggerUpdateBtn"${!version.update_available ? ' style="display:none"' : ''}>${t('admin.update_now')}</button>
|
||||
<a href="/api/status/backup?token=${token}" class="btn btn-secondary btn-sm" style="text-decoration:none">${t('admin.download_db_backup')}</a>
|
||||
<a href="/api/status" target="_blank" class="btn btn-secondary btn-sm" style="text-decoration:none">${t('admin.server_status')}</a>
|
||||
</div>
|
||||
|
|
@ -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 = `
|
||||
<div style="margin-top:12px;border:1px solid var(--border);border-radius:var(--radius);padding:12px;background:var(--bg-card)">
|
||||
<div style="display:flex;justify-content:space-between;align-items:center;margin-bottom:8px">
|
||||
<strong style="font-size:13px">${data.success ? (t('admin.update_success') || 'Update Successful') : (t('admin.update_failed') || 'Update Failed')}</strong>
|
||||
<button class="btn btn-secondary btn-sm" id="copyOutputBtn">${t('admin.copy') || 'Copy'}</button>
|
||||
<strong style="font-size:13px">${data.success ? (t('admin.update_success')) : (t('admin.update_failed'))}</strong>
|
||||
<button class="btn btn-secondary btn-sm" id="copyOutputBtn">${t('admin.copy')}</button>
|
||||
</div>
|
||||
<pre style="max-height:300px;overflow:auto;font-size:11px;margin:0;background:var(--bg-primary);padding:8px;border-radius:4px;white-space:pre-wrap;word-break:break-all">${esc(data.output || '')}</pre>
|
||||
</div>`;
|
||||
|
|
@ -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 = `
|
||||
<div style="margin-top:12px;border:1px solid var(--border);border-radius:var(--radius);padding:12px;background:var(--bg-secondary)">
|
||||
<div style="display:flex;justify-content:space-between;align-items:center;margin-bottom:8px">
|
||||
<strong style="font-size:13px">${t('admin.manual_update') || 'Manual Update Required'}</strong>
|
||||
<button class="btn btn-secondary btn-sm" id="copyCmdBtn">${t('admin.copy_command') || 'Copy'}</button>
|
||||
<strong style="font-size:13px">${t('admin.manual_update')}</strong>
|
||||
<button class="btn btn-secondary btn-sm" id="copyCmdBtn">${t('admin.copy_command')}</button>
|
||||
</div>
|
||||
<p style="font-size:12px;color:var(--text-muted);margin-bottom:8px">${t('admin.manual_update_desc') || 'Run this command on the server:'}</p>
|
||||
<p style="font-size:12px;color:var(--text-muted);margin-bottom:8px">${t('admin.manual_update_desc')}</p>
|
||||
<pre style="font-size:11px;margin:0;background:var(--bg-primary);padding:8px;border-radius:4px;white-space:pre-wrap;word-break:break-all">${esc(data.instructions)}</pre>
|
||||
</div>`;
|
||||
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 = `<p style="color:var(--danger)">${esc(err.message)}</p>`; }
|
||||
|
|
|
|||
|
|
@ -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');
|
||||
|
|
|
|||
|
|
@ -217,12 +217,12 @@ async function renderWallEditor(container, wallId) {
|
|||
<span style="font-size:11px;color:var(--text-muted);max-width:340px">Cols/rows/bezel are used by Auto-arrange. Drag freely on the canvas to override.</span>
|
||||
</div>
|
||||
<div style="margin-top:16px">
|
||||
<h3 style="font-size:14px;margin:0 0 8px">${t('wall.playlist') || 'Playlist'}</h3>
|
||||
<h3 style="font-size:14px;margin:0 0 8px">${t('wall.playlist')}</h3>
|
||||
<select id="wallPlaylist" class="input" style="width:300px;background:var(--bg-input)">
|
||||
<option value="">${t('wall.no_playlist') || 'No playlist'}</option>
|
||||
<option value="">${t('wall.no_playlist')}</option>
|
||||
${(playlists || []).map(p => `<option value="${esc(p.id)}" ${p.id === wall.playlist_id ? 'selected' : ''}>${esc(p.name)}${p.status === 'draft' ? ' (draft)' : ''}</option>`).join('')}
|
||||
</select>
|
||||
<button class="btn btn-primary btn-sm" id="setPlaylistBtn" style="margin-left:8px">${t('wall.set_playlist') || 'Set Playlist'}</button>
|
||||
<button class="btn btn-primary btn-sm" id="setPlaylistBtn" style="margin-left:8px">${t('wall.set_playlist')}</button>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
|
|
@ -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'); }
|
||||
});
|
||||
|
||||
|
|
|
|||
77
server/test/i18n-keys-exist.test.js
Normal file
77
server/test/i18n-keys-exist.test.js
Normal file
|
|
@ -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`);
|
||||
}
|
||||
});
|
||||
Loading…
Reference in a new issue