mirror of
https://github.com/got-feedBack/feedBack.git
synced 2026-07-21 12:21:49 +00:00
3717e4338d
Some checks are pending
ship-ci / ci (push) Waiting to run
app.js exported `esc` as an implicit global back when it was a classic
script. a9fce29 made it an ES module and 14b4058 carved `esc` into
js/dom.js; the window re-export list was rebuilt without it.
Out-of-tree plugins load screen.js as a classic script and call `esc()`
bare, so nothing in-tree catches the break: no-undef, a call-graph scan
and a grep all pass while the plugin throws in the field. The MIDI
plugin builds its device list with esc() inside the same try block that
catches requestMIDIAccess() failures, so the ReferenceError surfaced to
testers as "MIDI Access denied esc is not defined" — access had actually
been granted.
Pin the whole plugin-facing global surface by name, mirroring
tests/test_plugin_context_contract.py. Verified both ways against a
running app: without the fix the spec fails with "missing or not
functions: esc".
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
64 lines
3.3 KiB
TypeScript
64 lines
3.3 KiB
TypeScript
// The window globals are a THIRD-PARTY CONTRACT. Pin them.
|
|
//
|
|
// Out-of-tree plugins load their screen.js as a CLASSIC script and call these
|
|
// as bare globals. Nothing in core reads most of them, so a call-graph scan,
|
|
// ESLint's no-undef, and a grep all come back clean while the plugin breaks in
|
|
// the field. This is the frontend twin of tests/test_plugin_context_contract.py
|
|
// — same reasoning, same literal-list rule.
|
|
//
|
|
// This guard is retroactive: `esc` was an implicit global back when app.js was
|
|
// a classic script, went module-scoped in a9fce29, and got carved into
|
|
// js/dom.js in 14b4058. The re-export list at the bottom of app.js was rebuilt
|
|
// without it, and the MIDI plugin's device list threw "esc is not defined" for
|
|
// testers — reported as "MIDI Access denied", because the ReferenceError landed
|
|
// in a try/catch meant for permission failures.
|
|
//
|
|
// WHY A LITERAL LIST AND NOT A DERIVED ONE. Deriving the expected set from
|
|
// app.js would assert the code equals itself. The point is that a human has to
|
|
// look at a diff and consciously agree to change the contract.
|
|
|
|
import { test, expect } from '@playwright/test';
|
|
|
|
const PLUGIN_GLOBALS = [
|
|
'_confirmDialog', '_getArrangementNamingMode', '_libraryLocalFilename', '_librarySongArtUrl',
|
|
'_librarySongId', '_onHeaderClick', '_onNamingModeChange', '_trapFocusInModal',
|
|
'changeArrangement', 'checkPluginUpdates', 'clearLibFilters', 'clearLoop',
|
|
'deleteSelectedLoop', 'esc', 'exportDiagnostics', 'exportSettings', 'filterFavorites',
|
|
'filterLibrary', 'fullRescanLibrary', 'goFavPage', 'handleSliderInput',
|
|
'hideScanBanner', 'importSettings', 'loadPlugins', 'loadSavedLoop',
|
|
'loadSettings', 'onSectionPracticeModeChange', 'openEditModal', 'persistSetting',
|
|
'pickDlcFolder', 'pinCurrentArrangementDefault', 'playSong', 'previewDiagnostics',
|
|
'previewEditArt', 'renderGridCards', 'renderTreeInto', 'rescanLibrary',
|
|
'retuneSong', 'saveCurrentLoop', 'saveSettings', 'seekBy',
|
|
'setAvOffsetMs', 'setFavView', 'setInstrumentPathway', 'setLibView',
|
|
'setLibraryProvider', 'setLoopEnd', 'setLoopStart', 'setMastery',
|
|
'setSpeed', 'setViz', 'showScreen', 'sortFavorites',
|
|
'sortLibrary', 'syncLibrarySong', 'toggleAllArtists', 'toggleAllFavoriteArtists',
|
|
'toggleLibFilters', 'togglePlay', 'toggleSectionPracticePopover', 'uiPrompt',
|
|
'updatePlugin', 'uploadSongs',
|
|
'filterFavTreeLetter', 'filterTreeLetter', 'goFavTreePage', 'goTreePage',
|
|
];
|
|
|
|
test('plugin-facing window globals are all callable', async ({ page }) => {
|
|
await page.goto('/');
|
|
await page.waitForSelector('.screen.active', { timeout: 10000 });
|
|
|
|
const missing = await page.evaluate(
|
|
(names) => names.filter((n) => typeof (window as any)[n] !== 'function'),
|
|
PLUGIN_GLOBALS,
|
|
);
|
|
|
|
expect(missing, `window globals plugins depend on are missing or not functions: ${missing.join(', ')}`).toEqual([]);
|
|
});
|
|
|
|
// The plugin call site that actually broke: esc() interpolated into a template
|
|
// string. A global that exists but doesn't escape is its own bug.
|
|
test('window.esc escapes HTML metacharacters', async ({ page }) => {
|
|
await page.goto('/');
|
|
await page.waitForSelector('.screen.active', { timeout: 10000 });
|
|
|
|
const escaped = await page.evaluate(() => (window as any).esc('<img src=x onerror=alert(1)>'));
|
|
expect(escaped).not.toContain('<img');
|
|
expect(escaped).toContain('<');
|
|
});
|