Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 15 additions & 15 deletions AGENTS.md

Large diffs are not rendered by default.

16 changes: 1 addition & 15 deletions web/pages/page.banlist.php
Original file line number Diff line number Diff line change
Expand Up @@ -620,21 +620,7 @@ function setPostKey()
$where = "WHERE BA.bid = ?";
$advcrit = [$value];
break;
case "steamid":
// #1130: match both STEAM_0:Y:Z and STEAM_1:Y:Z stored variants;
// see SteamID::toSearchPattern() for rationale. The pre-switch
// normalisation block above has already canonicalised $value to
// STEAM_0 form, but the Y:Z tail is invariant so the pattern is
// the same either way.
$authidPattern = SteamID::toSearchPattern($value);
if ($authidPattern !== null) {
$where = "WHERE BA.authid REGEXP ?";
$advcrit = [$authidPattern];
} else {
$where = "WHERE BA.authid = ?";
$advcrit = [$value];
}
break;
case "steamid": // legacy exact-match URL; folded to always-partial
case "steam":
$where = "WHERE BA.authid LIKE ?";
$advcrit = ["%$value%"];
Expand Down
13 changes: 1 addition & 12 deletions web/pages/page.commslist.php
Original file line number Diff line number Diff line change
Expand Up @@ -497,18 +497,7 @@ function setPostKey()
$where = "WHERE CO.bid = ?";
$advcrit = [$value];
break;
case "steamid":
// #1130: match both STEAM_0:Y:Z and STEAM_1:Y:Z stored variants;
// see SteamID::toSearchPattern() for rationale.
$authidPattern = SteamID::toSearchPattern($value);
if ($authidPattern !== null) {
$where = "WHERE CO.authid REGEXP ?";
$advcrit = [$authidPattern];
} else {
$where = "WHERE CO.authid = ?";
$advcrit = [$value];
}
break;
case "steamid": // legacy exact-match URL; folded to always-partial
case "steam":
$where = "WHERE CO.authid LIKE ?";
$advcrit = ["%$value%"];
Expand Down
156 changes: 119 additions & 37 deletions web/scripts/comment-actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@
- `data-ctype="<B|C|S|P>"` — required
- `data-page="<int>"` — optional, defaults to -1

Confirm chrome is a single shared `<dialog>` (injected once)
matching the banlist / commslist delete modals — not
`window.confirm()`.

This file lives at panel scope so any future page that needs
comment-delete just adds the `data-action="comment-delete"`
attribute + the three data hooks and includes this script.
Expand Down Expand Up @@ -65,58 +69,103 @@
}
}

