100pah commented on a change in pull request #14434:
URL: https://github.com/apache/echarts/pull/14434#discussion_r591027667



##########
File path: src/component/tooltip/TooltipModel.ts
##########
@@ -46,6 +46,10 @@ export interface TooltipOption extends 
CommonTooltipOption<TopLevelFormatterPara
      * If show popup content
      */
     showContent?: boolean
+    /**
+     * escape html, prevent xss
+     */
+    escapeContent?: boolean

Review comment:
       I think we should neither add this option, nor `encodeHTML` internally.
   The reason is:
   + See the 
[comment](https://github.com/apache/echarts/issues/14429#issuecomment-794847914),
 users should do the encode themselves if the assemble html snippet in 
`tooltip.formatter`.
   + If echarts do the "encodeHTML" internally by this option `escapeContent`, 
echarts should not only do `encodeHTML` to part of the params but left other 
params not encoded. Otherwise users will be confused.
   But for some complex data structure like `params.data`, it's not an easy 
work to encode it.
   
   I think, the **principle** can be:
   
   If the html is assembled by echarts (e.g., when echarts give a default 
tooltip html formatter), echarts takes charge of the `encodeHTML`.
   If the html is assembled by users (e.g., when users set `tooltip.formatter` 
as a function and use `renderMode: 'html'`), users take charge of `encodeHTML`.
   
   
   




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to