Re-verified this CodeRabbit finding, previously deferred as "needs isolated
testing" - traced it to a confirmed, active bug rather than a style
concern:
htmx 1.9.10's own config defaults to allowScriptTags: true (confirmed in
the vendored htmx.min.js, which itself contains the same clone-and-reinsert
<script> mechanism internally). This means htmx ALREADY re-executes every
<script> tag in swapped content by default, exactly like a browser
navigating to a new page. The custom htmx:afterSwap listener in
htmx-config.js did the identical clone-and-reinsert a SECOND time on top of
htmx's own handling - so every inline <script> block in every HTMX-loaded
partial (overview, display, durations, plugin config, etc. - most partials
have one) executed twice per load.
Confirmed safe to delete outright, not just narrow: grepped every hand-written
JS file for a manual `dispatchEvent(... 'htmx:afterSwap' ...)` that might
have relied on this handler for a non-htmx code path (e.g. the direct-fetch
fallbacks like loadOverviewDirect) - none exists, so nothing depended on
this listener specifically; htmx's native handling covers every real
htmx-driven swap on its own.
Left in place, unchanged: the console.error/console.warn global override
a few lines up in the same file, which suppresses known-noisy
HTMX-timing-race messages. That one is a legitimate anti-pattern too
(broad substring matching can mask unrelated errors) but redesigning it
needs care to preserve real diagnostics while still hiding the specific
harmless races it targets - a scoped follow-up, not a same-day deletion
like this confirmed-duplicate handler.
Validation: all 27 fast web tests pass; JS brace/paren balance sanity
checked (no local Node/browser available in this sandbox to execute the
file directly - verify manually in-browser before merge).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ
- htmx-config.js: two more unused catch bindings dropped (optional catch
binding), matching the earlier fix.
- app-early.js: second forEach arrow (the stub updatePluginTabs copy)
braced so the callback no longer returns tab.remove()'s value.
The other 4 findings ("Non-serializable expression must be wrapped with
$(...)") are deliberately NOT "fixed": that rule belongs to a
browser-automation (WebdriverIO-style) lint context and is misfiring on
ordinary arrow-function constants. Converting them to function
declarations would look compliant but BREAK the code - all four arrows
intentionally capture the enclosing Alpine component's `this` for the
stub-to-full enhancement logic. The right remedy is disabling that
pattern for this repo in Codacy's Code Patterns settings (or dismissing
the four findings), not a code change.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ
Verified each finding against current code. Fixed:
- api_v3: plugin_rotation_order is now strictly validated (JSON list of
strings, 400 with a descriptive message otherwise) and popped from the
payload before any further handling.
- display_controller: _apply_plugin_rotation_order defensively ignores a
non-list value (keeps the existing rotation, logs a warning) and drops
non-string entries; new logs carry the [DisplayController] prefix.
Unit-tested both defensive paths.
- app.py: snapshot-read handler narrowed to OSError with debug logging;
flask-compress ImportError now emits one structured warning with the
install remedy.
- htmx-config: the response-error logger prints form FIELD NAMES only -
values (API keys, passwords) never reach the console.
- plugin-order-list: saved order/exclusions normalized with Array.isArray
(a saved "null" previously crashed .forEach); each row gained
keyboard/touch-accessible move-up/move-down buttons (HTML5 drag events
don't fire on most mobile browsers) that reorder and syncInputs()
immediately alongside native drag.
- app-shell: window.installedPlugins setter always takes the new list
(same-ID metadata/enabled updates were silently dropped); tab rebuild
stays gated on ID changes. LED dot renderer reads the frame with ONE
getImageData call instead of one per pixel (~9,200/frame at 192x48).
- plugins_manager: togglePlugin returns its request promise resolving the
API outcome; the install flow now shows the "installed and enabled"
toast (with Restart Now) only after enablement succeeds, and a warning
without a restart offer when it fails.
- a11y: hamburger aria-label flips Open/Close with drawer state; both
Advanced-section toggle buttons declare aria-controls/aria-expanded and
the shared toggleSection() keeps aria-expanded in sync; move buttons
have per-plugin aria-labels.
- Rotation/Vegas order-list bootstraps cap their retries (~5s) and show a
reload hint instead of spinning forever; Alpine app-state lookups prefer
[x-data="app()"] with a generic fallback.
Skipped, with reasons:
- executePluginAction arg order: caller (plugin_config.html) already
passes (actionId, index, pluginId) matching the signature exactly.
- generateFieldHtml XSS, entity-unescape blocks, dotToNested pollution,
and "app.loadInstalledPlugins" in app-shell: all inside the legacy
client-side config cluster whose entry points are shadowed by
plugins_manager.js / replaced by server-rendered forms (zero live
callers, verified) - queued for wholesale deletion in the follow-up
rather than patching dead code.
- custom-feeds-helpers.js findings (3): file was deleted in a prior commit.
- console.error/warn override removal and afterSwap script re-execution
removal: deliberate pre-existing workarounds every partial's inline
init currently depends on; reworking them safely needs isolated testing
(follow-up), and the error suppression is already double-gated
(insertBefore AND htmx match).
- "move durations bootstrap into a bundle": inline partial-scoped init is
the established pattern for HTMX partials in this codebase.
Validation: all 40 web tests pass; py_compile on all touched Python; all
touched templates parse; rotation-order defensive paths unit-tested.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ
Verified each of the 27 reported findings against current code; all fixed
except one rule class skipped with reason below.
- app-early.js: plugin tab buttons are now built with createElement/
createTextNode instead of innerHTML template strings (icon class and name
come from plugin manifests - semi-trusted input; the old code escaped the
name but interpolated the icon class). Both construction sites. Also the
forEach arrow no longer returns tab.remove()'s value.
- plugin-order-list.js: rows, empty state, and error state all built with
DOM APIs - the file no longer contains innerHTML at all (the now-unneeded
escapeHtml/escapeAttr helpers are removed); MODE_LABELS is a Map so the
vegas-mode lookup can't hit prototype properties.
- notification.js: actionCallbacks is a Map (get/set/delete) instead of a
plain object - resolves the object-injection-sink and dynamic-delete
findings; triggerAction also type-checks the callback.
- htmx-config.js: unused catch binding dropped; var -> const in the
afterSettle handler; the swapped-<script> re-execution reads/writes
textContent instead of innerHTML; the diagnostic form payload uses a
null-prototype object so a field named __proto__ can't pollute.
- custom-feeds-helpers.js DELETED (with its script tag): all three of its
functions (addCustomFeedRow, removeCustomFeedRow,
handleCustomFeedLogoUpload) are shadowed by the deferred
widgets/custom-feeds.js window assignments, which always win at call time
- the copies were dead even when they lived inline in base.html. This
also resolves the unused-function and unused-variable findings there.
Skipped: 4x "Non-serializable expression must be wrapped with $(...)" in
app-early.js - that rule targets code crossing a browser-automation
serialization boundary (e.g. page.evaluate); these are ordinary arrow
functions in plain browser code with no such boundary.
Validation: all 40 web tests pass (incl. the static-asset reference audit,
which confirms no template still points at the deleted file); Jinja parse OK.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ
- Add debugLog to the /* global */ headers of the six JS files that call it
(defined in base.html's first inline script) — resolves the wall of
"'debugLog' is not defined" ESLint errors failing the Codacy check.
- Fix the two js/double-escaping CodeQL alerts in app-shell.js: the
entity-unescape chains decoded & before </>, so a value
containing a pre-escaped "&lt;" wrongly double-decoded to "<".
& now decodes last (standard order). Pre-existing bug, made visible
when the inline scripts moved into scannable .js files.
- Page title / header drop the "- v3" suffix, matching the de-versioned
user-facing URL.
The remaining 7 CodeQL alerts are pre-existing patterns newly visible to
scanning (CodeQL doesn't see inline template JS): 4 github.com/htmx.org
URL-substring checks (the htmx ones match error-message text, not URLs —
false positives in context) and 1 innerHTML XSS-through-DOM in the GitHub
install flow. Triage/fix deferred to a focused follow-up rather than
expanding this PR.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ
base.html shipped ~4,200 lines of inline JavaScript inside the HTML
document, re-downloaded and re-parsed on every page load (gzip helps the
transfer, but inline scripts can never be browser-cached). The four
largest blocks - none containing any Jinja syntax, verified by scanning
every inline block for {{ }} / {% %} - now live as static files served
with the app's existing mtime-versioned immutable caching:
- js/htmx-config.js (246 lines): HTMX swap/script-execution config,
toggleSection helpers
- js/app-early.js (346 lines): early helpers + the app() stub that must
precede Alpine init
- js/app-shell.js (2,997 lines): SSE wiring + the full Alpine app()
implementation and tab logic
- js/custom-feeds-helpers.js (262 lines): custom-feeds table helpers
Each replacement <script src> is CLASSIC (no defer/async) at the exact
position of the inline block it replaces - identical execution timing and
DOM visibility to inline scripts, so relative ordering with the deferred
scripts and with each other is unchanged. base.html drops from ~4,940 to
1,079 lines.
Validation: extraction proven lossless by programmatically reassembling
the four files back into the template and comparing against git HEAD -
byte-for-byte identical. Jinja parse passes; script open/close tags
balanced (53/53, after excluding a literal "<script>" inside an HTML
comment).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ
