fix: settings page dead on load (TDZ), release v1.18.1 #52
Loading…
Reference in a new issue
No description provided.
Delete branch "refs/pull/52/head"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
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
normalizeSettingsStateruns at module top level. Function declarations hoist, so the call resolved fine — butconst MAX_VIDEO_AGE_PRESETSwas declared ~20 lines further down and was still in its temporal dead zone. Reading it threwReferenceError, 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_PRESETSmoved 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 --checkpasses 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.jsduring #51, and only because I happened to run that file rather than just syntax-check it. I did not apply the lesson tosettings.js.test/page-load.test.jsnow closes that gap: it loads settings, dashboard, popup and help the way the browser does — HTML plus scripts in manifest order, with a stubbedchromeAPI — and fails on any uncaught error orwindowerror 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.htmlandhelp.htmlall load clean.Release
website/downloads/cleanwatch-latest.ziprebuilt, byte-identical to the store package.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