From b22a07c6fae035e61dbae2896a3d766fa75d90f0 Mon Sep 17 00:00:00 2001 From: Matt Warren Date: Thu, 14 May 2026 12:25:54 -0700 Subject: [PATCH 1/2] Set correct column types and full cell description for simple tables --- src/Client/features/dataTableProvider.ts | 85 ++++++++++++++----- .../tests/unit/dataTableProvider.test.ts | 62 ++++++++------ 2 files changed, 98 insertions(+), 49 deletions(-) diff --git a/src/Client/features/dataTableProvider.ts b/src/Client/features/dataTableProvider.ts index 97e1f3a..0076952 100644 --- a/src/Client/features/dataTableProvider.ts +++ b/src/Client/features/dataTableProvider.ts @@ -16,7 +16,7 @@ import type { IServer, ResultTable } from './server'; import type { IWebView } from './webview'; import type { IClipboard } from './clipboard'; import { formatCfHtml } from './clipboard'; -import { resultTableToHtml, escapeHtml } from './html'; +import { resultTableToHtml } from './html'; import { resultTableToMarkdown } from './markdown'; // ─── Interfaces ───────────────────────────────────────────────────────────── @@ -45,17 +45,18 @@ export interface IDataTableProvider { // ─── Implementation ───────────────────────────────────────────────────────── -// Cell values and column names must be HTML-escaped before being handed to -// Simple-DataTables, which renders both via innerHTML. Without escaping, a -// value like `Name ` causes the grid to fail with -// "InvalidCharacterError: Failed to execute 'createElement' ..." because -// the angle-bracketed substring is parsed as a tag name. We reuse the shared -// `escapeHtml` from html.ts so this stays in sync with HTML rendering used -// elsewhere (clipboard "copy as HTML", markdown export, etc.). +// Cell values are passed raw (unescaped) to the webview. In the init +// script each value is wrapped in a Simple-DataTables cell object with +// all three fields set (data, text, order). That triggers the early-return +// path in Simple-DataTables' readDataCell, so it never parses cell strings +// as HTML — avoiding the InvalidCharacterError that values like +// "George " used to trigger — while still letting the library +// own row rendering (paging, virtualization). The library renders cell +// `text` as textContent, so characters like " and & display correctly +// without any escaping. function formatCellValue(value: unknown): string { if (value === null || value === undefined) return ''; - const text = (typeof value === 'object') ? JSON.stringify(value) : String(value); - return escapeHtml(text); + return (typeof value === 'object') ? JSON.stringify(value) : String(value); } /** Generate a short random token for message scoping. */ @@ -89,11 +90,10 @@ class DataTableView implements IDataTableView { }); const data = { - // Column names are HTML-escaped for the same reason as cell values: - // Simple-DataTables renders headings via innerHTML, so a column name - // containing `<...>` would either inject markup or throw the same - // `InvalidCharacterError` we saw with cell values. - columns: table.columns.map(c => ({ ...c, name: escapeHtml(c.name) })), + // Pass column names and cell values raw (no HTML escaping). + // The init script pre-populates the table DOM using textContent, + // so Simple-DataTables only ever reads back values via innerText. + columns: table.columns.map(c => ({ ...c })), rows: table.rows.map(row => row.map(cell => formatCellValue(cell))) }; const json = JSON.stringify(data).replace(/<\//g, '<\\/'); @@ -267,10 +267,14 @@ class DataTableView implements IDataTableView { var tableData = ${tableDataJson}; // ── Initialize Simple-DataTables grid ── - var headings = tableData.columns.map(function(c) { return c.name; }); - var data = tableData.rows; // Map Kusto column types to Simple-DataTables sort types so numeric and - // date columns sort correctly instead of as text. + // date columns sort correctly instead of as text. Importantly, we set + // an explicit type for EVERY column (not just numeric/date/bool), because + // Simple-DataTables defaults the type to "html" — and for html-typed + // columns the renderer treats cell.data as an array of node objects to + // splice into the . Our cell.data is a string, which breaks + // rendering. Using "string" type makes the renderer go through the + // text-content path and use cell.text instead. var columnSettings = []; tableData.columns.forEach(function(c, i) { var kustoType = c.type; @@ -279,14 +283,51 @@ class DataTableView implements IDataTableView { kustoType === 'real' || kustoType === 'decimal') ? 'number' : (kustoType === 'datetime') ? 'date' : (kustoType === 'bool') ? 'boolean' - : null; - if (sortType) { - columnSettings.push({ select: i, type: sortType }); + : 'string'; + columnSettings.push({ select: i, type: sortType }); + }); + + // Compute a sort-friendly "order" value for a raw cell string based on + // the Simple-DataTables column type. We supply this ourselves so the + // library does not need to parse the cell as HTML to derive it. + function computeOrder(text, sortType) { + if (sortType === 'number') { + var n = parseFloat(text); + return isNaN(n) ? text : n; + } + if (sortType === 'date') { + var t = Date.parse(text); + return isNaN(t) ? text : t; + } + if (sortType === 'boolean') { + var s = String(text).toLowerCase().trim(); + return (s === 'false' || s === '0' || s === '' || s === 'null' || s === 'undefined') ? 0 : 1; } + return text; + } + var columnSortTypes = columnSettings.map(function(c) { return c.type; }); + + // Build heading cell objects (data + text) and row cells (data + text + + // order). Plain objects whose keys are all in [data, text, order, + // attributes] are returned as-is by Simple-DataTables' readDataCell — + // it never parses cell strings as HTML in that path — avoiding the + // InvalidCharacterError that <...> values used to trigger. Rendering + // goes through the library's normal paged row rendering, so we keep + // the benefits of not materializing every / ourselves. + var headings = tableData.columns.map(function(c) { + return { data: c.name, text: c.name }; + }); + var rows = tableData.rows.map(function(row) { + return row.map(function(text, i) { + var sortType = columnSortTypes[i]; + return { data: text, text: text, order: computeOrder(text, sortType) }; + }); }); + var grid = new simpleDatatables.DataTable(tableEl, { - data: { headings: headings, data: data }, + data: { headings: headings, data: rows }, columns: columnSettings, + type: 'string', perPage: 100, perPageSelect: [50, 100, 500, 1000], searchable: true, diff --git a/src/Client/tests/unit/dataTableProvider.test.ts b/src/Client/tests/unit/dataTableProvider.test.ts index c6b3556..81c56ae 100644 --- a/src/Client/tests/unit/dataTableProvider.test.ts +++ b/src/Client/tests/unit/dataTableProvider.test.ts @@ -153,10 +153,12 @@ describe('SimpleDataTableProvider', () => { provider.createView(webview, table); const html: string = webview.setContent.mock.calls[0]![0]; - // formatCellValue produces '{"a":1}', then HTML-escapes the quotes - // to '{"a":1}', which is then JSON.stringified into the - // embedded data payload. - expect(html).toContain('{"a":1}'); + // formatCellValue produces '{"a":1}'. That string is then JSON + // serialized for the embedded payload, so the inner `"` become + // `\"`. We no longer HTML-escape cell values (Simple-DataTables + // renders cell text via textContent), so `"` must NOT appear. + expect(html).toContain('{\\"a\\":1}'); + expect(html).not.toContain('"'); }); it('escapes closing script tags in JSON data', () => { @@ -168,19 +170,26 @@ describe('SimpleDataTableProvider', () => { provider.createView(webview, table); const html: string = webview.setContent.mock.calls[0]![0]; - // The cell value is HTML-escaped before JSON serialization, so the - // raw `` text never appears in the embedded data. Only - // the real closing tag should appear (at the very end). + // The cell value is no longer HTML-escaped, but the embedded JSON + // is still protected against early `` termination by the + // ` `<\/` replacement at serialization time. Only the real + // closing tag should appear; the cell payload should use + // the `<\/script>` form. const matches = html.match(/<\/script>/g); expect(matches).toHaveLength(1); - // The escaped form of '<' (`<`) is what ends up in the JSON. - expect(html).toContain('</script>'); + expect(html).toContain('<\\/script>'); }); - it('HTML-escapes angle brackets in cell values so the grid does not parse them as tags', () => { + it('preserves angle brackets in cell values (passed via cell objects)', () => { // Repro for the customer bug: `print Requestor = "George Washington "` // crashed Simple-DataTables with `InvalidCharacterError: Failed to execute 'createElement'` // because it interpreted the `` substring as an HTML tag. + // + // The fix is to pass each cell as a Simple-DataTables cell + // object with all three of data/text/order set, which triggers + // the early-return path in readDataCell and bypasses HTML + // parsing entirely. Angle brackets therefore survive verbatim + // in the JSON payload. const table = makeTable( [{ name: 'Requestor', type: 'string' }], [['George Washington ']], @@ -189,13 +198,11 @@ describe('SimpleDataTableProvider', () => { provider.createView(webview, table); const html: string = webview.setContent.mock.calls[0]![0]; - // The raw `` substring must NOT survive into the grid's - // data payload, where Simple-DataTables would render it as HTML. - expect(html).not.toContain(''); - expect(html).toContain('<gwashington@contoso.com>'); + expect(html).toContain('George Washington '); + expect(html).not.toContain('<gwashington@contoso.com>'); }); - it('HTML-escapes ampersands and quotes in cell values', () => { + it('preserves ampersands and quotes in cell values', () => { const table = makeTable( [{ name: 'Col', type: 'string' }], [['Tom & Jerry'], ['She said "hi"'], ["It's fine"]], @@ -204,19 +211,20 @@ describe('SimpleDataTableProvider', () => { provider.createView(webview, table); const html: string = webview.setContent.mock.calls[0]![0]; - expect(html).toContain('Tom & Jerry'); - // Quote becomes ". - expect(html).toContain('"'); - // Apostrophes are intentionally not escaped (matching the shared - // escapeHtml helper in html.ts) since they are safe in element - // content. The literal `'` should make it through. + // Cell values are no longer HTML-escaped — they appear raw in + // the JSON payload (with JSON's own `\"` quote escaping). + expect(html).toContain('Tom & Jerry'); + expect(html).not.toContain('&'); + expect(html).not.toContain('"'); + expect(html).toContain('She said \\"hi\\"'); expect(html).toContain("It's fine"); }); - it('HTML-escapes angle brackets in column names so headings do not parse as tags', () => { - // Simple-DataTables renders headings via innerHTML too, so a column - // name containing `<...>` would otherwise either inject markup or - // throw the same InvalidCharacterError we saw with cell values. + it('preserves angle brackets in column names (passed via cell objects)', () => { + // Column names are passed as Simple-DataTables heading cell + // objects (data + text + type), which the library uses without + // re-parsing as HTML, so angle brackets survive raw in the JSON + // payload and do not need HTML-escaping. const table = makeTable( [{ name: 'Value ', type: 'real' }], [[42]], @@ -225,8 +233,8 @@ describe('SimpleDataTableProvider', () => { provider.createView(webview, table); const html: string = webview.setContent.mock.calls[0]![0]; - expect(html).not.toContain('Value '); - expect(html).toContain('Value <units>'); + expect(html).toContain('Value '); + expect(html).not.toContain('Value <units>'); }); it('includes Simple-DataTables initialization in the script', () => { From 277bdde0dd235ea677729a79528222179bbcde1a Mon Sep 17 00:00:00 2001 From: Matt Warren Date: Thu, 14 May 2026 12:37:38 -0700 Subject: [PATCH 2/2] Fixes for PR feedback --- src/Client/features/dataTableProvider.ts | 8 +++++--- src/Client/tests/unit/dataTableProvider.test.ts | 7 ++++--- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/Client/features/dataTableProvider.ts b/src/Client/features/dataTableProvider.ts index 0076952..8f4f9aa 100644 --- a/src/Client/features/dataTableProvider.ts +++ b/src/Client/features/dataTableProvider.ts @@ -91,8 +91,11 @@ class DataTableView implements IDataTableView { const data = { // Pass column names and cell values raw (no HTML escaping). - // The init script pre-populates the table DOM using textContent, - // so Simple-DataTables only ever reads back values via innerText. + // The init script wraps each cell as a { data, text, order } object + // and configures an explicit per-column `type` (never "html"), so + // Simple-DataTables takes its early-return path in readDataCell and + // never parses cell strings as HTML. Rendering goes through the + // textContent path, so '<', '>', '&', and '"' display verbatim. columns: table.columns.map(c => ({ ...c })), rows: table.rows.map(row => row.map(cell => formatCellValue(cell))) }; @@ -327,7 +330,6 @@ class DataTableView implements IDataTableView { var grid = new simpleDatatables.DataTable(tableEl, { data: { headings: headings, data: rows }, columns: columnSettings, - type: 'string', perPage: 100, perPageSelect: [50, 100, 500, 1000], searchable: true, diff --git a/src/Client/tests/unit/dataTableProvider.test.ts b/src/Client/tests/unit/dataTableProvider.test.ts index 81c56ae..fcb4fc9 100644 --- a/src/Client/tests/unit/dataTableProvider.test.ts +++ b/src/Client/tests/unit/dataTableProvider.test.ts @@ -222,9 +222,10 @@ describe('SimpleDataTableProvider', () => { it('preserves angle brackets in column names (passed via cell objects)', () => { // Column names are passed as Simple-DataTables heading cell - // objects (data + text + type), which the library uses without - // re-parsing as HTML, so angle brackets survive raw in the JSON - // payload and do not need HTML-escaping. + // objects ({ data, text }), with column types supplied separately + // via the `columns` config. The library returns these heading + // objects as-is without re-parsing as HTML, so angle brackets + // survive raw in the JSON payload and do not need HTML-escaping. const table = makeTable( [{ name: 'Value ', type: 'real' }], [[42]],