From 5f7665f74dc675175ecc36315b740b6f76176fdf Mon Sep 17 00:00:00 2001 From: amaannawab923 Date: Tue, 17 Jun 2025 21:03:04 +0530 Subject: [PATCH 1/9] Fixing context menu with html values in table cells --- .../plugin-chart-table/src/TableChart.tsx | 31 +++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx b/superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx index 8d921c3d3092..41fdc854bf55 100644 --- a/superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx +++ b/superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx @@ -243,6 +243,24 @@ function SelectPageSize({ const getNoResultsMessage = (filter: string) => filter ? t('No matching records found') : t('No records found'); +const isHtml = (value: any): boolean => { + if (typeof value !== 'string') { + return false; + } + return value.includes('<') && value.includes('>'); +}; + +const stripHtmlAndGetValue = (value: any): string | number | boolean => { + // Create a temporary DOM element to parse the HTML + const tempDiv = document.createElement('div'); + tempDiv.innerHTML = value; + + // Get the text content (this removes all HTML tags) + const textContent = tempDiv.textContent || tempDiv.innerText || ''; + + return textContent.trim(); +}; + export default function TableChart( props: TableChartTransformedProps & { sticky?: DataTableProps['sticky']; @@ -468,7 +486,11 @@ export default function TableChart( const drillToDetailFilters: BinaryQueryObjectFilterClause[] = []; filteredColumnsMeta.forEach(col => { if (!col.isMetric) { - const dataRecordValue = value[col.key]; + let dataRecordValue = value[col.key]; + if (isHtml(dataRecordValue)) { + dataRecordValue = stripHtmlAndGetValue(dataRecordValue); + } + drillToDetailFilters.push({ col: col.key, op: '==', @@ -489,7 +511,12 @@ export default function TableChart( { col: cellPoint.key, op: '==', - val: cellPoint.value as string | number | boolean, + val: isHtml(cellPoint.value) + ? (stripHtmlAndGetValue(cellPoint.value) as + | string + | number + | boolean) + : (cellPoint.value as string | number | boolean), }, ], groupbyFieldName: 'groupby', From e0e1e7d96b8e108dc7f51723f6f1156380c9c511 Mon Sep 17 00:00:00 2001 From: amaannawab923 Date: Thu, 14 Aug 2025 11:25:36 +0530 Subject: [PATCH 2/9] update: use superset ui core functions as per comments --- .../plugin-chart-table/src/TableChart.tsx | 32 +++++-------------- 1 file changed, 8 insertions(+), 24 deletions(-) diff --git a/superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx b/superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx index 41fdc854bf55..610ad18c1c79 100644 --- a/superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx +++ b/superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx @@ -52,6 +52,8 @@ import { t, tn, useTheme, + isProbablyHTML, + removeHTMLTags, } from '@superset-ui/core'; import { Dropdown, Menu, Tooltip } from '@superset-ui/chart-controls'; import { @@ -243,22 +245,11 @@ function SelectPageSize({ const getNoResultsMessage = (filter: string) => filter ? t('No matching records found') : t('No records found'); -const isHtml = (value: any): boolean => { - if (typeof value !== 'string') { - return false; +const sanitizeHtmlValue = (value: DataRecordValue): DataRecordValue => { + if (typeof value === 'string' && isProbablyHTML(value)) { + return removeHTMLTags(value); } - return value.includes('<') && value.includes('>'); -}; - -const stripHtmlAndGetValue = (value: any): string | number | boolean => { - // Create a temporary DOM element to parse the HTML - const tempDiv = document.createElement('div'); - tempDiv.innerHTML = value; - - // Get the text content (this removes all HTML tags) - const textContent = tempDiv.textContent || tempDiv.innerText || ''; - - return textContent.trim(); + return value; }; export default function TableChart( @@ -487,9 +478,7 @@ export default function TableChart( filteredColumnsMeta.forEach(col => { if (!col.isMetric) { let dataRecordValue = value[col.key]; - if (isHtml(dataRecordValue)) { - dataRecordValue = stripHtmlAndGetValue(dataRecordValue); - } + dataRecordValue = sanitizeHtmlValue(dataRecordValue); drillToDetailFilters.push({ col: col.key, @@ -511,12 +500,7 @@ export default function TableChart( { col: cellPoint.key, op: '==', - val: isHtml(cellPoint.value) - ? (stripHtmlAndGetValue(cellPoint.value) as - | string - | number - | boolean) - : (cellPoint.value as string | number | boolean), + val: sanitizeHtmlValue(cellPoint.value), }, ], groupbyFieldName: 'groupby', From d22eb48c3f43d680977b81c8610978c8c470d137 Mon Sep 17 00:00:00 2001 From: amaannawab923 Date: Thu, 14 Aug 2025 17:00:59 +0530 Subject: [PATCH 3/9] update: give appropriate name to function for extracting text from html --- .../plugins/plugin-chart-table/src/TableChart.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx b/superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx index be79d85e459c..35312b04ebb1 100644 --- a/superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx +++ b/superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx @@ -253,7 +253,7 @@ function SelectPageSize({ const getNoResultsMessage = (filter: string) => filter ? t('No matching records found') : t('No records found'); -const sanitizeHtmlValue = (value: DataRecordValue): DataRecordValue => { +const extractTextFromHTML = (value: DataRecordValue): DataRecordValue => { if (typeof value === 'string' && isProbablyHTML(value)) { return removeHTMLTags(value); } @@ -488,7 +488,7 @@ export default function TableChart( filteredColumnsMeta.forEach(col => { if (!col.isMetric) { let dataRecordValue = value[col.key]; - dataRecordValue = sanitizeHtmlValue(dataRecordValue); + dataRecordValue = extractTextFromHTML(dataRecordValue); drillToDetailFilters.push({ col: col.key, @@ -510,7 +510,7 @@ export default function TableChart( { col: cellPoint.key, op: '==', - val: sanitizeHtmlValue(cellPoint.value), + val: extractTextFromHTML(cellPoint.value), }, ], groupbyFieldName: 'groupby', From 0f51c9682a120fa5e881d4c990de26dbe22682d2 Mon Sep 17 00:00:00 2001 From: amaannawab923 Date: Thu, 14 Aug 2025 21:50:27 +0530 Subject: [PATCH 4/9] update: move extract text from html to html module in superset ui / core --- .../packages/superset-ui-core/src/utils/html.tsx | 8 ++++++++ .../plugins/plugin-chart-table/src/TableChart.tsx | 10 +--------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/superset-frontend/packages/superset-ui-core/src/utils/html.tsx b/superset-frontend/packages/superset-ui-core/src/utils/html.tsx index e1514976985e..2fe58a11ae75 100644 --- a/superset-frontend/packages/superset-ui-core/src/utils/html.tsx +++ b/superset-frontend/packages/superset-ui-core/src/utils/html.tsx @@ -17,6 +17,7 @@ * under the License. */ import { FilterXSS, getDefaultWhiteList } from 'xss'; +import { DataRecordValue } from '../types'; const xssFilter = new FilterXSS({ whiteList: { @@ -204,3 +205,10 @@ export function getParagraphContents( return paragraphContents; } + +export function extractTextFromHTML(value: DataRecordValue): DataRecordValue { + if (typeof value === 'string' && isProbablyHTML(value)) { + return removeHTMLTags(value); + } + return value; +} diff --git a/superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx b/superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx index 35312b04ebb1..c1465827879d 100644 --- a/superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx +++ b/superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx @@ -52,9 +52,8 @@ import { t, tn, useTheme, - isProbablyHTML, - removeHTMLTags, SupersetTheme, + extractTextFromHTML, } from '@superset-ui/core'; import { Input, @@ -253,13 +252,6 @@ function SelectPageSize({ const getNoResultsMessage = (filter: string) => filter ? t('No matching records found') : t('No records found'); -const extractTextFromHTML = (value: DataRecordValue): DataRecordValue => { - if (typeof value === 'string' && isProbablyHTML(value)) { - return removeHTMLTags(value); - } - return value; -}; - export default function TableChart( props: TableChartTransformedProps & { sticky?: DataTableProps['sticky']; From b7444726d08435e33e01cd70d9d35b12a21c9a0a Mon Sep 17 00:00:00 2001 From: amaannawab923 Date: Fri, 15 Aug 2025 12:28:23 +0530 Subject: [PATCH 5/9] update: unit tests for extract text from html --- .../superset-ui-core/src/utils/html.test.tsx | 99 +++++++++++++++++++ 1 file changed, 99 insertions(+) diff --git a/superset-frontend/packages/superset-ui-core/src/utils/html.test.tsx b/superset-frontend/packages/superset-ui-core/src/utils/html.test.tsx index e94230de2e49..87cb0abb6e21 100644 --- a/superset-frontend/packages/superset-ui-core/src/utils/html.test.tsx +++ b/superset-frontend/packages/superset-ui-core/src/utils/html.test.tsx @@ -24,6 +24,7 @@ import { removeHTMLTags, isJsonString, getParagraphContents, + extractTextFromHTML, } from './html'; describe('sanitizeHtml', () => { @@ -204,3 +205,101 @@ describe('getParagraphContents', () => { }); }); }); + +describe('extractTextFromHTML', () => { + it('should extract text from HTML div tags', () => { + const htmlString = '
Hello World
'; + const result = extractTextFromHTML(htmlString); + expect(result).toBe('Hello World'); + }); + + it('should extract text from nested HTML tags', () => { + const htmlString = '

Hello World

'; + const result = extractTextFromHTML(htmlString); + expect(result).toBe('Hello World'); + }); + + it('should extract text from multiple HTML elements', () => { + const htmlString = '

Title

Content

Footer'; + const result = extractTextFromHTML(htmlString); + expect(result).toBe('TitleContentFooter'); + }); + + it('should return original string when input is not HTML', () => { + const plainText = 'Just plain text'; + const result = extractTextFromHTML(plainText); + expect(result).toBe('Just plain text'); + }); + + it('should return original value when input is not a string', () => { + const numberValue = 12345; + const result = extractTextFromHTML(numberValue); + expect(result).toBe(12345); + + const nullValue = null; + const nullResult = extractTextFromHTML(nullValue); + expect(nullResult).toBe(null); + + const booleanValue = true; + const booleanResult = extractTextFromHTML(booleanValue); + expect(booleanResult).toBe(true); + }); + + it('should handle empty HTML tags', () => { + const htmlString = '
'; + const result = extractTextFromHTML(htmlString); + expect(result).toBe(''); + }); + + it('should handle HTML with only whitespace', () => { + const htmlString = '
'; + const result = extractTextFromHTML(htmlString); + expect(result).toBe(' '); + }); + + it('should extract text from HTML with attributes', () => { + const htmlString = '
Hello World
'; + const result = extractTextFromHTML(htmlString); + expect(result).toBe('Hello World'); + }); + + it('should handle self-closing tags', () => { + const htmlString = 'Image

Text after

'; + const result = extractTextFromHTML(htmlString); + expect(result).toBe('Text after'); + }); + + it('should handle complex HTML structure', () => { + const htmlString = ` + + Page Title + +

Main Title

+
+

First paragraph with emphasis.

+
    +
  • Item 1
    • +
  • Item 2
    • +
+
+ + + `; + const result = extractTextFromHTML(htmlString); + expect(result).toContain('Page Title'); + expect(result).toContain('Main Title'); + expect(result).toContain('First paragraph with emphasis.'); + expect(result).toContain('Item 1'); + expect(result).toContain('Item 2'); + }); + + it('should not extract text from strings that look like HTML but are not', () => { + const fakeHtmlString = ''; + const result = extractTextFromHTML(fakeHtmlString); + expect(result).toBe(''); + + const mathExpression = 'x < 5 and y > 10'; + const mathResult = extractTextFromHTML(mathExpression); + expect(mathResult).toBe('x < 5 and y > 10'); + }); +}); From 5999f56329e3f3041f1756a6818fba3895d91abf Mon Sep 17 00:00:00 2001 From: amaannawab923 Date: Thu, 25 Sep 2025 19:48:15 +0530 Subject: [PATCH 6/9] update: fix the codeql ci cd warning --- .../packages/superset-ui-core/src/utils/html.tsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/superset-frontend/packages/superset-ui-core/src/utils/html.tsx b/superset-frontend/packages/superset-ui-core/src/utils/html.tsx index 2fe58a11ae75..3055108f0ba7 100644 --- a/superset-frontend/packages/superset-ui-core/src/utils/html.tsx +++ b/superset-frontend/packages/superset-ui-core/src/utils/html.tsx @@ -170,7 +170,10 @@ export function safeHtmlSpan(possiblyHtmlString: string) { } export function removeHTMLTags(str: string): string { - return str.replace(/<[^>]*>/g, ''); + const doc = new DOMParser().parseFromString(str, 'text/html'); + const bodyText = doc.body?.textContent || ''; + const headText = doc.head?.textContent || ''; + return headText + bodyText; } export function isJsonString(str: string): boolean { From 04b9db9c6d64221585cc7c1ce2a7f840695b3b64 Mon Sep 17 00:00:00 2001 From: amaannawab923 Date: Fri, 7 Nov 2025 01:41:13 +0530 Subject: [PATCH 7/9] ci fix --- .../explore/components/PropertiesModal/PropertiesModal.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset-frontend/src/explore/components/PropertiesModal/PropertiesModal.test.tsx b/superset-frontend/src/explore/components/PropertiesModal/PropertiesModal.test.tsx index 4a194b1e7de5..8ee41ea4909d 100644 --- a/superset-frontend/src/explore/components/PropertiesModal/PropertiesModal.test.tsx +++ b/superset-frontend/src/explore/components/PropertiesModal/PropertiesModal.test.tsx @@ -91,7 +91,7 @@ fetchMock.get('glob:*/api/v1/chart/318*', { certification_details: 'Test certification details', certified_by: 'Test certified by', description: 'Test description', - cache_timeout: '1000', + cache_timeout: 1000, slice_name: 'Test chart new name', }, show_columns: [ From ebc119fe11d7ccbb0c609da426f7867c49b634fc Mon Sep 17 00:00:00 2001 From: amaannawab923 Date: Fri, 7 Nov 2025 02:00:06 +0530 Subject: [PATCH 8/9] test fix --- .../FiltersConfigModal/FiltersConfigModal.test.tsx | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.test.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.test.tsx index af57bee7597c..a585ecf2101d 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.test.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.test.tsx @@ -310,7 +310,12 @@ test('render time filter types as disabled if there are no temporal columns in t test('validates the name', async () => { defaultRender(); userEvent.click(screen.getByRole('button', { name: SAVE_REGEX })); - expect(await screen.findByText(NAME_REQUIRED_REGEX)).toBeInTheDocument(); + await waitFor( + async () => { + expect(await screen.findByText(NAME_REQUIRED_REGEX)).toBeInTheDocument(); + }, + { timeout: 10000 }, + ); }); test('validates the column', async () => { From 40877cffae2dfeff8b31a7bdd498a70cc38941b5 Mon Sep 17 00:00:00 2001 From: amaannawab923 Date: Fri, 7 Nov 2025 02:23:14 +0530 Subject: [PATCH 9/9] test fix --- .../FiltersConfigModal/FiltersConfigModal.test.tsx | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.test.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.test.tsx index a585ecf2101d..17385970c5fd 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.test.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.test.tsx @@ -357,9 +357,12 @@ test('validates the pre-filter value', async () => { jest.useRealTimers(); // Wait for validation to complete after timer switch - await waitFor(() => { - expect(screen.getByText(PRE_FILTER_REQUIRED_REGEX)).toBeInTheDocument(); - }); + await waitFor( + () => { + expect(screen.getByText(PRE_FILTER_REQUIRED_REGEX)).toBeInTheDocument(); + }, + { timeout: 15000 }, + ); }, 50000); // Slow-running test, increase timeout to 50 seconds. // eslint-disable-next-line jest/no-disabled-tests