refactor(editor): move the inspector panel to src/inspector.js (R2, step 24)

CHANGELOG.md

@@ -9,6 +9,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
+- **The inspector panel now lives in `src/inspector.js` (R2, step 24).** 639
+  lines: note attributes on one face, chord name/voicing/fingering/function on
+  the other. `src/main.js` is down to 9,144.
+  Most of its edits commit through a command and are undoable; the technique
+  toggles and boolean flags still mutate in place, which is a deliberate scope
+  limit from PR3b and unchanged here. All of them honour the read-only-roll lock.
+  Its 19 `window.editor*` handlers — the ones the panel's own `innerHTML` calls
+  by name — are exported plain functions that `main.js` re-attaches. `main.js`
+  keeps the bend-curve dialog and the canvas-resize scheduler.
+
+
 - **The MIDI keyboard recorder now lives in `src/midi-record.js` (R2, step 23),
   and the transport clock in `src/transport.js`.** `src/main.js` is down to
   9,779 — under ten thousand for the first time.

src/host.js

@@ -113,6 +113,12 @@ export const host = {
      * to none of them and stays in main.js.
      */
     finalizeActiveDrag: () => {},
+
+    // ── Dialogs and canvas geometry, for src/inspector.js ────────────
+    /** Open the bend-curve editor for a note. Async: resolves when it closes. */
+    promptBend: async () => {},
+    /** Re-measure the canvas on the next frame (a lane count changed). */
+    scheduleCanvasResize: () => {},
 };
 
 export function setHostHooks(hooks) { Object.assign(host, hooks); }

tests/inspector_time.test.mjs

@@ -22,21 +22,26 @@ import { S as realS } from '../src/state.js';
 import { EditHistory } from '../src/history.js';
 import { seedState, trackHooks } from './_history_env.mjs';
 
