Skip to content
This repository has been archived by the owner on Nov 5, 2024. It is now read-only.

Form field marks cleaned data as template safe #27

Closed
MrkGrgsn opened this issue Sep 7, 2021 · 1 comment · Fixed by #30
Closed

Form field marks cleaned data as template safe #27

MrkGrgsn opened this issue Sep 7, 2021 · 1 comment · Fixed by #30

Comments

@MrkGrgsn
Copy link
Contributor

MrkGrgsn commented Sep 7, 2021

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
Copy link
Owner

marksweb commented Sep 7, 2021

Option 2 seems sensible to me @MrkGrgsn 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants