Repository navigation
fix(1438): paint chromeless <head> surfaces in dark mode on first paint - #1450
Merged
Merged
Conversation
…aint The kickit page (and three sister surfaces) shipped no anti-FOUC bootloader, so a dark-mode operator who right-clicked a player on the public Servers page → "Kick player" landed on a stark-white full-page kickit document — the templates don't pull `theme.js`, so without the inline `<script>` in `<head>` there is no theme-resolution path at all and the page paints in light against the `:root` token defaults. Mirrors the chrome's #1367 inline bootloader pattern across every chromeless `<head>` surface in the panel: - `page_kickit.tpl` (the #1438 reporter's path; iframe AND top-level) - `page_blockit.tpl` (parity with kickit — same shape) - `page_uploadfile.tpl` (upload popup over a dark-mode parent) - `updater.tpl` (post-upgrade landing during a dark-mode admin's flow) The bootloader's resolution rule is semantically equivalent to `theme.js`'s `applyTheme(currentTheme())` minus the `localStorage.setItem` write (theme.js still owns persistence). It's intentionally more defensive than theme.js (`window.matchMedia &&` null check + a broader `try/catch` covering both `localStorage` SecurityError and missing `matchMedia`) so it cannot throw on the earliest realm setup phase where a syntax error would silently leave the page in light mode. Regression guards (two halves): - `web/tests/integration/IframeChromeAntiFoucBootloaderTest.php` (static-grep): asserts every required bootloader fragment ships in all five copies, the bootloader precedes `<link rel="stylesheet">` in each, the bootloader bodies are byte-equivalent across the five copies after whitespace normalization (catches drift edits that update one copy and forget the others), and NO `*.tpl` under `web/themes/default/install/` carries the bootloader (the install wizard runs against an unconfigured panel with no `localStorage` to read — exemption documented in AGENTS.md). - `web/tests/e2e/specs/flows/iframe-anti-fouc.spec.ts` (runtime): six chromium tests cover kickit dark + light + system-OS-dark + system-OS-light + blockit dark + uploadfile dark. The system-OS-light arm specifically guards against the regression mode where the bootloader unconditionally adds `html.dark` regardless of `matchMedia(...).matches`. The updater surface is NOT covered by the E2E spec because the dev stack auto-seeds the DB out of band via `docker/db-init/`, so `web/updater/data/<N>.php` migrations are never applied via the runner in dev and hitting `/updater/` raises "Column already exists" on migration 801. The static-grep integration test is the sufficient gate for that surface (the bootloader mechanism is identical across all five copies, enforced by the byte-equivalence test). `AGENTS.md` updated in lockstep (the "Anti-FOUC theme bootloader" Conventions block, the "Apply the persisted theme" row in "Where to find what", and the two paired Anti-patterns entries) to document the five-surface contract and the semantic-vs-byte equivalence distinction. Fixes #1438.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
theme.js, so without the inline<script>in<head>there is no theme-resolution path at all and the page paints in light against the:roottoken defaults.<head>surface in the panel:page_kickit.tpl,page_blockit.tpl(parity),page_uploadfile.tpl(popup over a dark-mode parent admin), andupdater.tpl(post-upgrade landing page during a dark-mode admin's flow).theme.js'sapplyTheme(currentTheme())minus thelocalStorage.setItemwrite (theme.js still owns persistence). It's intentionally more defensive than theme.js (window.matchMedia &&null check + a broadertry/catchcovering bothlocalStorageSecurityError and missingmatchMedia) so it cannot throw on the earliest realm setup phase where a syntax error would silently leave the page in light mode.Adversarial-review findings addressed
An adversarial review found four chromeless surfaces (not just two):
page_uploadfile.tpl(upload popup) andupdater.tpl(post-upgrade landing). Both have the same stark-white-over-dark-parent failure shape. Fixed — added the bootloader to both.testBootloaderBodiesAreEquivalentAfterNormalization()that extracts the IIFE body from each copy and asserts byte-for-byte equality after whitespace normalization._chrome.tpl. Fixed —testInstallWizardTemplatesDoNotCarryBootloader()now globs every*.tplunderweb/themes/default/install/.theme.js. The bootloader is intentionally more defensive. Fixed — AGENTS.md + the integration test docblock now distinguish semantic equivalence with theme.js from byte equivalence across copies.file_get_contentserrors. Fixed — removed the@.Regression guards
web/tests/integration/IframeChromeAntiFoucBootloaderTest.php(static-grep): asserts every required bootloader fragment ships in all five copies (core/header.tpl,page_kickit.tpl,page_blockit.tpl,page_uploadfile.tpl,updater.tpl), the bootloader precedes<link rel=\"stylesheet\">in each, the bootloader bodies are byte-equivalent across the five copies after whitespace normalization (catches drift edits that update one copy and forget the others), and NO*.tplunderweb/themes/default/install/carries the bootloader (the install wizard runs against an unconfigured panel with nolocalStorageto read — exemption documented in AGENTS.md). 4 tests, 8 assertions.web/tests/e2e/specs/flows/iframe-anti-fouc.spec.ts(runtime): six chromium tests cover kickit dark + light + system-OS-dark + system-OS-light + blockit dark + uploadfile dark. The system-OS-light arm specifically guards against the regression mode where the bootloader unconditionally addshtml.darkregardless ofmatchMedia(...).matches(the inverse of system-OS-dark — both arms together pin the full predicate). 6 passed, 6 mobile-chromium skipped (no admin storage state).The updater surface is NOT covered by the E2E spec because the dev stack auto-seeds the DB out of band via
docker/db-init/, soweb/updater/data/<N>.phpmigrations are never applied via the runner in dev and hitting/updater/raises "Column already exists" on migration 801. The static-grep integration test is the sufficient gate for that surface (the bootloader mechanism is identical across all five copies, enforced by the byte-equivalence test).Docs
AGENTS.mdupdated in lockstep:<html>BEFORE first paint" row in "Where to find what" rewritten to cover all five copies, both regression guards, and the updater-spec-exemption rationale.Test plan
./sbpp.sh test tests/integration/IframeChromeAntiFoucBootloaderTest.php— 4 passed./sbpp.sh e2e specs/flows/iframe-anti-fouc.spec.ts— 6 passed, 6 skipped./sbpp.sh e2e specs/flows/theme-fouc.spec.ts— 6 passed (no regression in chrome bootloader)./sbpp.sh phpstan— 241 files, no errors./sbpp.sh ts-check— no errorsFixes #1438.