From f6afbdbb15c366d7b6c5d967df8acf3f80c17c64 Mon Sep 17 00:00:00 2001 From: Chuck <33324927+ChuckBuilds@users.noreply.github.com> Date: Thu, 24 Sep 2026 17:36:53 -0400 Subject: [PATCH] fix(web): Cache/Logs error mix-up, store errors, tab fallbacks; remove ~2.5k lines of dead JS (#639) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(web): keep Cache and Logs helpers out of each other's way Both partials declared top-level showError and escapeHtml. Their scripts run at global scope after every HTMX swap, so whichever tab was opened last owned window.showError, and a Cache failure after visiting Logs rendered into the Logs panel (and the other way round). Each script is now an IIFE; Cache still exports deleteCacheFile for its row buttons. Co-Authored-By: Claude Opus 5.5 * fix(web): make the HTMX-failure fallbacks for tab panels actually run - The "HTMX never loaded" fallback read appElement.__x.$data, which is Alpine 2. The page ships Alpine 3, so the check was always false and the Overview never loaded without HTMX. It now reads Alpine.$data(). - The Overview and WiFi panels used hx-on::htmx:response-error, which htmx expands to "htmx:htmx:response-error", an event that never fires. - loadTabContent sent requests with as the source, so htmx fired its events on and no panel's hx-on handler ran at all. The panel is now the source. htmx also resolves its promise on a 4xx/5xx, and the panel was stamped data-loaded anyway, leaving a skeleton that never retried; it is now stamped only when no responseError fired. loadPluginsDirect, loadOverviewDirect and loadWifiDirect are merged into one window.loadPartialDirect(id, url), which also runs the partial's inline scripts before Alpine sees the markup (as htmx-config.js does on htmx:afterSwap). The ~10 s "htmx never arrived" path in loadTabContent uses it for every tab instead of four hard-coded ones. Co-Authored-By: Claude Opus 5.5 * fix(web): store and registry failures no longer wipe the Plugin Manager showError replaced the whole #plugins-content with an error message, so one failed store search, custom-registry install or saved-repository call took the installed list, the store and every control with it, with no way back short of reloading the tab. Those failures are now error notifications. The full-panel message is kept only for a first load of the installed list that failed (nothing to show yet); a failed refresh of an already-rendered list is a notification too. showSuccess's fallback branch, which wrote the message into innerHTML unescaped, is gone: showNotification always exists. The "Please try refreshing your browser" hint tested for the text "Failed to Fetch", which no browser produces (Chrome says "Failed to fetch", Firefox "NetworkError..."), so it never appeared. It now keys on the failure itself: a TypeError from fetch(), or PluginAPI's NETWORK_ERROR wrapper around one. Co-Authored-By: Claude Opus 5.5 * fix(web): escape plugin action and install output on every path executePluginAction escaped data.message and data.output when an action failed but put data.message straight into innerHTML when it succeeded, and set the OAuth step-2 button's innerHTML from the manifest's step2_button_text. Plugin actions run plugin code, so that is plugin- or server-controlled markup in the page. Both paths now escape, and the button label is set with textContent. The same pattern sat in the install-from-GitHub-URL status lines (plugin_id, the server's message, and error.message, which can echo a repository URL) and the custom-registry load error; those are escaped too. Co-Authored-By: Claude Opus 5.5 * fix(web): file-upload widget owns the image list and schedule editor plugins_manager.js loads after the widget bundle, so its older copies of deleteUploadedFile, updateImageList, hideUploadProgress, formatDate, openImageSchedule, toggleImageScheduleEnabled, updateImageSchedule{Mode, Time,Day} and updateCheckboxGroupData replaced the widget's. They are deleted; the widget files are the only definitions. Before switching over, the two sets were diffed and fixed so nothing regresses: - The old copy labelled the schedule/delete buttons for screen readers and lazy-loaded thumbnails; the widget now does both. - The schedule button did nothing on a card rendered by plugin_config.html whenever the image id is a UUID (every upload): the template turns "-" into "_" in the editor's id, and neither JS copy did. Both now use the template's rule. - The widget's "keep the open editor open" copied the editor's innerHTML into the new list. That dropped its event listeners and showed the old values, so after the first change the editor looked live but ignored input. A schedule edit now saves to the hidden input and updates the card's summary in place without re-rendering the list; a list re-render (upload, delete) rebuilds an open editor from the data. Editor controls are routed by one delegated change listener, so there are no per-element listeners to lose. - The old deleteUploadedFile had a JSON branch that removed a #file_ element and skipped the re-render. No template or script renders such an element, and JSON uploads are listed through updateImageList like images, so re-rendering (the widget's behaviour) is the consistent one; the branch was not carried over. - The template always renders the summary line (".image-schedule-summary", "Always shown" when unscheduled) so an edit has a line to update. The inline-handler test evaluated plugins_manager.js's updateImageList; test_file_upload_widget.js now covers the widget's list and editor. Co-Authored-By: Claude Opus 5.5 * refactor(web): delete the unused handleCredentialsUpload Its last caller went when plugin_config.html switched credential uploads to the file-upload widget's handleSingleFileSelect. Nothing in the web UI, the tests or the plugin monorepo references it. Co-Authored-By: Claude Opus 5.5 * refactor(web): delete dead and shadowed front-end code Nothing calls any of these (checked across web_interface/, test/ and the ledmatrix-plugins monorepo, including hx-*/x-*/onclick attributes): - app-shell.js: the Alpine methods refreshPlugins (it called a nonexistent this.searchPluginStore), loadPluginConfig, savePluginConfig, getSchemaPropertyType, escapeCssSelector, formatCommitInfo and formatDateInfo, and the top-level copies of savePluginConfig, getSchemaPropertyType, escapeCssSelector, formatCommitInfo, formatDateInfo and togglePluginFromTab. Plugin config forms save through hx-post in plugin_config.html. - window.reconnectSSE (app-shell.js); window.updateArrayTableAddButtonState (array-table.js). - toggleNestedSection, defined twice (app-shell.js and plugins_manager.js) and called from nowhere. - plugins_manager.js: the window.initializePlugins wrapper around an IIFE-local origInit that was always undefined, and __pluginDomReady, which was written but never read. - display.html's fixInvalidNumberInputs fallback: app-shell.js defines it before any partial loads. - base.html's window.loadCodeMirror and the two CodeMirror stylesheet preloads, and the .CodeMirror rules in plugins.html. The raw JSON editor is a plain textarea. Also deleted: app-shell.js definitions that a later script always replaced, so they never ran: executePluginAction (plugins_manager.js assigns its own), uninstallPlugin and its pollUninstallOperation (plugins_manager.js), and updateAllPlugins (install_manager.js). vendor/codemirror stays: test/test_web_smoke.py still requests codemirror.min.js as a sample static asset. Co-Authored-By: Claude Opus 5.5 * refactor(web): call showNotification without checking it exists app-shell.js defines window.showNotification (a stand-in that queues until the notification widget loads) and base.html runs it, deferred, before every other script that notifies: app.js, the utilities, the widget bundle, plugins_manager.js, and all partials, which HTMX loads after the page. The 81 `typeof showNotification === 'function'` / `!== 'undefined'` checks, the `window.showNotification || console.log` and `|| alert` fallbacks, and their else branches (alert(), console output, and schedule.html's own hand-built toast) could never take the fallback path. They are removed, as is fonts.html's second copy of the queueing stand-in. The stand-in in app-shell.js keeps its guard (it must not replace the widget's implementation if load order ever changes), and BaseWidget's public notify()/getNotificationFunction() keep their shape for widgets that plugins ship. Co-Authored-By: Claude Opus 5.5 * refactor(web): one HTML escaper, window.LEDEscape About 30 files each carried their own escapeHtml / escapeAttr / escHtml / _esc / escapeJs. They disagreed: several (notification.js, display.html's escapeHtml, operation_history.html, the app() stub) did not escape quotes, google-calendar-picker.js and tools.html's escHtml left ' alone, and some turned 0 into ''. Most were fine only because the quote-safe widget copies were preferred at runtime. window.LEDEscape now lives at the top of app-early.js, a blocking script in , so it exists before any other script runs: html(v) & < > " ' as entities, null/undefined as '' attr(v) the same, for call sites that want to say "attribute" jsStringAttr(v) a JS string literal safe inside an inline handler Every former copy is now a one-line name for it (kept so call sites do not change), widgets included, with no fallback. plugins_manager.js loses its four escapeJs wrappers (callers use jsStringAttr), the duplicate escapeAttr and escapeHtml inside renderInstalledCards and renderCustomRegistryPlugins, and the window.escapeHtml / window.escapeAttribute exports, which nothing read. addArrayObjectItem's fallback markup (with a sixth hand-written escape chain) is gone too: window.renderArrayObjectItem is defined earlier in the same file, so the fallback could not run. The unused escapeHtml methods on the Alpine app (app-early.js stub and app-shell.js) are deleted. test_html_escaping.js now runs LEDEscape and every remaining name for it, and fails if a hand-rolled escaper reappears anywhere in web_interface/. Suites that evaluate slices of plugins_manager.js or widget files load LEDEscape from app-early.js through test/js/led_escape.js. Co-Authored-By: Claude Opus 5.5 * fix(web): stop htmx re-running partial scripts after every tab load htmx-config.js runs each swapped-in ', 'a'b']) { + const out = LEDEscape.jsStringAttr(v); + ok(`${JSON.stringify(v)}: no raw quote or bracket`, !/["'<>]/.test(out), out); + // eslint-disable-next-line no-eval + ok(`${JSON.stringify(v)}: decodes back to the same string`, eval(decode(out)) === v, out); + } + ok('null and undefined become the empty string', + LEDEscape.html(null) === '' && LEDEscape.html(undefined) === '' && LEDEscape.jsStringAttr(null) === '""'); + ok('numbers are kept', LEDEscape.html(0) === '0', LEDEscape.html(0)); +} + +console.log('\n4c. no hand-rolled escaper outside app-early.js'); +{ + const skip = new Set(['static/v3/js/app-early.js', + // documentation example, kept self-contained on purpose + 'static/v3/js/widgets/example-color-picker.js']); + const found = []; + const walk = dir => fs.readdirSync(dir, { withFileTypes: true }).forEach(e => { + const p = path.join(dir, e.name); + const rel = path.relative(ROOT, p).split(path.sep).join('/'); + if (e.isDirectory()) { if (e.name !== 'vendor') walk(p); return; } + if (!/\.(js|html)$/.test(e.name) || /\.min\.js$/.test(e.name) || skip.has(rel)) return; + // Writing the entity for a quote is what an escaper does; nothing else in + // the UI needs to. + let text = fs.readFileSync(p, 'utf8'); + // In templates only the inline scripts count; Jinja's own |replace("'", "'") + // escaping of server-rendered values is not a JS escaper. + if (e.name.endsWith('.html')) text = (text.match(/]*>[\s\S]*?<\/script\s*>/gi) || []).join('\n'); + if (/['"`]"['"`]|['"`]'['"`]/.test(text)) found.push(rel); + }); + walk(path.join(ROOT, 'static')); + walk(path.join(ROOT, 'templates')); + ok('every escaper is window.LEDEscape', found.length === 0, found); + // Plugin pages may call these globals; they stay, as aliases. + const early = fs.readFileSync(path.join(ROOT, 'static/v3/js/app-early.js'), 'utf8'); + ok('window.escapeHtml is kept as an alias of LEDEscape.html', + early.includes('window.escapeHtml = window.LEDEscape.html;')); + ok('window.escapeAttribute is kept as an alias of LEDEscape.attr', + early.includes('window.escapeAttribute = window.LEDEscape.attr;')); +} + // ── url-input scheme handling (js/xss-through-dom) ───────────────────────── console.log('\n5. url-input never treats a scriptable scheme as a valid URL'); { diff --git a/test/js/unit/test_inline_handler_escaping.js b/test/js/unit/test_inline_handler_escaping.js index 984d4299..e32d6d8c 100644 --- a/test/js/unit/test_inline_handler_escaping.js +++ b/test/js/unit/test_inline_handler_escaping.js @@ -7,9 +7,8 @@ // // JSON.stringify makes a valid JS string but leaves `'` alone, so an entry id // of x' onmouseover='alert(1) closed the single-quoted attribute and added a -// handler of its own. The live window.updateImageList (plugins_manager.js loads -// last, so its copy beats the file-upload widget's) put the uploaded file's -// original name into the markup unescaped. +// handler of its own. (The uploaded-image list is no longer built here: the +// file-upload widget owns it, and test_file_upload_widget.js covers it.) // // Each case renders with the shipped function, parses the tag the way a browser // does (quoted attribute values, entities decoded), and checks two things: no @@ -54,6 +53,7 @@ global.document = { createElement: () => new TextEl(), }; global.window = global; +require('../led_escape').install(window); global.pluginLog = () => {}; global.isStorePluginInstalled = () => false; global.isNewPlugin = () => false; @@ -68,8 +68,6 @@ eval([ ].map(extract).join('\n') + '\nglobal.jsStringAttr = jsStringAttr; global.escapeHtml = escapeHtml;' + '\nglobal.renderPluginStore = renderPluginStore; global.renderSavedRepositories = renderSavedRepositories;' + '\nglobal.renderCustomRegistryPlugins = renderCustomRegistryPlugins; global.escapeAttribute = escapeAttribute;'); -// eslint-disable-next-line no-eval -eval(extract('window.updateImageList = function(fieldId, images) {')); // ── minimal HTML start-tag tokenizer ─────────────────────────────────────── function decodeEntities(s) { @@ -190,26 +188,5 @@ for (const hostile of [SQ, DQ, AMP]) { calls.length === 1 && calls[0][0] === 'removeSavedRepository' && calls[0][1] === hostile, calls); } -console.log('\n5. live window.updateImageList escapes the uploaded file name'); -window.getUploadConfig = () => ({ plugin_id: SQ }); -window.currentPluginConfig = null; -{ - const name = '' + DQ + '.png'; - window.updateImageList('f', [{ id: SQ, path: 'assets/x".png', filename: DQ, original_filename: name, size: 1 }]); - const html = els['f_image_list'].innerHTML; - ok('no raw markup from original_filename', !html.includes(', only the template attributes', - imgs.length === 1 && names(imgs[0]).join(',') === 'src,alt,loading,decoding,class,onerror', imgs.map(names)); - ok('alt carries the stored filename as text', attr(imgs[0], 'alt') === DQ, imgs[0]); - const buttons = tags(html, 'button'); - ok('two buttons, no extra attributes', - buttons.length === 2 && buttons.every(b => names(b).join(',') === 'type,onclick,class,title,aria-label'), - buttons.map(names)); - const del = runHandler(attr(buttons[1], 'onclick')); - ok('delete handler gets field, image and plugin ids intact', - del.length === 1 && del[0][0] === 'deleteUploadedImage' && del[0][1] === 'f' && del[0][2] === SQ && del[0][3] === SQ, del); -} - console.log(`\n${pass} passed, ${fail} failed\n`); process.exit(fail ? 1 : 0); diff --git a/test/js/unit/test_plugin_action_delegation.js b/test/js/unit/test_plugin_action_delegation.js index a65cc9ee..882a79b3 100644 --- a/test/js/unit/test_plugin_action_delegation.js +++ b/test/js/unit/test_plugin_action_delegation.js @@ -24,7 +24,7 @@ function slice(startMarker, endMarker) { return SRC.slice(a, b); } -const GLOBAL_DELEGATION = slice('(function setupGlobalEventDelegation() {', '// Note: configurePlugin'); +const GLOBAL_DELEGATION = slice('(function setupGlobalEventDelegation() {', '// GitHub Token Collapse Handler'); const HANDLER = slice('function handlePluginAction(event) {', 'function findInstalledPlugin(pluginId)'); let pass = 0, fail = 0; diff --git a/test/js/unit/test_render_cards.js b/test/js/unit/test_render_cards.js index 8faae80c..741a7824 100644 --- a/test/js/unit/test_render_cards.js +++ b/test/js/unit/test_render_cards.js @@ -30,12 +30,13 @@ global.document = { createElement: () => new FakeEl(), }; global.window = global; +require('../led_escape').install(window); global.pluginLog = () => {}; global.PLUGIN_DEBUG = false; global.debugLog = () => {}; function setupInstalledEventDelegation() {} // stubbed; tested separately -eval(slice('function escapeHtml(text)', '\nfunction ', )); +eval(slice('function escapeHtml(text)', '\nfunction isNewPlugin')); eval(slice('function renderInstalledCards(plugins, total)', '// Set up event delegation for plugin action buttons')); diff --git a/web_interface/static/v3/app.js b/web_interface/static/v3/app.js index 51b6f377..e683e6cd 100644 --- a/web_interface/static/v3/app.js +++ b/web_interface/static/v3/app.js @@ -1,6 +1,37 @@ /* global showNotification */ -// LED Matrix v3 JavaScript -// Additional helpers for HTMX and Alpine.js integration +/* + * app.js -- page-wide behaviour that is not the Alpine app itself. + * + * Deferred, first of the scripts at the end of ; after app-shell.js + * and Alpine. + * + * Load order (templates/v3/base.html): + * , blocking: debugLog and theme inline scripts; the htmx loader + * (injects htmx.min.js with a dynamic - + @@ -246,45 +301,6 @@ }); - - - - - - - - @@ -548,7 +564,7 @@