Repository navigation
fix(1462): pair the admin System Log table with a mobile card mirror - #1463
Merged
Merged
Conversation
`web/themes/default/page_admin_settings_logs.tpl` rendered the log
entries inside a `<table class="table">`. At `<=768px` the global
`.table { display: none; }` rule (originally paired with the
`.ban-cards` mobile mirror on the bans / comms lists) collapsed
every panel-side table; the System Log never shipped a paired
mobile surface, so on mobile the chrome around the table
rendered fine (page heading, "Clear log" button, pagination
footer) while the actual log rows were silently hidden — the
reporter's exact "no logs visible" symptom on the iPhone view.
Adds a `<ul class="log-cards">` block alongside the existing
table that mirrors `$log_items` as native `<details>`
disclosures. Native `<details>` carries the disclosure semantic
for free — keyboard-reachable, screen-reader-announced,
JS-free — so the mobile surface doesn't have to repeat the
desktop table's `role="button"` + `tabindex="0"` + `onkeydown`
triple (the desktop triple is unavoidable on `<tr>`, which has
no native disclosure semantic). Paired CSS: default
`.log-cards { display: none; }` + the matching `<=768px`
`display: block` and `>=769px` `display: none` rules next to
the sibling `.ban-cards` declarations so the surfaces never
compete at intermediate viewports.
Sister fix to the mobile surface — `page_admin_settings_logs.tpl`
also carried a pre-existing Smarty `auto_literal` hazard on
the desktop `<tr>`'s `onkeydown="if(...){event.preventDefault();...}"`
handler (added in #1451). The inline `{event.preventDefault();...}`
substring has no whitespace after the opening brace, so Smarty
parses it as a tag and the template fails fresh-compile with
`Unexpected '.', expected one of: '}'`. The bug compiled cleanly
for months because Smarty's compile-dir was caching an earlier
artifact; the moment an E2E spec exercises the page on a cold
`templates_c` the page 500s. Lifts the dispatcher into the
existing `<script>{literal}…{/literal}</script>` block as
`window.handleLogRowKey(event, row)` and the attribute reduces
to `onkeydown="handleLogRowKey(event, this);"` — no braces in
the attribute body, no Smarty hazard.
Regression guards (8 PHPUnit + 5 Playwright):
- `SystemLogMobileCardsRegressionTest.php` pins the mobile
wrapper, the per-row `<details>` shape, the surface-parity
contract (both surfaces iterate the same `$log_items`), the
CSS visibility ladder (default-hidden, `<=768px` shown,
`>=769px` hidden), the surviving desktop table, and the
Smarty hazard — the gate matches the broader `on\w+="…{<JS-identifier>…"`
bug class, not just the specific `onkeydown` + `{event.`
shape that motivated it, so a future copy-paste into a
sibling `onclick` / `onkeyup` / `onfocus` attribute surfaces
here instead of in CI on a cold compile-dir.
- `responsive/system-log-mobile.spec.ts` (mobile-chromium) drives
the seeded fixture end-to-end: desktop hidden + mobile cards
paint, `<details>` toggles via pointer click, `<details>`
toggles via Tab → Enter / Space (the keyboard contract the
abstraction was picked FOR), zero critical axe violations on
the expanded mobile chrome, and no horizontal viewport overflow.
Cards are anchored on the `data-id="<lid>"` primary key returned
by `seedSystemLogE2e` rather than a `hasText: <title>` substring,
so a future audit row with an overlapping title prefix can't
flake the spec.
Companion seeder: `seedSystemLogE2e(rows[]): Promise<number[]>`
in `web/tests/e2e/fixtures/db.ts` shells out to a new
`seed-system-log-e2e.php` shim that inserts directly into
`:prefix_log` and returns the inserted lid list. The shim
mirrors `seed-comms-e2e.php`'s contract (DB_NAME guard refuses
anything but `sourcebans_e2e`, stdin JSON, stdout JSON
response). `:prefix_log` is empty by default in the e2e seed
and there's no JSON action that emits audit rows directly —
they're side effects of authenticated panel writes — so the
direct INSERT is the narrowest shape that doesn't couple the
spec to an unrelated handler's audit message.
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
web/themes/default/page_admin_settings_logs.tplrendered the audit log inside a<table class="table">with no paired mobile surface. At<=768pxthe global.table { display: none; }rule (shared with bans / comms whose mobile shape is.ban-cards) collapsed the table, so the chrome around it (heading, "Clear log" button, pagination) painted while the rows themselves silently vanished. Reporter's "no logs visible" iPhone screenshot in Mobile view - Admin System log not displaying #1462 is the exact symptom.<ul class="log-cards">block alongside the existing table that mirrors$log_itemsas native<details>disclosures, paired with>=769px/<=768pxCSS visibility rules next to the sibling.ban-cardsdeclarations so the two surfaces never compete. Native<details>carries the disclosure semantic for free — keyboard-reachable, screen-reader-announced, JS-free — so the mobile surface doesn't have to repeat the desktop<tr>'srole="button"+tabindex="0"+onkeydowntriple.auto_literalhazard in the same template (the pre-existingonkeydown="if(...){event.preventDefault();...}"handler on the desktop<tr>, added in fix(1443): drop misleading row-widecursor: pointeron data tables #1451). The inline{event.preventDefault()…}substring has no whitespace after the opening brace, so Smarty parses it as a tag — the bug compiled cleanly for months becausetemplates_cwas cached, but the moment an E2E spec hit the page with a cold compile-dir the page 500'd. Lifts the dispatcher into the existing<script>{literal}…{/literal}</script>block aswindow.handleLogRowKey(event, row)and the attribute drops toonkeydown="handleLogRowKey(event, this);"— no braces in the attribute body.Regression guards
web/tests/integration/SystemLogMobileCardsRegressionTest.php(8 contracts) pins the mobile wrapper, the per-row<details>shape, the surface-parity contract (both surfaces iterate$log_items), the CSS visibility ladder (default-hidden,<=768pxshown,>=769pxhidden), the surviving desktop table, AND the Smarty hazard. The hazard regex matches the broaderon\w+="…{<JS-identifier>…"bug class, not just the specificonkeydown+{event.shape that motivated it, so a future copy-paste into a siblingonclick/onkeyup/onfocussurfaces here instead of in CI on a cold compile-dir. Smarty{* *}and JS/* */comments are stripped before the match so docblocks documenting the bug don't false-fire.web/tests/e2e/specs/responsive/system-log-mobile.spec.ts(5 specs, mobile-chromium) drives the seeded fixture end-to-end: desktop hidden + mobile cards paint,<details>toggles via pointer click,<details>toggles via Tab → Enter / Space (the keyboard contract the abstraction was picked FOR), zero critical axe violations on the expanded mobile chrome, and no horizontal viewport overflow. Cards anchor on thedata-id="<lid>"primary key returned by the seeder rather thanhasText:<title>, so a future audit row with an overlapping title prefix can't flake the spec.test.describe.configure({ mode: 'serial' })+beforeEachtruncate-and-seed pairs the spec with the sharedsourcebans_e2eDB contract.seedSystemLogE2e(rows[]): Promise<number[]>inweb/tests/e2e/fixtures/db.tsshells out to a newseed-system-log-e2e.phpshim that inserts directly into:prefix_logand returns the inserted lid list. The shim mirrorsseed-comms-e2e.php's contract (DB_NAMEguard refuses anything butsourcebans_e2e, stdin JSON in / stdout JSON out).:prefix_logis empty by default in the e2e seed and there's no JSON action that emits audit rows directly — they're side effects of authenticated panel writes — so the direct INSERT is the narrowest shape that doesn't couple the spec to an unrelated handler's audit message.Test plan
./sbpp.sh phpstan— clean./sbpp.sh ts-check— clean./sbpp.sh test— PHPUnit green (incl. newSystemLogMobileCardsRegressionTest's 8 contracts)./sbpp.sh composer api-contract— no diff./sbpp.sh e2e --project=mobile-chromium specs/responsive/system-log-mobile.spec.ts --workers=1— 5/5 green./sbpp.sh e2e --workers=1— full suite (mobile-chromium + chromium) green?p=admin&c=settings§ion=logsat iPhone 13 viewport, confirmed cards render with timestamps + admin name + IP + log-type pills, click-to-expand reveals the message body, keyboard Tab → Enter / Space toggles the disclosure, desktop-width viewport hides the cards and surfaces the existing table.Closes #1462.