Skip to content

Commit 5356bb0

Browse files
rumblefrogrumblefrog
authored
fix(dashboard): stop truncating server names server-side; let CSS handle the cut (sbpp#1487) (sbpp#1489)
The dashboard Servers widget forwarded `data-trunchostname="40"`, which made the JSON handler hard-cut the live hostname to 40 chars + "..." before the browser ever saw it. The hostname cell also carries the CSS `.truncate` class, so the name was clipped twice and the server-side cap won even when the column was wide enough to show more. The dashboard now sends `0`, a new "no server-side truncation" sentinel, so the full hostname reaches the client and `.truncate` does the responsive visual cut: the row shows as much of the name as fits in its column. - trunc(): treat a non-positive $len as "return verbatim" - server-tile-hydrate.js: resolveTrunc() forwards 0 (>= 0 guard, not the falsy-coercing `|| 70`) and Math.trunc()s fractional inputs - page_dashboard.tpl: data-trunchostname 40 -> 0 - tests: ServersTest (0 returns full hostname, positive still truncates), SystemFunctionsTest (trunc sentinel), DashboardServersWidgetHydrationTest - docs: AGENTS.md + correct the now-stale "matches the dashboard's 40" cross-references on the Add Admin grid / Server Groups (both keep 40)
1 parent 6faef18 commit 5356bb0

10 files changed

Lines changed: 288 additions & 60 deletions

‎AGENTS.md‎

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

‎web/includes/system-functions.php‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,9 +163,19 @@ function NextAid(): int
163163
* the byte length, not the multibyte codepoint length — kept that way
164164
* for byte-budget callers that pass an actual byte limit (e.g. SQL
165165
* column max size).
166+
*
167+
* A non-positive `$len` is the "no truncation" sentinel: the string is
168+
* returned verbatim. Callers that want the full value back pass `0`
169+
* instead of a large magic number. The dashboard's Servers widget uses
170+
* this so the live hostname is sent untrimmed and the surface's CSS
171+
* `.truncate` does the responsive visual cut, showing as much of the
172+
* name as fits in its column instead of a fixed server-side cap (#1487).
166173
*/
167174
function trunc(string $text, int $len): string
168175
{
176+
if ($len <= 0) {
177+
return $text;
178+
}
169179
return strlen($text) > $len ? substr($text, 0, $len) . '...' : $text;
170180
}
171181

‎web/scripts/server-tile-hydrate.js‎

Lines changed: 38 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,10 @@
9898
`?p=servers&s=<n>` deep link
9999
behaviour from page.servers.php)
100100
data-trunchostname="<n>" hostname truncation forwarded
101-
to the JSON action (default 70)
101+
to the JSON action (default 70;
102+
`0` = no server-side truncation,
103+
CSS `.truncate` handles the cut
104+
client-side, #1487)
102105
103106
----------------------------------------------------------------
104107
Public API (window.SBPP)
@@ -128,7 +131,7 @@
128131
* @typedef {Object} HydrateOptions
129132
* @property {ParentNode} [container] - tile-bearing wrapper (default: every `[data-server-hydrate]` in the doc).
130133
* @property {number} [openedIndex] - auto-expand the tile whose `data-index` matches this value (default: -1, i.e. don't auto-expand).
131-
* @property {number} [trunchostname] - hostname truncation forwarded to the JSON action (default 70).
134+
* @property {number} [trunchostname] - hostname truncation forwarded to the JSON action (default 70; `0` = no server-side truncation, CSS handles the cut client-side, #1487).
132135
*/
133136

134137
/**
@@ -429,6 +432,35 @@
429432
}
430433
}
431434

435+
/**
436+
* Resolve the hostname truncation hint forwarded to
437+
* `Actions.ServersHostPlayers`. The value is a max character count;
438+
* `0` is the explicit "no server-side truncation" sentinel — the
439+
* server returns the full hostname and the surface's CSS `.truncate`
440+
* does the responsive visual cut, so the row shows as much of the
441+
* name as fits in its column (#1487). A missing / non-numeric /
442+
* negative value defaults to 70 (the public servers list's
443+
* full-width budget).
444+
*
445+
* Note the deliberate `>= 0` (not `> 0`): a bare `(raw || 70)` /
446+
* `(raw > 0 ? raw : 70)` would coerce the `0` sentinel back to 70
447+
* and silently re-cap the hostname server-side — the exact bug this
448+
* helper exists to prevent.
449+
*
450+
* @param {unknown} raw - number from a programmatic call, the string
451+
* value of a `data-trunchostname` attribute, or null/undefined.
452+
* @returns {number} a non-negative integer truncation hint (0 = no
453+
* truncation). `Math.trunc` keeps a stray fractional input (e.g.
454+
* a hand-authored `data-trunchostname="1.5"`) from reaching the
455+
* PHP handler, which would `(int)`-floor it anyway.
456+
*/
457+
function resolveTrunc(raw) {
458+
var n = typeof raw === 'number'
459+
? raw
460+
: (typeof raw === 'string' && raw !== '' ? Number(raw) : NaN);
461+
return (typeof n === 'number' && isFinite(n) && n >= 0) ? Math.trunc(n) : 70;
462+
}
463+
432464
/**
433465
* @param {HTMLElement} tile
434466
* @param {HydrateOptions} opts
@@ -467,7 +499,7 @@
467499
var refresh = tile.querySelector('[data-testid="server-refresh"]');
468500
if (refresh instanceof HTMLButtonElement) refresh.disabled = true;
469501

470-
var trunc = (opts && opts.trunchostname) || 70;
502+
var trunc = resolveTrunc(opts ? opts.trunchostname : undefined);
471503
var openedIndex = (opts && typeof opts.openedIndex === 'number') ? opts.openedIndex : -1;
472504
sb.api.call(Actions.ServersHostPlayers, { sid: sid, trunchostname: trunc }).then(function (r) {
473505
anyTile.__sbppLoading = false;
@@ -511,9 +543,9 @@
511543
var openedIndex = (typeof opts.openedIndex === 'number')
512544
? opts.openedIndex
513545
: (containerEl ? Number(containerEl.getAttribute('data-opened-index') || -1) : -1);
514-
var trunc = (typeof opts.trunchostname === 'number' && opts.trunchostname > 0)
515-
? opts.trunchostname
516-
: (containerEl ? Number(containerEl.getAttribute('data-trunchostname') || 70) : 70);
546+
var trunc = (typeof opts.trunchostname === 'number')
547+
? resolveTrunc(opts.trunchostname)
548+
: resolveTrunc(containerEl ? containerEl.getAttribute('data-trunchostname') : null);
517549

518550
Array.prototype.forEach.call(tiles, function (/** @type {HTMLElement} */ tile) {
519551
loadTile(tile, { openedIndex: openedIndex, trunchostname: trunc }, container);

‎web/tests/api/ServersTest.php‎

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -435,6 +435,114 @@ public function testHostPlayersCoalescesRapidRepeatCallsViaCache(): void
435435
);
436436
}
437437

438+
/**
439+
* #1487 — a `trunchostname` of `0` is the "no server-side
440+
* truncation" sentinel: the handler returns the full hostname so
441+
* the dashboard widget's CSS `.truncate` can do the responsive
442+
* visual cut. Pre-fix the dashboard forwarded a fixed `40`-char
443+
* cap that chopped the name server-side before the browser ever
444+
* saw it; now it forwards `0` and the row shows as much of the
445+
* name as fits in its column.
446+
*/
447+
public function testHostPlayersReturnsFullHostnameWhenTrunchostnameZero(): void
448+
{
449+
$longHost = 'SourceBans++ Community | 24/7 FastDL | Dust2 Only | EU West';
450+
$this->assertGreaterThan(48, strlen($longHost), 'guard: the fixture hostname must exceed the legacy 48-char default to make the assertion meaningful');
451+
452+
SourceQueryCache::setProbeOverrideForTesting(static function () use ($longHost): array {
453+
return [
454+
'info' => [
455+
'HostName' => $longHost,
456+
'Players' => 3,
457+
'MaxPlayers' => 24,
458+
'Map' => 'de_dust2',
459+
'Os' => 'l',
460+
'Secure' => true,
461+
],
462+
'players' => [],
463+
];
464+
});
465+
466+
$sid = $this->seedServer(130);
467+
$env = $this->api('servers.host_players', ['sid' => $sid, 'trunchostname' => 0]);
468+
469+
$this->assertTrue($env['ok'], 'envelope: ' . json_encode($env));
470+
$this->assertSame(
471+
$longHost,
472+
$env['data']['hostname'],
473+
'trunchostname=0 must return the hostname verbatim (no "..."); CSS handles the visual cut client-side (#1487)',
474+
);
475+
$this->assertStringNotContainsString('...', $env['data']['hostname']);
476+
}
477+
478+
/**
479+
* #1487 — the inverse contract: a positive `trunchostname` still
480+
* truncates server-side. The dashboard opts out via `0`, but every
481+
* other surface (public list `70`, Add Admin grid / Server Groups
482+
* `40`) relies on the cap staying live, so the positive branch must
483+
* keep working.
484+
*/
485+
public function testHostPlayersTruncatesHostnameWhenTrunchostnamePositive(): void
486+
{
487+
$longHost = 'SourceBans++ Community | 24/7 FastDL | Dust2 Only | EU West';
488+
489+
SourceQueryCache::setProbeOverrideForTesting(static function () use ($longHost): array {
490+
return [
491+
'info' => [
492+
'HostName' => $longHost,
493+
'Players' => 3,
494+
'MaxPlayers' => 24,
495+
'Map' => 'de_dust2',
496+
'Os' => 'l',
497+
'Secure' => true,
498+
],
499+
'players' => [],
500+
];
501+
});
502+
503+
$sid = $this->seedServer(131);
504+
$env = $this->api('servers.host_players', ['sid' => $sid, 'trunchostname' => 10]);
505+
506+
$this->assertTrue($env['ok'], 'envelope: ' . json_encode($env));
507+
$this->assertSame(
508+
substr($longHost, 0, 10) . '...',
509+
$env['data']['hostname'],
510+
'a positive trunchostname must still cap the hostname server-side (#1487 keeps the positive branch intact for the non-dashboard surfaces)',
511+
);
512+
}
513+
514+
/**
515+
* #1487 — the property handler shares the same `trunc()` call, so
516+
* the `0` sentinel must disable truncation there too. Pinned so a
517+
* future refactor that splits the two handlers' truncation paths
518+
* can't silently re-cap one of them.
519+
*/
520+
public function testHostPropertyReturnsFullHostnameWhenTrunchostnameZero(): void
521+
{
522+
$longHost = 'SourceBans++ Community | 24/7 FastDL | Dust2 Only | EU West';
523+
524+
SourceQueryCache::setProbeOverrideForTesting(static function () use ($longHost): array {
525+
return [
526+
'info' => [
527+
'HostName' => $longHost,
528+
'Players' => 0,
529+
'MaxPlayers' => 24,
530+
'Map' => 'de_dust2',
531+
'Os' => 'l',
532+
'Secure' => true,
533+
],
534+
'players' => [],
535+
];
536+
});
537+
538+
$sid = $this->seedServer(132);
539+
$env = $this->api('servers.host_property', ['sid' => $sid, 'trunchostname' => 0]);
540+
541+
$this->assertTrue($env['ok'], 'envelope: ' . json_encode($env));
542+
$this->assertSame($longHost, $env['data']['hostname']);
543+
$this->assertStringNotContainsString('...', $env['data']['hostname']);
544+
}
545+
438546
/**
439547
* #1311 regression — a `host_players` call against an unreachable
440548
* server must NOT keep hammering the socket. Negative caching

‎web/tests/integration/AddAdminServerHostHydrationTest.php‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -135,12 +135,15 @@ public function testTemplateIncludesHydrationHelperScript(): void
135135
* at first paint; without this attribute the Add Admin rows are
136136
* skipped entirely.
137137
*
138-
* `data-trunchostname="40"` matches the dashboard widget's
139-
* cramped-column hint — the per-row card is ~18rem wide
138+
* `data-trunchostname="40"` caps the hostname server-side — this
139+
* grid's per-row card is a FIXED ~18rem wide
140140
* (`grid-template-columns: repeat(auto-fill, minmax(18rem, 1fr))`)
141-
* and a long hostname would trip `truncate`'s ellipsis
142-
* prematurely. The presence of the attribute is the contract;
143-
* the helper falls back to 70 otherwise.
141+
* so a small fixed cap keeps a long hostname from tripping
142+
* `truncate`'s ellipsis. (The dashboard widget dropped its cap to
143+
* `0` in #1487 — its column is fluid, so CSS sizes the cut to the
144+
* rendered width; this grid's column is fixed, so the cap stays.)
145+
* The presence of the attribute is the contract; the helper falls
146+
* back to 70 otherwise.
144147
*/
145148
public function testGridOptsIntoAutoHydration(): void
146149
{
@@ -155,9 +158,9 @@ public function testGridOptsIntoAutoHydration(): void
155158
'The Add Admin per-server access grid wrapper must carry '
156159
. '`data-server-hydrate="auto" data-trunchostname="40"` on the SAME `<div>` opener '
157160
. 'so the shared hydration helper auto-runs on first paint and forwards the '
158-
. 'cramped-column truncation hint to the JSON action (#1405). The 40-char hint '
159-
. 'matches the dashboard widget\'s convention — same column constraint, same '
160-
. 'truncation budget.',
161+
. 'cramped-column truncation hint to the JSON action (#1405). The 40-char cap '
162+
. 'suits this grid\'s FIXED ~18rem columns; the dashboard widget dropped to `0` '
163+
. '(client-side CSS truncation) in #1487 because its column is fluid.',
161164
);
162165
}
163166

‎web/tests/integration/DashboardServersWidgetHydrationTest.php‎

Lines changed: 30 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -46,10 +46,13 @@
4646
* subtitle came back". Each of those is a structural change to the
4747
* template's source text — `file_get_contents` + assert pins them
4848
* directly without booting Smarty, seeding `:prefix_servers` rows,
49-
* or instantiating the View DTO. The E2E suite covers the runtime
50-
* observable (the hostname actually paints in after the JSON action
51-
* lands); this PHPUnit guard is the contract gate, sub-millisecond
52-
* and deterministic.
49+
* or instantiating the View DTO. This PHPUnit guard is the contract
50+
* gate for the forwarded attribute value (sub-millisecond,
51+
* deterministic). The API-side half of the #1487 contract —
52+
* `trunchostname=0` returns the hostname verbatim — is pinned by
53+
* `web/tests/api/ServersTest.php`; the dashboard widget has no
54+
* dedicated runtime (Playwright) spec for the hostname paint
55+
* (`smoke/dashboard.spec.ts` only smoke-checks that the page mounts).
5356
*
5457
* Pattern mirrors `ServerMapImageRenderTest` — same setUp, same
5558
* source-shape assertions.
@@ -94,28 +97,36 @@ public function testTemplateIncludesHydrationHelperScript(): void
9497
* document at first paint; without this attribute the dashboard
9598
* rows are skipped entirely.
9699
*
97-
* `data-trunchostname="40"` forwards to `api_servers_host_players`
98-
* as the SourceQuery truncation hint — the dashboard column is
99-
* cramped (shared with the Latest Bans card under the 2-up grid)
100-
* and the public list's `=70` would overflow the `truncate`
101-
* ellipsis. The number itself is a UX call but the presence of
102-
* the attribute is the contract: the helper falls back to 70
103-
* otherwise.
100+
* `data-trunchostname="0"` is the "no server-side truncation"
101+
* sentinel (#1487). The hostname cell carries the `truncate` CSS
102+
* class, so the browser cuts the name with an ellipsis at exactly
103+
* the column's rendered width — as much as fits. Pre-#1487 this
104+
* forwarded `40` as a fixed server-side cap that fought the CSS:
105+
* a hostname got chopped to 40 chars + "..." server-side even when
106+
* the column was wide enough to show more, then `truncate` clipped
107+
* it again. The `0` lets the client decide, so the row adapts to
108+
* the actual rendered width instead of a magic number. The helper's
109+
* `resolveTrunc` forwards `0` verbatim (its `>= 0` guard
110+
* deliberately does NOT coerce the sentinel back to the 70
111+
* default), and the server's `trunc()` treats `0` as "return the
112+
* full string".
104113
*/
105114
public function testServerListWrapperOptsIntoAutoHydration(): void
106115
{
107116
// Match against a `<div …>` opener that carries BOTH attrs.
108-
// `[\s\S]*?` is the non-greedy "any char including newline"
109-
// span; `\bdata-…\b` ensures we don't trip on a prefix match.
117+
// `[^>]*` stays within the single opening tag so we don't trip
118+
// on a later element; `\bdata-…\b` ensures we don't prefix-match.
110119
$this->assertMatchesRegularExpression(
111-
'/<div\b[^>]*\bdata-server-hydrate="auto"[^>]*\bdata-trunchostname="40"/',
120+
'/<div\b[^>]*\bdata-server-hydrate="auto"[^>]*\bdata-trunchostname="0"/',
112121
$this->template,
113122
'The dashboard Servers widget\'s row-list wrapper must carry '
114-
. '`data-server-hydrate="auto" data-trunchostname="40"` so the shared hydration '
115-
. 'helper auto-runs on first paint and forwards the cramped-column truncation hint '
116-
. 'to the JSON action (#1375). Pre-fix the widget had neither attribute and the '
117-
. 'rows stayed at the IP:port placeholder forever — same regression class as '
118-
. 'the admin Server Management list before #1313.',
123+
. '`data-server-hydrate="auto" data-trunchostname="0"` so the shared hydration '
124+
. 'helper auto-runs on first paint and forwards the "no server-side truncation" '
125+
. 'sentinel to the JSON action (#1487). The hostname cell\'s `truncate` CSS does '
126+
. 'the responsive cut client-side so the row shows as much of the name as fits in '
127+
. 'its column. Pre-#1487 this forwarded `40`, a fixed server-side cap that chopped '
128+
. 'the name before the browser ever saw it. Pre-#1375 the widget had neither '
129+
. 'attribute and the rows stayed at the IP:port placeholder forever.',
119130
);
120131
}
121132

‎web/tests/unit/SystemFunctionsTest.php‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,11 @@
4949
* `sys_get_temp_dir()` so it's hermetic — no fixture, no DB, no
5050
* docker-mounted `web/demos/` (which is shared across stacks and
5151
* could be polluted by other tests).
52+
*
53+
* The suite also pins `\trunc()`'s #1487 contract (the non-positive
54+
* "no truncation" sentinel + the unchanged positive branch). Those
55+
* cases are pure-string and need no temp tree; they live here because
56+
* `trunc()` ships in the same `system-functions.php` under test.
5257
*/
5358
final class SystemFunctionsTest extends TestCase
5459
{
@@ -220,6 +225,56 @@ public function testMixedTreeFiles(): void
220225
$this->assertSame(sizeFormat($expected), getDirSize($this->root));
221226
}
222227

228+
/**
229+
* #1487 — `trunc($text, 0)` is the "no truncation" sentinel: the
230+
* string comes back verbatim. The pre-#1487 shape ran the
231+
* unconditional `strlen($text) > $len ? substr($text, 0, $len) . '...'`
232+
* path, so `$len === 0` produced a bare `'...'` (because
233+
* `substr($text, 0, 0)` is `''`) — actively destructive, not a
234+
* no-op. The dashboard Servers widget forwards `0` through
235+
* `api_servers_host_players` so the live hostname reaches the
236+
* browser untrimmed and the cell's CSS `.truncate` does the
237+
* responsive visual cut.
238+
*/
239+
public function testTruncReturnsVerbatimForZeroLength(): void
240+
{
241+
$host = 'SourceBans++ Community | 24/7 FastDL | Dust2 Only';
242+
$this->assertSame(
243+
$host,
244+
trunc($host, 0),
245+
'trunc($text, 0) must return $text verbatim (the no-truncation sentinel), '
246+
. 'never the pre-#1487 destructive bare "..." that substr($text, 0, 0) . "..." produced.'
247+
);
248+
$this->assertStringNotContainsString('...', trunc($host, 0));
249+
}
250+
251+
/**
252+
* #1487 — negative lengths fall into the same `$len <= 0` sentinel
253+
* branch. A negative arrival is almost certainly a caller bug, but
254+
* returning the string verbatim is the safe, non-destructive choice
255+
* (it never builds a `substr(..., 0, <negative>)` window, which on
256+
* PHP counts from the end of the string and would silently corrupt
257+
* the output).
258+
*/
259+
public function testTruncReturnsVerbatimForNegativeLength(): void
260+
{
261+
$this->assertSame('abcdef', trunc('abcdef', -5));
262+
}
263+
264+
/**
265+
* #1487 — the positive branch is unchanged: a string longer than
266+
* `$len` bytes is cut to `$len` + `'...'`; a string within budget
267+
* (or exactly at it) comes back untouched, no gratuitous ellipsis.
268+
* Every non-dashboard surface (public list 70, Add Admin grid /
269+
* Server Groups 40) relies on this branch staying live.
270+
*/
271+
public function testTruncStillTruncatesPositiveLength(): void
272+
{
273+
$this->assertSame('abcde...', trunc('abcdefghij', 5), 'over budget → cut + ellipsis');
274+
$this->assertSame('abc', trunc('abc', 5), 'under budget → verbatim, no ellipsis');
275+
$this->assertSame('abcde', trunc('abcde', 5), 'exactly at budget → verbatim, no ellipsis');
276+
}
277+
223278
/**
224279
* Convenience: write `$contents` to `$rel` (path relative to
225280
* `$this->root`), creating any missing parent directories

0 commit comments

Comments
 (0)