plotly / dash-core-components

OBSOLETE: now part of https://github.com/plotly/dash
https://dash.plotly.com
MIT License
271 stars 145 forks source link

[BUG] Breaking change in Markdown between Dash 1.7 and 1.8 #746

Open anders-kiaer opened 4 years ago

anders-kiaer commented 4 years ago

Describe your context

pip list | grep dash:

dash                                  1.8.0                          
dash-core-components                  1.7.0                          
dash-html-components                  1.0.2                          
dash-renderer                         1.2.3                          
dash-table                            4.6.0     

Describe the bug

On dash==1.7 the following works as expected:

import dash
import dash_core_components as dcc
import dash_html_components as html

app = dash.Dash()

app.layout = html.Div(
    children=[
        dcc.Markdown(
            """
<p>Some text above</p>
<img src="some_path.png">
<p>Some text below</p>
""",
            dangerously_allow_html=True,
        )
    ]
)

if __name__ == "__main__":
    app.run_server(debug=True)

When upgrading to dash==1.8 most/all of the Markdown content is not rendered.

After some debugging it turned out that going from dash==1.7 to dash==1.8, we need to have e.g. <img ... /> instead of <img ... > in the markdown. The closing slash is necessary only on dash==1.8, not dash==1.7.

Expected behavior

dash==1.8 accepting the same Markdown input as dash==1.7. Not using a closing / on img tags is also quite common, e.g. the examples on MDN web docs do not have a closing /.

Using the common Python html sanitizer bleach also removes the closing /.

>>> bleach.clean("<img />", tags=["img"])
'<img>'
alexcjohnson commented 4 years ago

Thanks @anders-kiaer ! (moved to the DCC repo) @Marc-Andre-Rivet related to https://github.com/plotly/dash-core-components/pull/711?

anders-kiaer commented 4 years ago

@alexcjohnson @Marc-Andre-Rivet Done some 🕵️‍♂️ and can confirm it is related to #711, more specifically the introduction/use of JsxParser here: https://github.com/plotly/dash-core-components/blob/638cffb559652b238e77e95c8029a2eab5fbb109/src/fragments/Markdown.react.js#L139-L143

Discussed with the maintainer of JsxParser (https://github.com/TroyAlford/react-jsx-parser/issues/17#issuecomment-619409813), and clarified that JsxParser needs any html to be "JSX compliant" also for void html tags, i.e. <somevoidtag ... /> instead <somevoidtag ...>.

Not sure where dash-core-components want to go with this issue. A minimal solution could perhaps be to mention in the dcc.Markdown component documentation that if html input is given, any void tags would need to be closed with /> and not only >.


For our use-case (where user input is sanitized before given to dcc.Markdown), we have used a "workaround" involving regex (which works ok since the user provided content is already sanitized/cleaned). A minimal example:

import re
import bleach

user_input = '<img src="some_src.png">Some text<hr>'

sanitized = bleach.clean(
    text=user_input, tags=["img", "hr", "br"], attributes={"img": ["src"]},
)

jsx_friendly = (
    re.sub("<img (.*?[^/])>", r"<img \1/>", sanitized)
    .replace("<br>", "<br/>")
    .replace("<hr>", "<hr/>")
) # == '<img src="some_src.png"/>Some text<hr/>'

# jsx_friendly is then given to dcc.Markdown...