korbit-ai[bot] commented on code in PR #33791:
URL: https://github.com/apache/superset/pull/33791#discussion_r2152589116
##########
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();
+};
Review Comment:
### Missing security and environment documentation <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The stripHtmlAndGetValue function's security implications and browser
dependency are not documented.
###### Why this matters
The function uses innerHTML which has security implications, and it requires
a browser environment. Without documentation, developers may not be aware of
these important constraints.
###### Suggested change ∙ *Feature Preview*
/**
* Strips HTML tags from a string and returns the text content.
* @warning Only use with trusted HTML content as it uses innerHTML
* @warning Requires browser environment (document object)
*/
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/8e46509b-a402-41ec-83d0-25ac21962751/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/8e46509b-a402-41ec-83d0-25ac21962751?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/8e46509b-a402-41ec-83d0-25ac21962751?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/8e46509b-a402-41ec-83d0-25ac21962751?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/8e46509b-a402-41ec-83d0-25ac21962751)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:c7991bc1-0311-46bc-b6a9-959598361703 -->
[](c7991bc1-0311-46bc-b6a9-959598361703)
##########
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('>');
+};
Review Comment:
### Unreliable HTML Detection <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The HTML detection logic is too simplistic and could lead to false positives
or negatives in identifying HTML content.
###### Why this matters
The current implementation might incorrectly identify strings containing '<'
and '>' as HTML even when they're not, or miss malformed HTML content.
###### Suggested change ∙ *Feature Preview*
Use a more robust HTML detection method:
```typescript
const isHtml = (value: any): boolean => {
if (typeof value !== 'string') {
return false;
}
const doc = new DOMParser().parseFromString(value, 'text/html');
return Array.from(doc.body.childNodes).some(node => node.nodeType === 1);
}
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/7c51800b-8f76-4fd3-bdad-07e908d1a37c/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/7c51800b-8f76-4fd3-bdad-07e908d1a37c?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/7c51800b-8f76-4fd3-bdad-07e908d1a37c?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/7c51800b-8f76-4fd3-bdad-07e908d1a37c?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/7c51800b-8f76-4fd3-bdad-07e908d1a37c)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:782aa2a6-7469-4e57-a9af-68edbae7dec9 -->
[](782aa2a6-7469-4e57-a9af-68edbae7dec9)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