-const src = fs.readFileSync(new URL('../src/main.js', import.meta.url), 'utf8');
-
-// Brace-match extraction of a named class/function/const (the waveform_render
-// harness pattern) — drives the real source, no re-implementation.
-function extractNamed(decl) {
-    const start = src.indexOf(decl);
-    assert.ok(start >= 0, `not found: ${decl}`);
-    const open = src.indexOf('{', start);
+// The inspector's bounds table, coercion helper and field dispatcher are
+// module-private (the dispatcher only reaches the page as a re-attached
+// window.*), so they are still sliced — with the `export` keyword stripped,
+// since that is a SyntaxError inside `new Function`.
+const inspSrc = fs.readFileSync(new URL('../src/inspector.js', import.meta.url), 'utf8');
+const unexport = (code) => code.replace(/^export\s+/gm, '');
+function extractFromInspector(decl) {
+    const start = inspSrc.indexOf(decl);
+    assert.ok(start >= 0, `not found in inspector.js: ${decl}`);
+    const open = inspSrc.indexOf('{', start);
     let depth = 0;
-    for (let i = open; i < src.length; i++) {
-        if (src[i] === '{') depth++;
-        else if (src[i] === '}' && --depth === 0) return src.slice(start, i + 1);
+    for (let i = open; i < inspSrc.length; i++) {
+        if (inspSrc[i] === '{') depth++;
+        else if (inspSrc[i] === '}' && --depth === 0) return unexport(inspSrc.slice(start, i + 1));
     }
     throw new Error(`unbalanced braces for ${decl}`);
 }
+
+// Brace-match extraction of a named class/function/const (the waveform_render
+// harness pattern) — drives the real source, no re-implementation.
 // The commands are real imports and resolve their target through notes(), which
 // reads the REAL S. Seed one arrangement and point it at CURRENT so the cases
 // can keep asserting on the array object they built.
@@ -49,8 +54,8 @@ const setCurrent = (arr) => { CURRENT = arr; realS.arrangements[0].notes = arr;
 // helper are still in main.js, so they are still brace-matched out of it.
 const api = new Function(
     '"use strict";'
-    + extractNamed('const _INSPECTOR_BOUNDS =') + '\n'
-    + extractNamed('function _coerceInspectorNumber') + '\n'
+    + extractFromInspector('export const _INSPECTOR_BOUNDS =') + '\n'
+    + extractFromInspector('export function _coerceInspectorNumber') + '\n'
     + 'return { _INSPECTOR_BOUNDS, _coerceInspectorNumber };'
 )();
 const { _INSPECTOR_BOUNDS, _coerceInspectorNumber } = api;
@@ -139,28 +144,27 @@ t('time: a note already at the target gets a zero delta (no-op move)', () => {
 let DISPATCH_NOTES = [];
 const dispatchS = { sel: new Set(), drumEditMode: false, tempoMapMode: false, history: null };
 let renderCount = 0;                 // # of _renderInspector() calls (reject branch)
-const win = {};                      // captures `window.editorInspectorSetField = …`
 // `dispatchS` stays sandbox-local: editorInspectorSetField reads its `sel` and
 // mode flags. EditHistory itself closes over the real `S` (seeded above), which
 // only supplies the arrangement tag — irrelevant to these cases.
-new Function(
-    'notes', 'S', 'draw', 'updateStatus', '_renderInspector', 'window',
+// The dispatcher reaches main.js through `host` now, and calls the module-local
+// _renderInspector directly — both are injected here, so the reject branch's
+// re-render stays observable.
+const setField = new Function(
+    'notes', 'S', 'host', '_renderInspector',
     'MoveNoteCmd', 'ResizeSustainGroupCmd',
     '"use strict";'
-    + extractNamed('const _INSPECTOR_BOUNDS =') + '\n'
-    + extractNamed('function _coerceInspectorNumber') + '\n'
-    + extractNamed('function _editorCurrentNoteIndices') + '\n'
-    + extractNamed('window.editorInspectorSetField =') + '\n'
-    + 'return {};'
+    + extractFromInspector('export const _INSPECTOR_BOUNDS =') + '\n'
+    + extractFromInspector('export function _coerceInspectorNumber') + '\n'
+    + extractFromInspector('export function editorInspectorSetField') + '\n'
+    + 'return editorInspectorSetField;'
 )(
     () => DISPATCH_NOTES,
     dispatchS,
-    () => {}, () => {},          // editorInspectorSetField's own draw/updateStatus
+    { draw() {}, updateStatus() {}, editorCurrentNoteIndices: () => [...dispatchS.sel] },
     () => { renderCount++; },
-    win,
     MoveNoteCmd, ResizeSustainGroupCmd,
 );
-const setField = win.editorInspectorSetField;
 
 // Fresh notes + selection + history per case.
 // `dispatchS` stays sandbox-local for editorInspectorSetField's `sel` and mode

tests/inspector_xss.test.mjs

@@ -0,0 +1,61 @@
+/*
+ * The inspector escapes note-derived values before assigning innerHTML.
+ *
+ * A feedpak is an untrusted file. The server's _note() coerces string/fret to
+ * ints (routes.py), so a hostile value cannot reach the client through the load
+ * path today — but the panel must not DEPEND on that: a note that ever arrived
+ * un-coerced would inject markup. This drives the real _renderInspector with a
+ * hostile fret and asserts the value is escaped in the innerHTML it writes
+ * (CodeRabbit, #176).
+ *
+ * Run: node tests/inspector_xss.test.mjs
+ */
+import assert from 'node:assert';
+import { _renderInspector } from '../src/inspector.js';
+import { S } from '../src/state.js';
+
+const PAYLOAD = '<img src=x onerror="window.__XSS=1">';
+
+// Capture the innerHTML the panel writes, with no jsdom. `_renderInspector`
+// only needs #editor-inspector; give it a stub that records every assignment.
+let lastHtml = '';
+const panel = {
+    _html: '',
+    get innerHTML() { return this._html; },
+    set innerHTML(v) { this._html = v; lastHtml = v; },
+    classList: { contains: () => false, add() {}, remove() {} },
+    querySelectorAll: () => [],
+};
+globalThis.document = { getElementById: (id) => (id === 'editor-inspector' ? panel : null) };
+
+// A single selected note whose fret is the payload — exactly what would arrive
+// from a persisted note that dodged coercion.
+Object.assign(S, {
+    arrangements: [{ id: 'a1', name: 'Lead', notes: [
+        { time: 0, string: PAYLOAD, fret: PAYLOAD, sustain: 0, techniques: {} },
+    ] }],
+    currentArr: 0,
+    sel: new Set([0]),
+});
+
+let pass = 0, fail = 0;
+function t(name, fn) {
+    try { fn(); pass++; console.log('  ok   ' + name); }
+    catch (e) { fail++; console.error('  FAIL ' + name + ': ' + e.message); }
+}
+
+t('the hostile fret does not survive as a raw <img> tag', () => {
+    _renderInspector();
+    assert.ok(lastHtml.length > 0, 'the panel rendered something');
+    assert.ok(!/<img\s/i.test(lastHtml),
+        'a raw <img> tag reached innerHTML — the value was not escaped:\n' + lastHtml.slice(0, 400));
+});
+
+t('the payload is present, but as escaped entities', () => {
+    // It should still be visible to the user — escaped, not stripped.
+    assert.ok(lastHtml.includes('&lt;img') || lastHtml.includes('&lt;'),
+        'the payload was neither escaped nor present; expected &lt;img…');
+});
+
+console.log(`\n${pass} passed, ${fail} failed`);
+if (fail) process.exit(1);

tests/view_switcher.test.mjs

@@ -261,6 +261,27 @@ t('read-only roll: SONG-scope undo still works (drum edit reverts)', () => {
 //    (regressions for #119: these bypass EditHistory, so the exec lock
 //     alone can't stop them — each entry point is guarded at its source).
 
+// The inspector moved to src/inspector.js, where its handlers are exported
+// function declarations rather than `window.NAME = (…) => {…}` arrows, and reach
+// main.js through `host`. Same body, different header.
+const inspSrc = fs.readFileSync(new URL('../src/inspector.js', import.meta.url), 'utf8');
+function extractInspectorFn(name, globals) {
+    const marker = 'export function ' + name + '(';
+    const start = inspSrc.indexOf(marker);
+    assert.ok(start >= 0, `export function ${name} must exist in inspector.js`);
+    const open = inspSrc.indexOf('{', start);
+    let depth = 0, end = -1;
+    for (let i = open; i < inspSrc.length; i++) {
+        if (inspSrc[i] === '{') depth++;
+        else if (inspSrc[i] === '}' && --depth === 0) { end = i; break; }
+    }
+    assert.ok(end > 0, `unbalanced braces extracting ${name}`);
+    const decl = inspSrc.slice(start, end + 1).replace(/^export\s+/, '');
+    const names = Object.keys(globals);
+    const fn = new Function(...names, '"use strict";' + decl + '\nreturn ' + name + ';');
+    return fn(...names.map(k => globals[k]));
+}
+
 // Extract a `window.NAME = (...) => { ... };` arrow assignment and rebuild
 // it as a callable with the named globals stubbed in.
 function extractWinFn(name, globals) {
@@ -304,13 +325,12 @@ t('read-only roll: inspector editorInspectorSetFlag does not mutate the fretted
     const note = { string: 0, fret: 3, techniques: {} };
     const locked = { value: true };
     let notices = 0, renders = 0;
-    const setFlag = extractWinFn('editorInspectorSetFlag', {
+    const setFlag = extractInspectorFn('editorInspectorSetFlag', {
         _selectedNotes: () => [note],
         _rollReadOnly: () => locked.value,
         _rollLockNotice: () => { notices++; },
         _renderInspector: () => { renders++; },
-        draw: () => {},
-        updateStatus: () => {},
+        host: { draw() {}, updateStatus() {} },
     });
     setFlag('accent', true);
     assert.strictEqual(note.techniques.accent, undefined, 'no write while read-only');