document.addEventListener('click', function (e) {
var t = /** @type {Element|null} */ (e.target);
if (!t) return;
var trigger = /** @type {HTMLElement|null} */ (t.closest && t.closest('[data-action="comment-delete"]'));
if (!trigger) return;
e.preventDefault();
/** @type {{cid: number, ctype: string, page: number, trigger: HTMLElement}|null} */
var pending = null;

var cid = parseInt(trigger.getAttribute('data-cid') || '0', 10);
var ctype = trigger.getAttribute('data-ctype') || '';
var page = parseInt(trigger.getAttribute('data-page') || '-1', 10);
/** @returns {HTMLDialogElement} */
function ensureDialog() {
var existing = /** @type {HTMLDialogElement|null} */ (document.getElementById('comment-delete-dialog'));
if (existing) return existing;

if (!cid || !ctype) {
toast('error', 'Delete failed', 'Missing comment context.');
return;
}
var d = document.createElement('dialog');
d.id = 'comment-delete-dialog';
d.className = 'palette';
d.setAttribute('aria-labelledby', 'comment-delete-dialog-title');
d.setAttribute('data-testid', 'comment-delete-dialog');
d.setAttribute('hidden', '');
d.setAttribute('style', 'max-width:32rem;width:90vw;padding:1.25rem;border-radius:0.75rem;border:1px solid var(--border)');
d.innerHTML =
'<form method="dialog" data-testid="comment-delete-form">'
+ '<h2 id="comment-delete-dialog-title" style="font-size:var(--fs-lg);font-weight:600;margin:0 0 0.25rem">Delete comment</h2>'
+ '<p class="text-sm text-muted m-0" style="margin-bottom:0.75rem">'
+ 'Delete this comment? This cannot be undone.'
+ '</p>'
+ '<div class="flex gap-2 mt-4" style="justify-content:flex-end">'
+ '<button type="button" class="btn btn--secondary" data-testid="comment-delete-cancel" value="cancel">Cancel</button>'
+ '<button type="submit" class="btn btn--danger" data-testid="comment-delete-submit" value="confirm">'
+ '<i data-lucide="trash-2" style="width:13px;height:13px"></i> Delete comment'
+ '</button>'
+ '</div>'
+ '</form>';
document.body.appendChild(d);
var lucide = /** @type {any} */ (window).lucide;
if (lucide && typeof lucide.createIcons === 'function') lucide.createIcons();
return d;
}

// Confirm prompt — comment deletion is irreversible and the
// legacy helper used the native confirm() too. We keep it as
// a native `confirm()` rather than a `<dialog>` because the
// trash-can appears in dense threads (potentially 10+ per
// page) and the dialog scaffolding noise per row would
// dwarf the affordance.
if (!window.confirm('Are you sure you want to delete this comment?')) {
return;
/**
* @param {{cid: number, ctype: string, page: number, trigger: HTMLElement}} ctx
*/
function openDeleteDialog(ctx) {
pending = ctx;
var d = ensureDialog();
d.removeAttribute('hidden');
try { d.showModal(); }
catch (_e) { d.setAttribute('open', ''); }
var submitBtn = /** @type {HTMLButtonElement|null} */ (d.querySelector('[data-testid="comment-delete-submit"]'));
if (submitBtn) {
try { submitBtn.focus(); } catch (_e) { /* focus may throw */ }
}
}

function closeDeleteDialog() {
var d = /** @type {HTMLDialogElement|null} */ (document.getElementById('comment-delete-dialog'));
if (!d) return;
try { d.close(); } catch (_e) { /* not opened modally */ }
d.setAttribute('hidden', '');
pending = null;
}

/**
* @param {{cid: number, ctype: string, page: number, trigger: HTMLElement}} ctx
*/
function runDelete(ctx) {
var a = api(), A = actions();
if (!a || !A) {
toast('error', 'Delete failed', 'The API client is unavailable. Reload the page and try again.');
return;
}

setBusy(trigger, true);
var submitBtn = /** @type {HTMLButtonElement|null} */ (
document.querySelector('#comment-delete-dialog [data-testid="comment-delete-submit"]')
);
setBusy(submitBtn, true);
setBusy(ctx.trigger, true);
a.call(A.BansRemoveComment, {
cid: cid,
ctype: ctype,
page: page,
cid: ctx.cid,
ctype: ctx.ctype,
page: ctx.page,
}).then(function (r) {
// sb.api.call follows r.redirect natively when the envelope
// sets it; on success api_bans_remove_comment surfaces a
// `message.redir` field that drives the navigation back to
// the same paginated view. Mirror SbppGroupsAdd's shape.
if (!r) { setBusy(trigger, false); return; }
if (!r) {
setBusy(submitBtn, false);
setBusy(ctx.trigger, false);
return;
}
if (r.redirect) return;
if (r.ok === false) {
setBusy(trigger, false);
setBusy(submitBtn, false);
setBusy(ctx.trigger, false);
var em = (r.error && r.error.message) || 'Failed to delete comment.';
toast('error', 'Delete failed', em);
return;
}
var data = r.data || {};
var msg = data.message || {};
closeDeleteDialog();
toast('success', msg.title || 'Comment Deleted', msg.body || 'The comment was deleted.');
// Honour the handler's redir envelope (sb.api.call only
// auto-redirects on r.redirect, NOT on data.message.redir).
Expand All @@ -127,17 +176,50 @@
else window.location.reload();
}, 1200);
}).catch(function (err) {
// #1402 adversarial review MEDIUM 4: defensive .catch() arm
// so a throw inside the success callback (or a sb.api.call
// internal failure) doesn't leave the trash-can stuck in
// its busy state. The trash-can appears in dense threads
// (potentially 10+ per page) and a stuck row reads as a
// broken affordance — the operator clicks again, gets the
// confirm prompt, and the second click stays no-op'd
// because the bubble-phase delegate sees `aria-busy` and
// the dispatch silently re-fires.
setBusy(trigger, false);
setBusy(submitBtn, false);
setBusy(ctx.trigger, false);
toast('error', 'Delete failed', String(err && err.message ? err.message : err));
});
}

document.addEventListener('click', function (e) {
var t = /** @type {Element|null} */ (e.target);
if (!t) return;

if (t.closest && t.closest('[data-testid="comment-delete-cancel"]')) {
e.preventDefault();
closeDeleteDialog();
return;
}

var trigger = /** @type {HTMLElement|null} */ (t.closest && t.closest('[data-action="comment-delete"]'));
if (!trigger) return;
e.preventDefault();

var cid = parseInt(trigger.getAttribute('data-cid') || '0', 10);
var ctype = trigger.getAttribute('data-ctype') || '';
var page = parseInt(trigger.getAttribute('data-page') || '-1', 10);

if (!cid || !ctype) {
toast('error', 'Delete failed', 'Missing comment context.');
return;
}

openDeleteDialog({ cid: cid, ctype: ctype, page: page, trigger: trigger });
});

document.addEventListener('submit', function (e) {
var form = /** @type {Element|null} */ (e.target);
if (!form || !(/** @type {Element} */ (form)).closest) return;
if (!form.matches('[data-testid="comment-delete-form"]')) return;
e.preventDefault();
if (!pending) return;
runDelete(pending);
});

document.addEventListener('cancel', function (e) {
var t = /** @type {Element|null} */ (e.target);
if (!t || t.id !== 'comment-delete-dialog') return;
pending = null;
});
})();
34 changes: 10 additions & 24 deletions web/tests/e2e/specs/flows/comment-delete-dispatcher.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@
* rather than `onclick="RemoveComment(...)"`.
* 2. `web/scripts/comment-actions.js` (new file) carries the
* single document-level event delegate that consumes those
* data attributes, prompts via `window.confirm`, and dispatches
* to `Actions.BansRemoveComment` via `sb.api.call`.
* data attributes, opens a styled `<dialog>` confirm, and
* dispatches to `Actions.BansRemoveComment` via `sb.api.call`.
* 3. The script is included globally in `core/footer.tpl` so the
* contract is symmetric across every page that renders a
* `delcomlink`.
Expand All @@ -34,8 +34,8 @@
*
* - fires the click handler on a `[data-action="comment-delete"]`
* trigger (proving the dispatcher loaded + is listening),
* - shows a `window.confirm` prompt before calling the API
* (proving the destructive-action gate),
* - paints `[data-testid="comment-delete-dialog"]` before calling
* the API (proving the destructive-action gate),
* - sends the API call with cid + ctype + page extracted from
* the data attributes,
* - aborts when the user dismisses the confirm.
Expand Down Expand Up @@ -98,14 +98,6 @@ test.describe('flow: comment-delete dispatcher (#1402 — RemoveComment zombie)'

await page.goto(ADMIN_BANS_PROTESTS_ROUTE);

// Auto-accept the window.confirm prompt the dispatcher
// raises before firing the API.
page.once('dialog', async (dialog) => {
expect(dialog.type()).toBe('confirm');
expect(dialog.message()).toMatch(/delete/i);
await dialog.accept();
});

// Inject a synthetic trigger anywhere on the page. The
// dispatcher is a document-level delegate, so the position
// is irrelevant.
Expand All @@ -122,6 +114,8 @@ test.describe('flow: comment-delete dispatcher (#1402 — RemoveComment zombie)'
});

