fix(Context-Menu): Fixing Context Menu for Table Chart with Html Content #33791

amaannawab923 · 2025-06-17T15:35:09Z

SUMMARY

screen-capture.45.webm

Fixes: #33771

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

Has associated issue:
Required feature flags:
Changes UI
Includes DB Migration (follow approval process in SIP-59)
- Migration is atomic, supports rollback & is backwards-compatible
- Confirm DB migration upgrade and downgrade tested
- Runtime estimates and downtime expectations provided
Introduces new feature or API
Removes existing feature or API

korbit-ai

Review by Korbit AI

Korbit automatically attempts to detect when you fix issues in new commits.

Category	Issue	Status
	Missing security and environment documentation ▹ view	✅ Fix detected
	Unreliable HTML Detection ▹ view	✅ Fix detected

Files scanned

File Path	Reviewed
superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx	✅

Explore our documentation to understand the languages and file types we support and the files we ignore.

Check out our docs on how you can make Korbit work best for you and your team.

Loving Korbit!? Share us on LinkedIn Reddit and X

superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx

github-actions · 2025-06-17T17:19:07Z

@sadpandajoe Processing your ephemeral environment request here. Action: up. More information on how to use or configure ephemeral environments

github-actions · 2025-06-17T17:26:31Z

@sadpandajoe Ephemeral environment spinning up at http://35.89.173.179:8080. Credentials are 'admin'/'admin'. Please allow several minutes for bootstrapping and startup.

mistercrunch · 2025-08-12T20:36:10Z

superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx

 const getNoResultsMessage = (filter: string) =>
  filter ? t('No matching records found') : t('No records found');

