django-cms / djangocms-frontend

django CMS frontend is a plugin bundle for django CMS providing several components for the frontend, currently implemented with the popular Bootstrap 5 framework.
Other
43 stars 20 forks source link

fix: Bug fixes for figure, block quote, heading plugins #66

Closed fsbraun closed 1 year ago

fsbraun commented 1 year ago

This PR fixes #65 and #63. Also, it includes some changes required for the html linter and German translations

codecov[bot] commented 1 year ago

Codecov Report

Merging #66 (be8c243) into master (20af31d) will decrease coverage by 0.38%. The diff coverage is 54.83%.

@@            Coverage Diff             @@
##           master      #66      +/-   ##
==========================================
- Coverage   86.44%   86.06%   -0.39%     
==========================================
  Files         105      105              
  Lines        2774     2798      +24     
  Branches      542      548       +6     
==========================================
+ Hits         2398     2408      +10     
- Misses        285      294       +9     
- Partials       91       96       +5     
Impacted Files Coverage Δ
djangocms_frontend/contrib/content/cms_plugins.py 100.00% <ø> (ø)
djangocms_frontend/contrib/image/cms_plugins.py 92.30% <ø> (ø)
djangocms_frontend/contrib/image/forms.py 93.75% <ø> (-0.10%) :arrow_down:
...ms_frontend/contrib/image/frameworks/bootstrap5.py 66.66% <0.00%> (-8.34%) :arrow_down:
..._frontend/contrib/content/frameworks/bootstrap5.py 70.58% <50.00%> (-4.42%) :arrow_down:
djangocms_frontend/fields.py 80.30% <50.00%> (-1.45%) :arrow_down:
djangocms_frontend/templatetags/frontend.py 52.23% <50.00%> (-0.71%) :arrow_down:
djangocms_frontend/contrib/carousel/forms.py 100.00% <100.00%> (ø)
djangocms_frontend/contrib/content/forms.py 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

lgtm-com[bot] commented 1 year ago

This pull request introduces 1 alert when merging 96624557bd923e24940eb370b437e4383accd4cd into 8c78a76aedb627c7201a0ec3cf5fa786b739c59a - view on LGTM.com

new alerts:

lgtm-com[bot] commented 1 year ago

This pull request introduces 1 alert when merging e2f6085fff1c0c74e2d58a63b0daf20e0907fe64 into 35efb2ea91863565064bd4538f810df41b3a7f03 - view on LGTM.com

new alerts:

lgtm-com[bot] commented 1 year ago

This pull request introduces 1 alert when merging 7860d3cd1a6f7cb011d05c0e4805fba19b8f0ddf into 35efb2ea91863565064bd4538f810df41b3a7f03 - view on LGTM.com

new alerts:

lgtm-com[bot] commented 1 year ago

This pull request introduces 1 alert when merging be8c243684b21591146686686f7b4d46eceafbf4 into 20af31d9b53c41b80d8793c8af394d16b5ca3565 - view on LGTM.com

new alerts:

marksweb commented 1 year ago

@fsbraun The main thing I'm getting here is that we need to be careful of what data is stored in a text field while maybe allowing some markup?

If so, it might be worth adding a dependency on django-bleach and using it's form fields to sanitise the data; https://pypi.org/project/django-bleach/

fsbraun commented 1 year ago

Right now, djangocms_text_ckeditor sanitizes html. What I've added in this PR is that - if ckeditor is not installed - its HTMLField is replaced by a CharField and all html gets escaped. It's either ckeditor or no html right now. What do you think? (Maybe we should talk about djangocms_text_ckeditor's sanitation compared to bleach? I have no idea how they compare. What I know is that people have customized sanitation rules installed for djangocms_text_ckeditor.

marksweb commented 1 year ago

@fsbraun Yeah that's a good point. If someone is using djangocms_text_ckeditor and has something specific setup, then they come to use this then they'd not need to worry. There's a strong chance they're already using that app so keeping the dependency doesn't seem so bad.