fix: settings page dead on load (TDZ), release v1.18.1 #52

Merged
365DevNet merged 0 commits from refs/pull/52/head into main 2026-08-03 22:17:17 +02:00
365DevNet commented 2026-08-03 22:15:56 +02:00 (Migrated from codeberg.org)

Summary

The settings page rendered its header, search box and footer but no topics and no settings at all. Reported against 1.18.0. 1.17.0 has the same fault — this has been broken since that release, not introduced by 1.18.0.

This is my bug, and it should not have reached you.

Cause

settings.js:400  let currentSettings = normalizeSettingsState(SYNC_SETTINGS_DEFAULTS);
       -> :479  normalizeSettingsState -> normalizeMaximumVideoAge(...)
       -> :424  reads MAX_VIDEO_AGE_PRESETS   <- const declared at :421, below this point

normalizeSettingsState runs at module top level. Function declarations hoist, so the call resolved fine — but const MAX_VIDEO_AGE_PRESETS was declared ~20 lines further down and was still in its temporal dead zone. Reading it threw ReferenceError, which aborted the script, so every statement after line 400 — including the entire page render — never ran. The header and footer are static HTML, which is why the shell still looked plausible.

Introduced in 4806bbc (v1.17.0) alongside the max-video-age filter.

Fix

MAX_VIDEO_AGE_PRESETS moved up with the other module constants, above every use, with a comment explaining why its position is load-bearing.

Why it escaped review — and what now prevents it

node --check passes on the broken file. It is syntactically valid; it fails only when executed. Nothing in the suite executed the page scripts, so nothing caught it.

What stings is that I caught this exact bug class in defaults.js during #51, and only because I happened to run that file rather than just syntax-check it. I did not apply the lesson to settings.js.

test/page-load.test.js now closes that gap: it loads settings, dashboard, popup and help the way the browser does — HTML plus scripts in manifest order, with a stubbed chrome API — and fails on any uncaught error or window error event. It also asserts the settings page actually renders its topic list and controls, because the failure left a convincing-looking shell behind.

Verified the test catches the bug: re-introduced the fault and confirmed 3 tests fail; restored the fix and all pass. A regression test that does not fail on the regression is worthless.

68 → 74 tests. dashboard.html, popup.html and help.html all load clean.

Release

  • v1.18.1, website/downloads/cleanwatch-latest.zip rebuilt, byte-identical to the store package.
  • Sanity-checked the built artifact, not just the working tree: extracted the zip, loaded its settings.html + scripts, and confirmed it renders 196 controls.

Action needed on the store

The 1.18.0 build in review has this bug. If it can still be withdrawn, replace it with 1.18.1 rather than letting it go live. If it has already been approved, 1.18.1 should be uploaded immediately — a blank settings page is the kind of thing that draws one-star reviews fast.

Test plan

  • npm test — 74/74
  • Regression test verified to fail with the bug re-introduced
  • Packaged zip loads and renders 196 controls
  • Load unpacked 1.18.1 and open Settings, Dashboard, Popup and Help
## Summary The settings page rendered its header, search box and footer but **no topics and no settings at all**. Reported against 1.18.0. **1.17.0 has the same fault** — this has been broken since that release, not introduced by 1.18.0. This is my bug, and it should not have reached you. ## Cause ``` settings.js:400 let currentSettings = normalizeSettingsState(SYNC_SETTINGS_DEFAULTS); -> :479 normalizeSettingsState -> normalizeMaximumVideoAge(...) -> :424 reads MAX_VIDEO_AGE_PRESETS <- const declared at :421, below this point ``` `normalizeSettingsState` runs **at module top level**. Function declarations hoist, so the call resolved fine — but `const MAX_VIDEO_AGE_PRESETS` was declared ~20 lines further down and was still in its **temporal dead zone**. Reading it threw `ReferenceError`, which aborted the script, so every statement after line 400 — including the entire page render — never ran. The header and footer are static HTML, which is why the shell still looked plausible. Introduced in `4806bbc` (v1.17.0) alongside the max-video-age filter. ## Fix `MAX_VIDEO_AGE_PRESETS` moved up with the other module constants, above every use, with a comment explaining why its position is load-bearing. ## Why it escaped review — and what now prevents it `node --check` passes on the broken file. It is *syntactically valid*; it fails only when executed. Nothing in the suite executed the page scripts, so nothing caught it. What stings is that **I caught this exact bug class in `defaults.js` during #51**, and only because I happened to run that file rather than just syntax-check it. I did not apply the lesson to `settings.js`. `test/page-load.test.js` now closes that gap: it loads **settings, dashboard, popup and help** the way the browser does — HTML plus scripts in manifest order, with a stubbed `chrome` API — and fails on any uncaught error or `window` error event. It also asserts the settings page actually renders its topic list and controls, because the failure left a convincing-looking shell behind. **Verified the test catches the bug**: re-introduced the fault and confirmed 3 tests fail; restored the fix and all pass. A regression test that does not fail on the regression is worthless. 68 → 74 tests. `dashboard.html`, `popup.html` and `help.html` all load clean. ## Release - **v1.18.1**, `website/downloads/cleanwatch-latest.zip` rebuilt, byte-identical to the store package. - Sanity-checked the **built artifact**, not just the working tree: extracted the zip, loaded its `settings.html` + scripts, and confirmed it renders **196 controls**. ## Action needed on the store **The 1.18.0 build in review has this bug.** If it can still be withdrawn, replace it with 1.18.1 rather than letting it go live. If it has already been approved, 1.18.1 should be uploaded immediately — a blank settings page is the kind of thing that draws one-star reviews fast. ## Test plan - [x] `npm test` — 74/74 - [x] Regression test verified to fail with the bug re-introduced - [x] Packaged zip loads and renders 196 controls - [ ] Load unpacked 1.18.1 and open Settings, Dashboard, Popup and Help
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
365DevNet/CleanWatch!52
No description provided.