From 047ead9de13c17868b31e7836cf7f612078ab6f8 Mon Sep 17 00:00:00 2001 From: Maximiliano Jabase Date: Wed, 5 Aug 2026 20:32:58 -0300 Subject: [PATCH 1/4] polish mods list add and edit chrome --- web/pages/admin.edit.mod.php | 2 - web/themes/default/css/theme.css | 7 ++ web/themes/default/page_admin_edit_mod.tpl | 21 ++-- web/themes/default/page_admin_mods_add.tpl | 19 +-- web/themes/default/page_admin_mods_list.tpl | 127 ++++++++++++++++---- 5 files changed, 135 insertions(+), 41 deletions(-) diff --git a/web/pages/admin.edit.mod.php b/web/pages/admin.edit.mod.php index 75e548951..7d7072097 100644 --- a/web/pages/admin.edit.mod.php +++ b/web/pages/admin.edit.mod.php @@ -12,8 +12,6 @@ global $userbank, $theme; -new \Sbpp\View\AdminTabs([], $userbank, $theme); - require_once __DIR__ . '/_admin_edit_helpers.php'; $modId = isset($_GET['id']) ? (int) $_GET['id'] : 0; diff --git a/web/themes/default/css/theme.css b/web/themes/default/css/theme.css index e4ccefca1..5fd064729 100644 --- a/web/themes/default/css/theme.css +++ b/web/themes/default/css/theme.css @@ -2130,6 +2130,11 @@ details.queue-row > summary > .row-actions { .admins-list-card:last-child { border-bottom: none; } .admins-list-card__body { padding: 0.75rem 1rem 0.25rem; } +.mods-list-cards { display: none; } +.mods-list-card { border-bottom: 1px solid var(--border); } +.mods-list-card:last-child { border-bottom: none; } +.mods-list-card__body { padding: 0.75rem 1rem 0.25rem; } + /* ---- Responsive ---- */ [data-mobile-menu] { display: none; } @media (max-width: 1024px) { @@ -2148,6 +2153,7 @@ details.queue-row > summary > .row-actions { dance as `.ban-cards` — hidden at desktop, block at mobile. */ .log-cards { display: block; } .admins-list-cards { display: block; } + .mods-list-cards { display: block; } /* #1181: filter chip rows wrap onto multiple lines on mobile instead of horizontal-scrolling, so every chip is reachable without a swipe. The .scroll-x desktop affordance is the @@ -2159,6 +2165,7 @@ details.queue-row > summary > .row-actions { .ban-cards { display: none; } .log-cards { display: none; } .admins-list-cards { display: none; } + .mods-list-cards { display: none; } } /* ---- Utility classes used by templates ---- */ diff --git a/web/themes/default/page_admin_edit_mod.tpl b/web/themes/default/page_admin_edit_mod.tpl index b8b70cd33..83f08a0ad 100644 --- a/web/themes/default/page_admin_edit_mod.tpl +++ b/web/themes/default/page_admin_edit_mod.tpl @@ -27,7 +27,18 @@ template variable — no MooTools-era `$('enabled').checked = …` re-paint script. *} -
+
+
+ {* nofilter: name is htmlspecialchars'd on store in admin.edit.mod.php; + auto-escaping here would double-encode (#1108 / #1113). *} +

+ Edit mod · {$name nofilter} +

+

+ Update the configuration for this game mod. +

+
+
{csrf_field}
-
-
-

Edit Mod

-

Update the configuration for this game mod.

-
-
-
+
{* nofilter: mod metadata is htmlspecialchars(strip_tags($_POST[…]))'d in admin.edit.mod.php before INSERT/UPDATE, so values pulled back out of `:prefix_mods` are already entity-encoded; auto-escaping the value attribute would double-encode (#1113 audit). The id="icon_hid" element is the channel the popup uploader writes into via window.opener.icon(). *} diff --git a/web/themes/default/page_admin_mods_add.tpl b/web/themes/default/page_admin_mods_add.tpl index beba3bc54..24743e6d1 100644 --- a/web/themes/default/page_admin_mods_add.tpl +++ b/web/themes/default/page_admin_mods_add.tpl @@ -42,7 +42,7 @@ without depending on element ids. The icon-callback hidden input is `#icon_hid` to mirror the edit-mod template. *} -
+
{if NOT $permission_add}
@@ -50,6 +50,15 @@
{else} +
+

+ Add mod +

+

+ Configure a new game mod that can be assigned to bans and servers. +

+
+ {csrf_field}
-
-
-

Add Mod

-

Configure a new game mod that can be assigned to bans and servers.

-
-
-
+
{* #1402: the legacy `` hidden was a vestigial reference to the v1.x ProcessMod() flow; the new submit handler has no equivalent. Replaced with `#icon_hid` so the upload-icon diff --git a/web/themes/default/page_admin_mods_list.tpl b/web/themes/default/page_admin_mods_list.tpl index 290fca42f..273dc1bbe 100644 --- a/web/themes/default/page_admin_mods_list.tpl +++ b/web/themes/default/page_admin_mods_list.tpl @@ -43,15 +43,20 @@
{else} -
-
-
-

Server Mods

-

{$mod_count} configured

-
-
+
+

+ Mods + ({$mod_count}) +

+

+ Game mods that can be assigned to bans and servers. +

+
+ +
{if $mod_count > 0} - +
+
@@ -67,11 +72,11 @@ {foreach from=$mod_list item=mod} {if $permission_editmods || $permission_deletemods} - @@ -126,6 +142,65 @@ {/foreach}
Name
-
+
{$mod.name} @@ -87,12 +92,20 @@ {/if}
-
+
+ {* Icon-only row actions matching banlist / admins list: + Lucide icon + `data-tooltip` + `aria-label` inside + `.row-actions--icons`. Keep `data-testid` / `data-action` + / `data-fallback-href` wiring unchanged. *} +
{if $permission_editmods} - Edit + data-testid="editmod-link" + data-tooltip="Edit" + aria-label="Edit mod {$mod.name|escape}"> + + {/if} {if $permission_deletemods} {* #1397: data-action wires the delete button to the inline @@ -110,14 +123,17 @@ beyond the bug; the fallback is a graceful degradation for the rare case where the JSON dispatcher itself is missing (e.g. third-party theme that stripped api.js). *} - + data-tooltip="Delete" + aria-label="Delete mod {$mod.name|escape}"> + + {/if}
+
+ + {* Mobile cards — paired surface for the global + `@media (max-width: 768px) { .table { display: none } }` + rule. Same display dance as `.admins-list-cards`. *} +
+ {foreach from=$mod_list item=mod} +
+
+ +
+
{$mod.name}
+
+ {$mod.modfolder} + · SU {$mod.steam_universe} +
+
+ {if $mod.enabled} + Enabled + {else} + Disabled + {/if} +
+
+
+ {if $permission_editmods || $permission_deletemods} +
+ {if $permission_editmods} + + + + {/if} + {if $permission_deletemods} + + {/if} +
+ {/if} +
+ {/foreach} +
{else}

No mods configured yet.

@@ -258,10 +333,13 @@ /** * @param {string} mid - * @returns {Element|null} + * @returns {NodeListOf} */ - function rowForMid(mid) { - return document.querySelector('[data-testid="mod-row"][data-id="' + mid + '"]'); + function rowsForMid(mid) { + return document.querySelectorAll( + '[data-testid="mod-row"][data-id="' + mid + '"],' + + '[data-testid="mods-list-card"][data-id="' + mid + '"]' + ); } /** @@ -276,7 +354,7 @@ if (!el) return; var n = Number((el.textContent || '').replace(/[^0-9]/g, '')); if (!Number.isFinite(n) || n <= 0) return; - el.textContent = String(n - 1); + el.textContent = '(' + (n - 1).toLocaleString() + ')'; } /** @returns {HTMLDialogElement|null} */ @@ -397,8 +475,11 @@ toast('error', 'Delete failed', msg); return; } - var row = rowForMid(ctx.mid); - if (row && row.parentNode) row.parentNode.removeChild(row); + var rows = rowsForMid(ctx.mid); + for (var i = 0; i < rows.length; i++) { + var row = rows[i]; + if (row && row.parentNode) row.parentNode.removeChild(row); + } decrementCount(); closeDeleteDialog(); toast('success', 'Mod deleted', ctx.name + ' has been removed.'); From bd4e05534a465343b9901255034b701859136a5c Mon Sep 17 00:00:00 2001 From: Maximiliano Jabase Date: Thu, 6 Aug 2026 00:44:00 -0300 Subject: [PATCH 2/4] fix mods count badge assertions for parenthesised format --- web/tests/e2e/specs/flows/mod-delete-confirm.spec.ts | 2 +- web/tests/integration/ModsDeleteDialogTest.php | 9 ++++----- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/web/tests/e2e/specs/flows/mod-delete-confirm.spec.ts b/web/tests/e2e/specs/flows/mod-delete-confirm.spec.ts index cacc4b000..2610b8835 100644 --- a/web/tests/e2e/specs/flows/mod-delete-confirm.spec.ts +++ b/web/tests/e2e/specs/flows/mod-delete-confirm.spec.ts @@ -210,7 +210,7 @@ test.describe('flow: admin mod delete confirm modal (#1397 — RemoveMod zombie) // Count badge decrements by exactly one (defensive — the // chrome's `decrementCount` reads the span's text and writes // back `n - 1`). - await expect(countBadge).toHaveText(String(startingCount - 1)); + await expect(countBadge).toHaveText(`(${startingCount - 1})`); // Success toast surfaces. Anchor on `data-kind="success"` plus // a hasText filter on the title our handler emits ("Mod diff --git a/web/tests/integration/ModsDeleteDialogTest.php b/web/tests/integration/ModsDeleteDialogTest.php index 095ac0493..2ccbd38f6 100644 --- a/web/tests/integration/ModsDeleteDialogTest.php +++ b/web/tests/integration/ModsDeleteDialogTest.php @@ -218,18 +218,17 @@ public function testPageTailScriptUsesActionsConstant(): void * The mod count badge must carry `data-testid="mod-count"` so the * page-tail script can decrement it after a delete and the E2E * spec can read it as the pre / post-delete oracle. Without the - * testid the badge is unreachable in a theme-agnostic way (the - * surrounding `

… configured

` shape is too brittle to - * regex against). + * testid the badge is unreachable in a theme-agnostic way. The + * rendered text is `(N)` (same shape as the Admins list badge). */ public function testCountBadgeCarriesTestid(): void { $html = $this->renderModsPage(); $this->assertMatchesRegularExpression( - '/]*data-testid="mod-count"[^>]*>\s*\d+\s*<\/span>/', + '/]*data-testid="mod-count"[^>]*>\s*\(?\d+\)?\s*<\/span>/', $html, - 'The mod count number must be wrapped in .' + 'The mod count number must be wrapped in (optionally parenthesised, matching the Admins list badge).' ); } From ca7fe6a50756cc81be552214e61decd55e8e8c08 Mon Sep 17 00:00:00 2001 From: rumblefrog Date: Sun, 6 Sep 2026 16:17:52 -0400 Subject: [PATCH 3/4] fix(admin-mods): address adversarial review findings --- web/pages/admin.edit.mod.php | 2 + web/pages/admin.mods.php | 3 +- .../specs/flows/mod-delete-confirm.spec.ts | 46 +++++++++++++--- .../integration/EditAdminTabsChromeTest.php | 32 +++++++++++ .../integration/ModsDeleteDialogTest.php | 55 ++++++++++++++++--- web/themes/default/page_admin_edit_mod.tpl | 21 ++++--- web/themes/default/page_admin_mods_list.tpl | 29 +++++----- 7 files changed, 147 insertions(+), 41 deletions(-) diff --git a/web/pages/admin.edit.mod.php b/web/pages/admin.edit.mod.php index 7d7072097..75e548951 100644 --- a/web/pages/admin.edit.mod.php +++ b/web/pages/admin.edit.mod.php @@ -12,6 +12,8 @@ global $userbank, $theme; +new \Sbpp\View\AdminTabs([], $userbank, $theme); + require_once __DIR__ . '/_admin_edit_helpers.php'; $modId = isset($_GET['id']) ? (int) $_GET['id'] : 0; diff --git a/web/pages/admin.mods.php b/web/pages/admin.mods.php index d0378f4b9..5511f9e9d 100644 --- a/web/pages/admin.mods.php +++ b/web/pages/admin.mods.php @@ -37,8 +37,9 @@ return; } +// mid=0 is the reserved Web pseudo-mod, not a configurable game mod. $mod_list = $GLOBALS['PDO']->query("SELECT * FROM `:prefix_mods` WHERE mid > 0 ORDER BY name ASC")->resultset(); -$mod_count = (int) $GLOBALS['PDO']->query("SELECT COUNT(mid) AS cnt FROM `:prefix_mods`")->single()['cnt']; +$mod_count = (int) $GLOBALS['PDO']->query("SELECT COUNT(mid) AS cnt FROM `:prefix_mods` WHERE mid > 0")->single()['cnt']; \Sbpp\View\Renderer::render($theme, new \Sbpp\View\AdminModsListView( permission_listmods: $canList, diff --git a/web/tests/e2e/specs/flows/mod-delete-confirm.spec.ts b/web/tests/e2e/specs/flows/mod-delete-confirm.spec.ts index 2610b8835..8292cb793 100644 --- a/web/tests/e2e/specs/flows/mod-delete-confirm.spec.ts +++ b/web/tests/e2e/specs/flows/mod-delete-confirm.spec.ts @@ -43,11 +43,11 @@ * * Project gating * -------------- - * Pin to chromium (desktop). The flow mutates `:prefix_mods` and the - * suite shares a single `sourcebans_e2e` DB across projects - * (`workers: 1` is the suite-wide mitigation per AGENTS.md). Mobile - * coverage doesn't add value for an admin-only chrome that's - * structurally identical at every viewport. + * Pin to chromium. The flow mutates `:prefix_mods` and the suite shares + * a single `sourcebans_e2e` DB across projects (`workers: 1` is the + * suite-wide mitigation per AGENTS.md). The main mutation flow resizes + * that desktop context to exercise the mobile-card mirror without + * adding a second mutating project. */ import { expect, test } from '../../fixtures/auth.ts'; @@ -64,6 +64,10 @@ const FIXTURE = { }; test.describe('flow: admin mod delete confirm modal (#1397 — RemoveMod zombie)', () => { + // Both tests truncate and mutate the shared E2E database. Keep focused + // local runs serial even though the project enables full parallelism. + test.describe.configure({ mode: 'serial' }); + test.skip(({ isMobile }) => isMobile, 'flow spec runs only on desktop chromium'); test.beforeEach(async () => { @@ -128,13 +132,25 @@ test.describe('flow: admin mod delete confirm modal (#1397 — RemoveMod zombie) const targetMid = Number(targetMidAttr); expect(Number.isFinite(targetMid) && targetMid > 0).toBe(true); - // The count badge text is a bare digit. Read the starting value - // so we can assert it decrements by exactly one after the delete. + // The hidden mobile mirror must be removed by the same successful + // response, otherwise resizing after deletion resurrects the mod. + const targetCard = page + .locator('[data-testid="mods-list-card"]') + .filter({ hasText: FIXTURE.name }) + .first(); + await expect(targetCard).toHaveCount(1); + + // Read the parenthesised starting value so we can assert it + // decrements by exactly one after the delete. const countBadge = page.locator('[data-testid="mod-count"]').first(); await expect(countBadge).toBeVisible(); const startingCountText = (await countBadge.textContent()) ?? ''; const startingCount = Number(startingCountText.replace(/[^0-9]/g, '')); expect(Number.isFinite(startingCount) && startingCount > 0).toBe(true); + expect(await page.locator('[data-testid="mod-row"]').count()) + .toBe(startingCount); + expect(await page.locator('[data-testid="mods-list-card"]').count()) + .toBe(startingCount); // ---- 3. Click the trash button — dialog must open ---------------- // The delete button is inside `.row-actions` on the row's @@ -177,8 +193,18 @@ test.describe('flow: admin mod delete confirm modal (#1397 — RemoveMod zombie) // The row is still there — cancel didn't delete anything. await expect(targetRow).toBeVisible(); - // ---- 5. Re-open + supply reason + confirm ----------------------- - await deleteButton.click(); + // ---- 5. Switch to mobile + re-open + confirm -------------------- + await page.setViewportSize({ width: 390, height: 844 }); + await expect(page.locator('[data-testid="mods-table"]')).toBeHidden(); + await expect(page.locator('[data-testid="mods-list-cards"]')).toBeVisible(); + await expect(targetCard).toBeVisible(); + await expect(targetCard.locator('[data-testid="editmod-link-mobile"]')).toBeVisible(); + + const mobileDeleteButton = targetCard.locator( + '[data-testid="deletemod-btn-mobile"]', + ); + await expect(mobileDeleteButton).toBeVisible(); + await mobileDeleteButton.click(); await expect(dialog).toBeVisible(); const reasonInput = dialog.locator('[data-testid="mod-delete-reason"]'); @@ -204,6 +230,7 @@ test.describe('flow: admin mod delete confirm modal (#1397 — RemoveMod zombie) // ---- 6. Row removed in place ------------------------------------ await expect(targetRow).toHaveCount(0); + await expect(targetCard).toHaveCount(0); // Dialog closes after success. await expect(dialog).toBeHidden(); @@ -221,6 +248,7 @@ test.describe('flow: admin mod delete confirm modal (#1397 — RemoveMod zombie) await expect(successToast).toBeVisible(); // ---- 7. Audit log carries the reason ---------------------------- + await page.setViewportSize({ width: 1280, height: 720 }); // Navigate to the audit log and assert the most recent // "MOD Deleted" entry's body contains the reason. The audit // page's row body is `
`. diff --git a/web/tests/integration/EditAdminTabsChromeTest.php b/web/tests/integration/EditAdminTabsChromeTest.php index d0d4dae87..fb4b89661 100644 --- a/web/tests/integration/EditAdminTabsChromeTest.php +++ b/web/tests/integration/EditAdminTabsChromeTest.php @@ -103,4 +103,36 @@ public function testEditAdminHandlersSkipOrphanBackStrip(): void ); } } + + public function testStandaloneEditModKeepsBackStrip(): void + { + $src = (string) file_get_contents(ROOT . 'pages/admin.edit.mod.php'); + + $this->assertStringContainsString( + 'new \\Sbpp\\View\\AdminTabs([],', + $src, + 'The standalone mod editor must keep the shared Back strip; it has no in-template section tabs.', + ); + } + + public function testEditModMetadataDoesNotRenderAsRawHtml(): void + { + $src = (string) file_get_contents(self::THEME . 'page_admin_edit_mod.tpl'); + + $this->assertStringContainsString( + 'Edit mod · {$name|unescape:\'html\'}', + $src, + 'Stored entities should be decoded before Smarty applies its final automatic escape.', + ); + $this->assertSame( + 5, + substr_count($src, '|unescape:\'html\''), + 'The title plus every name, folder, and icon sink must decode once before the final escape.', + ); + $this->assertStringNotContainsString( + 'nofilter', + $src, + 'Legacy or externally seeded raw mod metadata must never bypass Smarty escaping.', + ); + } } diff --git a/web/tests/integration/ModsDeleteDialogTest.php b/web/tests/integration/ModsDeleteDialogTest.php index 2ccbd38f6..5545ef27d 100644 --- a/web/tests/integration/ModsDeleteDialogTest.php +++ b/web/tests/integration/ModsDeleteDialogTest.php @@ -110,6 +110,10 @@ public function testDeleteButtonCarriesDataActionWiring(): void // Verify the supporting attributes are all present. $this->assertStringContainsString('data-name="' . $this->targetName . '"', $html); + $this->assertStringContainsString('src="images/games/default&alt.png"', $html); + $this->assertStringContainsString('>delete&target', $html); + $this->assertStringNotContainsString('&amp;', $html, + 'Entity-encoded mod metadata must be decoded before Smarty applies its final escape.'); $this->assertStringContainsString('data-fallback-href="index.php?p=admin&c=mods"', $html, 'The fallback URL lands the no-JS / no-dispatcher operator on the mods list ' . '(no legacy GET handler exists for `o=remove`).'); @@ -226,9 +230,47 @@ public function testCountBadgeCarriesTestid(): void $html = $this->renderModsPage(); $this->assertMatchesRegularExpression( - '/]*data-testid="mod-count"[^>]*>\s*\(?\d+\)?\s*<\/span>/', + '/]*data-testid="mod-count"[^>]*>\s*\(\d+\)\s*<\/span>/', $html, - 'The mod count number must be wrapped in (optionally parenthesised, matching the Admins list badge).' + 'The mod count must be parenthesised inside , matching the Admins list badge.' + ); + } + + /** + * `mid = 0` is the reserved Web pseudo-mod. It is deliberately absent + * from both list renderings, so the heading count must exclude it too. + */ + public function testCountBadgeMatchesDesktopRowsAndMobileCards(): void + { + $html = $this->renderModsPage(); + + $matched = preg_match( + '/]*data-testid="mod-count"[^>]*>\s*\((\d+)\)\s*<\/span>/', + $html, + $countMatch, + ); + $this->assertSame(1, $matched, 'The parenthesised mod count must render.'); + + $desktopRows = preg_match_all('/]*data-testid="mod-row"[^>]*>/', $html); + $mobileCards = preg_match_all('/]*data-testid="mods-list-card"[^>]*>/', $html); + + $this->assertNotFalse($desktopRows); + $this->assertNotFalse($mobileCards); + $this->assertSame((int) $countMatch[1], $desktopRows, + 'The heading count must equal the visible game-mod rows, excluding the reserved Web pseudo-mod.'); + $this->assertSame($desktopRows, $mobileCards, + 'Every desktop mod row must have one mobile-card mirror.'); + } + + public function testCountDecrementKeepsAsciiDigitsForSubsequentDeletes(): void + { + $html = $this->renderModsPage(); + + $this->assertStringContainsString("String(n - 1) + ')'", $html); + $this->assertStringNotContainsString( + '(n - 1).toLocaleString()', + $html, + 'Localized numerals cannot be parsed by the next ASCII-digit decrement.', ); } @@ -236,15 +278,14 @@ private function seedTargetMod(): void { $pdo = Fixture::rawPdo(); - // Use a name that's safe through htmlspecialchars and survives - // the data-name attribute round-trip without escaping noise so - // the data-name assertion stays readable. - $this->targetName = 'DeleteTargetMod'; + // Mirror api_mods_add's escaping-on-store shape. The rendered + // list should decode this once, then let Smarty escape it once. + $this->targetName = 'Delete & Target'; $pdo->prepare(sprintf( 'INSERT INTO `%s_mods` (name, icon, modfolder, steam_universe, enabled) VALUES (?, ?, ?, ?, ?)', DB_PREFIX, - ))->execute([$this->targetName, 'default.png', 'deletetargetmod', 0, 1]); + ))->execute([$this->targetName, 'default&alt.png', 'delete&target', 0, 1]); $this->targetMid = (int) $pdo->lastInsertId(); } diff --git a/web/themes/default/page_admin_edit_mod.tpl b/web/themes/default/page_admin_edit_mod.tpl index 83f08a0ad..5ad4a95c1 100644 --- a/web/themes/default/page_admin_edit_mod.tpl +++ b/web/themes/default/page_admin_edit_mod.tpl @@ -29,10 +29,10 @@ *}
- {* nofilter: name is htmlspecialchars'd on store in admin.edit.mod.php; - auto-escaping here would double-encode (#1108 / #1113). *} + {* Mod names are entity-encoded on store. Decode before Smarty's + automatic final escape so the heading stays both readable and safe. *}

- Edit mod · {$name nofilter} + Edit mod · {$name|unescape:'html'}

Update the configuration for this game mod. @@ -49,18 +49,19 @@

- {* nofilter: mod metadata is htmlspecialchars(strip_tags($_POST[…]))'d in admin.edit.mod.php before INSERT/UPDATE, so values pulled back out of `:prefix_mods` are already entity-encoded; auto-escaping the value attribute would double-encode (#1113 audit). The id="icon_hid" element is the channel the popup uploader writes into via window.opener.icon(). *} - + {* Mod metadata is entity-encoded on store. Decode that layer + before Smarty's automatic final escape so legacy raw values + cannot break out of text or attribute contexts. *} +
- {* nofilter: see the icon_hid annotation above — name is htmlspecialchars'd on store, double-encoding it in the value attribute would render literal &… to admins (#1108 / #1113 audit). *}
- {* nofilter: see the icon_hid annotation above — folder is htmlspecialchars'd on store. *}

Folder name on disk (e.g. cstrike for Counter-Strike: Source). @@ -125,8 +125,7 @@ {if $mod_icon} Current: - {* nofilter: see the icon_hid annotation above — icon filename is htmlspecialchars'd on store. *} - {$mod_icon nofilter} + {$mod_icon|unescape:'html'} {/if}

diff --git a/web/themes/default/page_admin_mods_list.tpl b/web/themes/default/page_admin_mods_list.tpl index 273dc1bbe..4ab4e296c 100644 --- a/web/themes/default/page_admin_mods_list.tpl +++ b/web/themes/default/page_admin_mods_list.tpl @@ -69,20 +69,23 @@ + {* Mod metadata is entity-encoded on store. Decode before + Smarty's automatic final escape at every text and + attribute sink so values stay readable without raw HTML. *} {foreach from=$mod_list item=mod}
- - {$mod.name} + {$mod.name|unescape:'html'}
- {$mod.modfolder} + {$mod.modfolder|unescape:'html'} {$mod.steam_universe} {if $mod.enabled} @@ -103,7 +106,7 @@ href="index.php?p=admin&c=mods&o=edit&id={$mod.mid|escape:'url'}" data-testid="editmod-link" data-tooltip="Edit" - aria-label="Edit mod {$mod.name|escape}"> + aria-label="Edit mod {$mod.name|unescape:'html'}"> {/if} @@ -127,11 +130,11 @@ type="button" data-action="mod-delete" data-mid="{$mod.mid}" - data-name="{$mod.name|escape}" + data-name="{$mod.name|unescape:'html'}" data-fallback-href="index.php?p=admin&c=mods" data-testid="deletemod-btn" data-tooltip="Delete" - aria-label="Delete mod {$mod.name|escape}"> + aria-label="Delete mod {$mod.name|unescape:'html'}"> {/if} @@ -151,16 +154,16 @@ {foreach from=$mod_list item=mod}
-
-
{$mod.name}
+
{$mod.name|unescape:'html'}
- {$mod.modfolder} + {$mod.modfolder|unescape:'html'} · SU {$mod.steam_universe}
@@ -179,7 +182,7 @@ href="index.php?p=admin&c=mods&o=edit&id={$mod.mid|escape:'url'}" data-testid="editmod-link-mobile" data-tooltip="Edit" - aria-label="Edit mod {$mod.name|escape}"> + aria-label="Edit mod {$mod.name|unescape:'html'}"> {/if} @@ -188,11 +191,11 @@ type="button" data-action="mod-delete" data-mid="{$mod.mid}" - data-name="{$mod.name|escape}" + data-name="{$mod.name|unescape:'html'}" data-fallback-href="index.php?p=admin&c=mods" data-testid="deletemod-btn-mobile" data-tooltip="Delete" - aria-label="Delete mod {$mod.name|escape}"> + aria-label="Delete mod {$mod.name|unescape:'html'}"> {/if} @@ -354,7 +357,7 @@ if (!el) return; var n = Number((el.textContent || '').replace(/[^0-9]/g, '')); if (!Number.isFinite(n) || n <= 0) return; - el.textContent = '(' + (n - 1).toLocaleString() + ')'; + el.textContent = '(' + String(n - 1) + ')'; } /** @returns {HTMLDialogElement|null} */ From bd8e3ac35bf631167988ce57346369957c2434ba Mon Sep 17 00:00:00 2001 From: rumblefrog Date: Sun, 6 Sep 2026 16:25:39 -0400 Subject: [PATCH 4/4] fix(admin-mods): handle final-row and narrow-screen states --- web/includes/View/AdminModsListView.php | 4 +- web/pages/admin.mods.php | 1 + .../specs/flows/mod-delete-confirm.spec.ts | 37 ++++++++++++++ .../integration/EditAdminTabsChromeTest.php | 5 ++ .../integration/ModsDeleteDialogTest.php | 28 ++++++++++- web/themes/default/css/theme.css | 2 + web/themes/default/page_admin_edit_mod.tpl | 2 +- web/themes/default/page_admin_mods_list.tpl | 50 +++++++++++++++---- 8 files changed, 117 insertions(+), 12 deletions(-) diff --git a/web/includes/View/AdminModsListView.php b/web/includes/View/AdminModsListView.php index ced82f9fb..29a113903 100644 --- a/web/includes/View/AdminModsListView.php +++ b/web/includes/View/AdminModsListView.php @@ -16,7 +16,8 @@ * Property names keep the historical `permission_*` shape (rather than * the newer `can_*` convention `Sbpp\View\Perms::for()` produces) because * the template references `{$permission_listmods}` / - * `{$permission_editmods}` / `{$permission_deletemods}`. See + * `{$permission_addmods}` / `{$permission_editmods}` / + * `{$permission_deletemods}`. See * `AdminServersListView` / `AdminBansAddView` for the canonical * `can_*` convention on Views that don't have to honour a historical * variable name. @@ -30,6 +31,7 @@ final class AdminModsListView extends View */ public function __construct( public readonly bool $permission_listmods, + public readonly bool $permission_addmods, public readonly bool $permission_editmods, public readonly bool $permission_deletemods, public readonly int $mod_count, diff --git a/web/pages/admin.mods.php b/web/pages/admin.mods.php index 5511f9e9d..7f12c7b8e 100644 --- a/web/pages/admin.mods.php +++ b/web/pages/admin.mods.php @@ -43,6 +43,7 @@ \Sbpp\View\Renderer::render($theme, new \Sbpp\View\AdminModsListView( permission_listmods: $canList, + permission_addmods: $canAdd, permission_editmods: $userbank->HasAccess(WebPermission::mask(WebPermission::Owner, WebPermission::EditMods)), permission_deletemods: $userbank->HasAccess(WebPermission::mask(WebPermission::Owner, WebPermission::DeleteMods)), mod_count: $mod_count, diff --git a/web/tests/e2e/specs/flows/mod-delete-confirm.spec.ts b/web/tests/e2e/specs/flows/mod-delete-confirm.spec.ts index 8292cb793..604214ff0 100644 --- a/web/tests/e2e/specs/flows/mod-delete-confirm.spec.ts +++ b/web/tests/e2e/specs/flows/mod-delete-confirm.spec.ts @@ -300,6 +300,33 @@ test.describe('flow: admin mod delete confirm modal (#1397 — RemoveMod zombie) .first(); await expect(row).toBeVisible(); + // Model the final visible mod without deleting fixture rows from the + // shared DB. This isolates the client-side 1 → 0 transition that must + // reveal the already-mounted empty state after a successful delete. + const targetMid = await row.getAttribute('data-id'); + expect(targetMid).not.toBeNull(); + await page.locator('[data-testid="mod-row"]').evaluateAll((nodes, keepMid) => { + nodes.forEach((node) => { + if (node.getAttribute('data-id') !== keepMid) node.remove(); + }); + }, targetMid); + await page.locator('[data-testid="mods-list-card"]').evaluateAll((nodes, keepMid) => { + nodes.forEach((node) => { + if (node.getAttribute('data-id') !== keepMid) node.remove(); + }); + }, targetMid); + + const targetCard = page + .locator('[data-testid="mods-list-card"]') + .filter({ hasText: 'e2e-no-reason-mod' }); + const countBadge = page.locator('[data-testid="mod-count"]'); + await countBadge.evaluate((node) => { + node.textContent = '(1)'; + }); + await expect(page.locator('[data-testid="mod-row"]')).toHaveCount(1); + await expect(targetCard).toHaveCount(1); + await expect(page.locator('[data-testid="mods-empty"]')).toBeHidden(); + await row.locator('[data-testid="deletemod-btn"]').click(); const dialog = page.locator('[data-testid="mod-delete-dialog"]'); await expect(dialog).toBeVisible(); @@ -314,7 +341,17 @@ test.describe('flow: admin mod delete confirm modal (#1397 — RemoveMod zombie) expect(env.ok, JSON.stringify(env)).toBe(true); await expect(row).toHaveCount(0); + await expect(targetCard).toHaveCount(0); + await expect(countBadge).toHaveText('(0)'); + await expect(page.locator('[data-testid="mods-table-wrap"]')).toHaveAttribute('hidden', ''); + await expect(page.locator('[data-testid="mods-list-cards"]')).toHaveAttribute('hidden', ''); + await expect(page.locator('[data-testid="mods-empty"]')).toBeVisible(); + await page.setViewportSize({ width: 390, height: 844 }); + await expect(page.locator('[data-testid="mods-list-cards"]')).toBeHidden(); + await expect(page.locator('[data-testid="mods-empty"]')).toBeVisible(); + + await page.setViewportSize({ width: 1280, height: 720 }); await page.goto(AUDIT_ROUTE); const auditDetail = page .locator('.audit-row__detail') diff --git a/web/tests/integration/EditAdminTabsChromeTest.php b/web/tests/integration/EditAdminTabsChromeTest.php index fb4b89661..e67ed7cc9 100644 --- a/web/tests/integration/EditAdminTabsChromeTest.php +++ b/web/tests/integration/EditAdminTabsChromeTest.php @@ -124,6 +124,11 @@ public function testEditModMetadataDoesNotRenderAsRawHtml(): void $src, 'Stored entities should be decoded before Smarty applies its final automatic escape.', ); + $this->assertStringContainsString( + 'overflow-wrap:anywhere', + $src, + 'A schema-width mod name must not overflow the edit heading on narrow viewports.', + ); $this->assertSame( 5, substr_count($src, '|unescape:\'html\''), diff --git a/web/tests/integration/ModsDeleteDialogTest.php b/web/tests/integration/ModsDeleteDialogTest.php index 5545ef27d..5a188014b 100644 --- a/web/tests/integration/ModsDeleteDialogTest.php +++ b/web/tests/integration/ModsDeleteDialogTest.php @@ -112,6 +112,8 @@ public function testDeleteButtonCarriesDataActionWiring(): void $this->assertStringContainsString('data-name="' . $this->targetName . '"', $html); $this->assertStringContainsString('src="images/games/default&alt.png"', $html); $this->assertStringContainsString('>delete&target', $html); + $this->assertStringContainsString('title="delete&target"', $html, + 'The truncated mobile folder must expose its full value.'); $this->assertStringNotContainsString('&amp;', $html, 'Entity-encoded mod metadata must be decoded before Smarty applies its final escape.'); $this->assertStringContainsString('data-fallback-href="index.php?p=admin&c=mods"', $html, @@ -266,7 +268,8 @@ public function testCountDecrementKeepsAsciiDigitsForSubsequentDeletes(): void { $html = $this->renderModsPage(); - $this->assertStringContainsString("String(n - 1) + ')'", $html); + $this->assertStringContainsString("String(next) + ')'", $html); + $this->assertStringContainsString('if (next === 0) showEmptyState();', $html); $this->assertStringNotContainsString( '(n - 1).toLocaleString()', $html, @@ -274,6 +277,29 @@ public function testCountDecrementKeepsAsciiDigitsForSubsequentDeletes(): void ); } + public function testZeroGameModsRendersActionableEmptyState(): void + { + Fixture::rawPdo()->exec(sprintf( + 'DELETE FROM `%s_mods` WHERE mid > 0', + DB_PREFIX, + )); + + $html = $this->renderModsPage(); + + $this->assertStringContainsString('data-testid="mod-count">(0)', $html); + $this->assertSame(0, preg_match_all('/]*data-testid="mod-row"[^>]*>/', $html)); + $this->assertStringContainsString('data-testid="mods-empty-add"', $html); + + $matched = preg_match( + '/
]*data-testid="mods-empty"[^>]*>/s', + $html, + $emptyTag, + ); + $this->assertSame(1, $matched, 'The mods empty state must render.'); + $this->assertStringNotContainsString(' hidden', $emptyTag[0], + 'The server-rendered empty state must be visible when no game mods remain.'); + } + private function seedTargetMod(): void { $pdo = Fixture::rawPdo(); diff --git a/web/themes/default/css/theme.css b/web/themes/default/css/theme.css index 5fd064729..2dc9f6797 100644 --- a/web/themes/default/css/theme.css +++ b/web/themes/default/css/theme.css @@ -2131,6 +2131,7 @@ details.queue-row > summary > .row-actions { .admins-list-card__body { padding: 0.75rem 1rem 0.25rem; } .mods-list-cards { display: none; } +.mods-list-cards[hidden] { display: none; } .mods-list-card { border-bottom: 1px solid var(--border); } .mods-list-card:last-child { border-bottom: none; } .mods-list-card__body { padding: 0.75rem 1rem 0.25rem; } @@ -2245,6 +2246,7 @@ details.queue-row > summary > .row-actions { padding: 2.5rem 1.5rem; color: var(--text-muted); } +.empty-state[hidden] { display: none; } .empty-state__icon { width: 2.5rem; height: 2.5rem; diff --git a/web/themes/default/page_admin_edit_mod.tpl b/web/themes/default/page_admin_edit_mod.tpl index 5ad4a95c1..0b98dcd40 100644 --- a/web/themes/default/page_admin_edit_mod.tpl +++ b/web/themes/default/page_admin_edit_mod.tpl @@ -31,7 +31,7 @@
{* Mod names are entity-encoded on store. Decode before Smarty's automatic final escape so the heading stays both readable and safe. *} -

+

Edit mod · {$name|unescape:'html'}

diff --git a/web/themes/default/page_admin_mods_list.tpl b/web/themes/default/page_admin_mods_list.tpl index 4ab4e296c..d877fa19d 100644 --- a/web/themes/default/page_admin_mods_list.tpl +++ b/web/themes/default/page_admin_mods_list.tpl @@ -7,6 +7,7 @@ Variable contract (kept in sync by SmartyTemplateRule): - $permission_listmods — gate the whole tab body. + - $permission_addmods — gate the empty-state "Add mod" CTA. - $permission_editmods — gate the per-row "Edit" link. - $permission_deletemods — gate the per-row "Delete" button. - $mod_count — total mods configured. @@ -55,7 +56,7 @@

{if $mod_count > 0} -
+
@@ -162,9 +163,11 @@ onerror="this.style.visibility='hidden'">
{$mod.name|unescape:'html'}
-
- {$mod.modfolder|unescape:'html'} - · SU {$mod.steam_universe} +
+ {$mod.modfolder|unescape:'html'} + SU {$mod.steam_universe}
{if $mod.enabled} @@ -204,11 +207,28 @@
{/foreach}
- {else} -
-

No mods configured yet.

-
{/if} + +
0}hidden{/if}> + +

No mods configured yet

+

Add a game mod before assigning it to bans or servers.

+ {if $permission_addmods} +
+ + + Add mod + +
+ {/if} +
{if $permission_deletemods} @@ -345,6 +365,16 @@ ); } + /** @returns {void} */ + function showEmptyState() { + var tableWrap = document.querySelector('[data-testid="mods-table-wrap"]'); + var cards = document.querySelector('[data-testid="mods-list-cards"]'); + var empty = document.querySelector('[data-testid="mods-empty"]'); + if (tableWrap) tableWrap.setAttribute('hidden', ''); + if (cards) cards.setAttribute('hidden', ''); + if (empty) empty.removeAttribute('hidden'); + } + /** * Drop one from the count badge. Reads the digits out of the badge's * textContent so a third-party theme that wraps the count @@ -357,7 +387,9 @@ if (!el) return; var n = Number((el.textContent || '').replace(/[^0-9]/g, '')); if (!Number.isFinite(n) || n <= 0) return; - el.textContent = '(' + String(n - 1) + ')'; + var next = n - 1; + el.textContent = '(' + String(next) + ')'; + if (next === 0) showEmptyState(); } /** @returns {HTMLDialogElement|null} */