+const isHtml = (value: any): boolean => {


comment crafted with Claude ->

Hi @amaannawab923, thanks for working on this fix!

For handling HTML content in table cells, we already have comprehensive HTML utilities available in the codebase that should be used:

📍 HTML Utilities Location: @superset-ui/core/src/utils/html

Key functions available:

removeHTMLTags(str) - Strips all HTML tags from a string (useful for getting plain text for context menu operations)

sanitizeHtml(htmlString) - Safely sanitizes HTML content using XSS filtering

isProbablyHTML(text) - Detects if a string contains valid HTML

safeHtmlSpan(possiblyHtmlString) - Returns a safe React component for rendering HTML

The table chart already uses these utilities in superset-frontend/plugins/plugin-chart-table/src/utils/formatValue.ts:54 for sanitizing HTML content when allowRenderHtml is enabled.

For the context menu issue with HTML tags, you might want to use removeHTMLTags() to extract the plain text value for drill-down operations. This is already being done in the drill detail menu items (see
DrillDetailMenuItems.tsx:93).

Example usage:
import { removeHTMLTags } from '@superset-ui/core';

// When handling context menu for HTML cells
const plainTextValue = removeHTMLTags(htmlCellValue);

This approach ensures consistency with how HTML is handled throughout the codebase and maintains proper security sanitization.

mistercrunch · 2025-08-12T20:43:41Z

Converting to draft until adjusted / ready for another round of review

…context-menu-html

amaannawab923 · 2025-08-14T05:58:01Z

Converting to draft until adjusted / ready for another round of review

Did the changes as per recommendation @mistercrunch

korbit-ai

Review by Korbit AI

Korbit automatically attempts to detect when you fix issues in new commits.

Category	Issue	Status
	Insufficient HTML Sanitization ▹ view	✅ Fix detected

Files scanned

File Path	Reviewed
superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx	✅

Explore our documentation to understand the languages and file types we support and the files we ignore.

Check out our docs on how you can make Korbit work best for you and your team.

Loving Korbit!? Share us on LinkedIn Reddit and X

superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx

mistercrunch · 2025-08-14T07:54:16Z

superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx

 const getNoResultsMessage = (filter: string) =>
  filter ? t('No matching records found') : t('No records found');

+const sanitizeHtmlValue = (value: DataRecordValue): DataRecordValue => {


can't use the sanitizeHtml from the utils? Guessing this is different? does this function belongs in the utils or is it component-specific?

Yes this is different , its about extracting value from Html Tag ->
Input :
< /a href="https://www.google.com/search?q=United Kingdom">United Kingdom</a>"

Expected Output:
United Kingdom

Sanitise Html function would just sanitise that html to remove malicious code

For now , i guess this is component specific , because it will only come to use here in Table

I think the function name is causing confusion with html sanitisation , should probably rename it to , ExtractTextFromHtml

mistercrunch · 2025-08-14T17:56:47Z

Add simple covering unit tests and I think this is mergeable!

amaannawab923 · 2025-08-15T06:59:23Z

Add simple covering unit tests and I think this is mergeable!

Done ! Added Unit tests & Thanks for the review

mistercrunch

LGTM

mistercrunch · 2025-08-15T22:10:36Z

@msyavuz why the hold?

msyavuz · 2025-08-15T23:27:12Z

@mistercrunch The issue has a bounty assigned. We need to remove that before this get's merged.

Fixing context menu with html values in table cells

5f7665f

pull-request-size bot added the size/M label Jun 17, 2025

github-actions bot added the plugins label Jun 17, 2025

korbit-ai bot reviewed Jun 17, 2025

View reviewed changes

superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx Outdated Show resolved Hide resolved

superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx Outdated Show resolved Hide resolved

sadpandajoe requested review from kgabryje and geido June 17, 2025 17:18

sadpandajoe added the testenv-up label Jun 17, 2025

mistercrunch reviewed Aug 12, 2025

View reviewed changes

mistercrunch marked this pull request as draft August 12, 2025 20:43

update: use superset ui core functions as per comments

e0e1e7d

pull-request-size bot added size/S and removed size/M labels Aug 14, 2025

Merge branch 'master' of https://github.com/apache/superset into fix/…

fdb3ba1

…context-menu-html

amaannawab923 marked this pull request as ready for review August 14, 2025 05:58

amaannawab923 requested a review from mistercrunch August 14, 2025 05:58

dosubot bot added the viz:charts:table Related to the Table chart label Aug 14, 2025

korbit-ai bot reviewed Aug 14, 2025

View reviewed changes

superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx Outdated Show resolved Hide resolved

mistercrunch reviewed Aug 14, 2025

View reviewed changes

amaannawab923 requested a review from mistercrunch August 14, 2025 09:53

amaannawab923 added 2 commits August 14, 2025 17:00

update: give appropriate name to function for extracting text from html

d22eb48

update: move extract text from html to html module in superset ui / core

0f51c96

github-actions bot added the packages label Aug 14, 2025

update: unit tests for extract text from html

b744472

pull-request-size bot added size/L and removed size/S labels Aug 15, 2025

amaannawab923 closed this Aug 15, 2025

amaannawab923 reopened this Aug 15, 2025

msyavuz added the hold! On hold label Aug 15, 2025

mistercrunch approved these changes Aug 15, 2025

View reviewed changes

msyavuz removed the hold! On hold label Sep 24, 2025

update: fix the codeql ci cd warning

5999f56

amaannawab923 closed this Sep 25, 2025

amaannawab923 reopened this Sep 25, 2025

fix(Context-Menu): Fixing Context Menu for Table Chart with Html Content #33791

Are you sure you want to change the base?

fix(Context-Menu): Fixing Context Menu for Table Chart with Html Content #33791

Conversation

amaannawab923 commented Jun 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

Uh oh!

korbit-ai bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Review by Korbit AI

Korbit automatically attempts to detect when you fix issues in new commits.

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Jun 17, 2025

Uh oh!

github-actions bot commented Jun 17, 2025

Uh oh!

mistercrunch Aug 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mistercrunch commented Aug 12, 2025

Uh oh!

amaannawab923 commented Aug 14, 2025

Uh oh!

korbit-ai bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Review by Korbit AI

Korbit automatically attempts to detect when you fix issues in new commits.

Uh oh!

Uh oh!

mistercrunch Aug 14, 2025

Choose a reason for hiding this comment

Uh oh!

amaannawab923 Aug 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amaannawab923 Aug 14, 2025

Choose a reason for hiding this comment

Uh oh!

mistercrunch commented Aug 14, 2025

Uh oh!

amaannawab923 commented Aug 15, 2025

Uh oh!

mistercrunch left a comment

Choose a reason for hiding this comment

Uh oh!

mistercrunch commented Aug 15, 2025

Uh oh!

msyavuz commented Aug 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

amaannawab923 commented Jun 17, 2025 •

edited

Loading

korbit-ai bot left a comment •

edited

Loading

mistercrunch Aug 12, 2025 •

edited

Loading

korbit-ai bot left a comment •

edited

Loading

amaannawab923 Aug 14, 2025 •

edited

Loading

msyavuz commented Aug 15, 2025 •

edited

Loading