search

marksweb / django-bleach

Bleach is a Python module that takes any HTML input, and returns valid, sanitised HTML that contains only an allowed subset of HTML tags, attributes and styles. django-bleach is a Django app that makes using bleach extremely easy.
MIT License
148 stars 23 forks source link

Form field marks cleaned data as template safe #27

Closed MrkGrgsn closed 3 years ago

MrkGrgsn commented 3 years ago

Is your feature request related to a problem? Please describe. As I mentioned in the description of #25, when displaying a confirmation page at the end of a django-formtools wizard, the wizard renders the submitted form values to the page. When using the BleachField form field, these values have been bleached and thus are safe for rendering without escaping, however they are not marked as safe.

Describe the solution you'd like I propose that the form field marks values as safe after bleaching so they can be rendered without explicit intervention by calling mark_safe.

Describe alternatives you've considered The implementation has several options:

  1. Provide a child of the BleachField model field that performs the mark_safe.
  2. The existing BleachField form field always performs mark_safe after bleaching.
  3. The existing BleachField form field accepts an optional argument that directs it to perform mark_safe after bleaching.

Option 1 is backwards compatible but would require projects to override the form field class for model forms using Meta.field_classes, which will be fine once #25 is fixed.

Option 2 changes types, although I would expect this to have no impact, and would provide the benefits of the new behaviour to projects without local changes. However, projects that already escape the value, perhaps to render the markup, but I can't imagine what the use case for doing this would be so perhaps it's purely hypothetical.

Option 3 is also backwards compatible and would also require projects to modify models forms to get the new behaviour but the changes are more convoluted than for Option 1, because you would need to declare the whole form field on the model form instead of just providing a new class in Meta.field_classes.

I would prefer Option 2 because it suits my project but maybe there is some risk of breaking current usage with it, so the risk averse choice would be Option 1.

marksweb commented 3 years ago

Option 2 seems sensible to me @MrkGrgsn 👍