await page.locator('[data-testid="synth-delcomlink"]').click();
await expect(page.locator('[data-testid="comment-delete-dialog"]')).toBeVisible();
await page.locator('[data-testid="comment-delete-submit"]').click();

// The API call must have landed with the values from the
// data attributes.
Expand Down Expand Up @@ -151,11 +145,6 @@ test.describe('flow: comment-delete dispatcher (#1402 — RemoveComment zombie)'

await page.goto(ADMIN_BANS_PROTESTS_ROUTE);

// Dismiss the prompt.
page.once('dialog', async (dialog) => {
await dialog.dismiss();
});

await page.evaluate(() => {
const a = document.createElement('a');
a.setAttribute('href', '#');
Expand All @@ -169,14 +158,11 @@ test.describe('flow: comment-delete dispatcher (#1402 — RemoveComment zombie)'
});

await page.locator('[data-testid="synth-delcomlink-cancel"]').click();
await expect(page.locator('[data-testid="comment-delete-dialog"]')).toBeVisible();
await page.locator('[data-testid="comment-delete-cancel"]').click();

// The cancelled-confirm path returns synchronously from the
// dispatcher (window.confirm → false → return without
// touching the API). Playwright's click() awaits the click
// event's handlers, so by the time the awaited click resolves
// the dispatcher has already early-returned. No settle timer
// needed (AGENTS.md "Playwright E2E specifics" flags
// `waitForTimeout` for negative assertions as an anti-pattern).
// Cancel closes the dialog without touching the API.
await expect(page.locator('[data-testid="comment-delete-dialog"]')).toBeHidden();
expect(apiCalls, 'cancelled confirm must NOT call the API').toBe(0);
});

Expand Down
Loading
Loading