fix(analytics): render Channels group-header sprites as HTML, not escaped text (#1657) (#1658)

Fixes #1657 ## Bug On `/analytics` → **Channels** tab, the "Channel Activity" table's group-header rows ("My Channels", "Network", "Encrypted") rendered literal HTML source text: ``` <SVG CLASS="PH-ICON" ARIA-HIDDEN="TRUE"><USE HREF="/ICONS/PHOSPHOR-SPRITE.SVG#PH-KEY"/></SVG> My Channels ``` instead of the actual Phosphor sprites. Per-row encrypted/lock icons rendered fine — the bug was isolated to the group-header render path. ## Root cause `public/analytics.js` `channelTbodyHtml` builds each group-section header by wrapping the section label in `esc()`: ```js esc(sections[si].label) + ' <span class="text-muted">(' + rows.length + ')</span>' ``` But the labels (`sections[].label`) are hardcoded sprite-bearing strings: ```js { key: 'mine', label: '<svg class="ph-icon" aria-hidden="true"><use href="…#ph-key"/></svg> My Channels' }, ``` `esc()` HTML-encoded the `<` / `>` so the browser displayed the source text rather than rendering the sprite. Affects all 3 groups (and any future group with a sprite). ## Fix Drop the `esc()` wrap on the hardcoded label (single line change, same pattern as M3 commit 4ca73ced for mobile channel avatars). The `(<count>)` suffix is numeric and was always safe. ## Tests New `test-issue-1657-analytics-channels-group-sprites-e2e.js` (mobile 375 viewport, matching the bug report): - (1) at least one group-header row renders - (2) every header row contains a real `<svg.ph-icon>` child - (3) per-group sprite refs resolve (My Channels → `#ph-key`, Network → `#ph-radio`, Encrypted → `#ph-lock`) - (4) the Channel Activity table's `innerText` contains no literal `<svg` substring (escape-leak gate) Wired into the CI E2E lane (`.github/workflows/deploy.yml`) immediately after the M4 icons E2E. ## TDD evidence - Red commit: `8f8781c1` — test + CI wiring only, no production change. Test asserts behavior that did not exist on master → CI fails on this commit. - Green commit: `8385fa54` — 1-line fix to `public/analytics.js`. Test passes. ## Anti-tautology proof Hot-patched staging with the `analytics.js` from the red commit (pre-fix), reloaded `/analytics?tab=channels` at 375 viewport, and the in-browser DOM probe returned: ``` headers[0].text = "<svg class=\"ph-icon\" aria-hidden=\"true\"><use href=\"/icons/phosphor-sprite.svg#ph…" headers[0].svgs = 0 // (2) would fail headers[1].svgs = 0 // (2) would fail literalSvg = true // (4) would fail ``` Restored the fixed file; same probe returned `svgs=1`, correct `uses[]` refs, `literalSvg=false`. ## Staging verification Hot-patched `corescope-staging-go:/app/public/analytics.js` (no restart needed — static file). Mobile dark @ 375 viewport shows Network → radio sprite and Encrypted → lock sprite rendering correctly. (My Channels group not present because the e2e fixture has no `mine`-tagged channels — expected; the test skips that assertion when the row is absent.) ## Scope discipline Touched only: - `public/analytics.js` (1-line `esc()` removal + comment) - `test-issue-1657-…-e2e.js` (new) - `.github/workflows/deploy.yml` (1-line E2E wire) No broadening, no helper renames, no related-but-different escape-removal opportunism. --------- Co-authored-by: openclaw-bot <bot@openclaw.local>
2026-06-28 18:22:09 +00:00 · 2026-06-11 07:34:51 -07:00
parent 89eade6e7b
commit fb6bb085a5
3 changed files with 154 additions and 1 deletions
@@ -403,6 +403,7 @@ jobs:
          CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-issue-1648-m2-icons-e2e.js 2>&1 | tee -a e2e-output.txt
          CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-issue-1648-m3-icons-e2e.js 2>&1 | tee -a e2e-output.txt
          CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-issue-1648-m4-icons-e2e.js 2>&1 | tee -a e2e-output.txt
+          CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-issue-1657-analytics-channels-group-sprites-e2e.js 2>&1 | tee -a e2e-output.txt
          BASE_URL=http://localhost:13581 node test-issue-1224-channels-mobile-ux-e2e.js 2>&1 | tee -a e2e-output.txt
          BASE_URL=http://localhost:13581 node test-issue-1367-channels-chat-app-e2e.js 2>&1 | tee -a e2e-output.txt
          BASE_URL=http://localhost:13581 node test-issue-1236-map-mobile-e2e.js 2>&1 | tee -a e2e-output.txt
@@ -1033,7 +1033,11 @@
        if (!rows.length) continue;
        parts.push(
          '<tr class="ch-section-row"><td colspan="6" class="ch-section-header">' +
-          esc(sections[si].label) + ' <span class="text-muted">(' + rows.length + ')</span>' +
+          // sections[].label is a hardcoded sprite-bearing string (no user
+          // input) — must be inserted raw so the <svg> renders. esc() here
+          // HTML-escaped the angle brackets and surfaced literal "<svg…>"
+          // text in the table (#1657).
+          sections[si].label + ' <span class="text-muted">(' + rows.length + ')</span>' +
          '</td></tr>'
        );
        for (var ri = 0; ri < rows.length; ri++) parts.push(channelRowHtml(rows[ri]));
@@ -0,0 +1,148 @@
+#!/usr/bin/env node
+/* Issue #1657 — Analytics → Channels: group-header rows must render real
+ * <svg> Phosphor sprites (My Channels / Network / Encrypted), NOT the
+ * HTML-escaped literal "<svg class=\"ph-icon\"…>" source text.
+ *
+ * Bug: public/analytics.js passed the hardcoded group-section label (which
+ * contains the sprite markup) through esc(), HTML-encoding the angle
+ * brackets so the browser displayed the source instead of rendering it.
+ *
+ * This test asserts (in real Chromium against a running server):
+ *   (1) The Channel Activity table has at least one group-header row.
+ *   (2) Each rendered group-header row contains a real <svg.ph-icon>
+ *       element (NOT escaped text).
+ *   (3) For each expected group (key/radio/lock), if the label text is
+ *       present, an actual <use href="…#ph-{key|radio|lock}"> resolves
+ *       inside the same row.
+ *   (4) The Channel Activity table's innerText contains zero literal
+ *       "<svg" substrings (case-insensitive) — i.e. no escape leak.
+ *
+ * CHROMIUM_REQUIRE=1 makes Chromium-launch failure a HARD FAIL.
+ */
+'use strict';
+
+const { chromium } = require('playwright');
+const assert = require('assert');
+
+const BASE = process.env.BASE_URL || 'http://localhost:13581';
+
+let passes = 0, failures = 0;
+function pass(msg) { console.log(`  ✓ ${msg}`); passes++; }
+function fail(msg) { console.error(`  ✗ ${msg}`); failures++; }
+
+async function main() {
+  const requireChromium = process.env.CHROMIUM_REQUIRE === '1';
+  let browser;
+  try {
+    browser = await chromium.launch({ headless: true });
+  } catch (err) {
+    if (requireChromium) {
+      console.error(`HARD FAIL — Chromium unavailable: ${err.message}`);
+      process.exit(1);
+    }
+    console.warn(`SKIP — Chromium unavailable: ${err.message}`);
+    process.exit(0);
+  }
+
+  // Mobile viewport (375 wide) matches the staging screenshot that surfaced the bug.
+  const ctx = await browser.newContext({ viewport: { width: 375, height: 800 } });
+  const page = await ctx.newPage();
+
+  await page.goto(`${BASE}/#/analytics?tab=channels`, { waitUntil: 'domcontentloaded' });
+
+  // Wait for the channels table tbody to populate.
+  try {
+    await page.waitForFunction(() => {
+      const tb = document.getElementById('channelsTbody');
+      return tb && tb.querySelectorAll('tr').length > 0;
+    }, null, { timeout: 12000 });
+  } catch {
+    fail('channelsTbody never populated within 12s');
+    await browser.close();
+    console.log(`\ntest-issue-1657: ${passes} passed, ${failures} failed`);
+    process.exit(failures ? 1 : 0);
+  }
+
+  // Allow grouped render (decorate + buildHashKeyMap promise) to settle.
+  await page.waitForTimeout(800);
+
+  const result = await page.evaluate(() => {
+    const tb = document.getElementById('channelsTbody');
+    if (!tb) return { error: 'tbody missing' };
+    const headerRows = Array.from(tb.querySelectorAll('tr.ch-section-row'));
+    const perHeader = headerRows.map((tr) => {
+      const cell = tr.querySelector('td.ch-section-header') || tr.querySelector('td');
+      const text = (cell && cell.textContent) || '';
+      const svgs = cell ? cell.querySelectorAll('svg.ph-icon').length : 0;
+      const uses = cell
+        ? Array.from(cell.querySelectorAll('svg.ph-icon use'))
+            .map((u) => (u.getAttribute('href') || u.getAttribute('xlink:href') || ''))
+        : [];
+      return { text: text.trim(), svgs, uses };
+    });
+    const tableEl = document.getElementById('channelsTable') || tb.parentElement;
+    const innerHTML = tableEl ? tableEl.innerHTML : '';
+    // innerText reflects what the user sees; literal "<svg" in innerText
+    // means the markup got HTML-escaped before being inserted.
+    const innerText = tableEl ? (tableEl.innerText || tableEl.textContent || '') : '';
+    return { headerRows: perHeader, hasLiteralSvgInText: /<svg/i.test(innerText), htmlLen: innerHTML.length };
+  });
+
+  if (result.error) {
+    fail(result.error);
+    await browser.close();
+    process.exit(1);
+  }
+
+  // (1) at least one header row
+  if (result.headerRows.length === 0) {
+    fail('(1) no group-header rows rendered (expected at least one of My Channels / Network / Encrypted)');
+  } else {
+    pass(`(1) ${result.headerRows.length} group-header row(s) present`);
+  }
+
+  // (2) every header row has a real SVG element
+  let allHaveSvg = true;
+  for (const h of result.headerRows) {
+    if (h.svgs < 1) {
+      fail(`(2) group-header row "${h.text.slice(0, 40)}" has 0 svg.ph-icon children (escape leak)`);
+      allHaveSvg = false;
+    }
+  }
+  if (allHaveSvg && result.headerRows.length) pass(`(2) all ${result.headerRows.length} group-header rows contain a real <svg.ph-icon>`);
+
+  // (3) per expected group, verify the sprite ref
+  const expected = [
+    { match: /My Channels/i, ref: /#ph-key$/, name: 'My Channels → #ph-key' },
+    { match: /Network/i,     ref: /#ph-radio$/, name: 'Network → #ph-radio' },
+    { match: /Encrypted/i,   ref: /#ph-lock$/, name: 'Encrypted → #ph-lock' },
+  ];
+  for (const e of expected) {
+    const row = result.headerRows.find((h) => e.match.test(h.text));
+    if (!row) {
+      // Not all groups are guaranteed to have rows (depends on fixture data).
+      // Skip silently — group only renders if it has channels.
+      continue;
+    }
+    const ok = row.uses.some((u) => e.ref.test(u));
+    if (!ok) fail(`(3) ${e.name}: expected ${e.ref} but uses=${JSON.stringify(row.uses)}`);
+    else pass(`(3) ${e.name} rendered as real sprite`);
+  }
+
+  // (4) no literal "<svg" in the table's user-visible text
+  if (result.hasLiteralSvgInText) {
+    fail('(4) Channel Activity table innerText contains literal "<svg" — sprite markup was HTML-escaped');
+  } else {
+    pass('(4) Channel Activity table innerText is free of literal "<svg" leaks');
+  }
+
+  await browser.close();
+  console.log(`\ntest-issue-1657: ${passes} passed, ${failures} failed`);
+  assert.strictEqual(failures, 0, `${failures} assertion(s) failed`);
+  process.exit(0);
+}
+
+main().catch((err) => {
+  console.error('test-issue-1657: FAIL —', err);
+  process.exit(1);
+});