Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Investigate display filters in decidim instances #12674

Open
alecslupu opened this issue Apr 5, 2024 · 0 comments
Open

Investigate display filters in decidim instances #12674

alecslupu opened this issue Apr 5, 2024 · 0 comments
Assignees

Comments

@alecslupu
Copy link
Contributor

alecslupu commented Apr 5, 2024

During the review of #12647, we have noticed there may be some edge cases in the listing of the filters where some elements are being escaped, and some that may not.

We need to investigate that the escaping is done properly in any decidim

Actually, this has been implemented by the redesign team, and this is why we never caught it before.

The method called directory_filter_categories_values defined in application_helper is being used with the following stackTrace:

  1. decidim-meetings/app/views/decidim/meetings/shared/_filters.html.erb:7
  2. decidim-core/app/views/decidim/shared/_filters.html.erb:35
  3. decidim-core/app/views/decidim/shared/filters/_check_boxes_tree.html.erb:10
  4. decidim-core/lib/decidim/filter_form_builder.rb:49
  5. decidim-core/app/views/decidim/shared/filters/_dropdown_label.html.erb:23
  6. decidim-core/app/views/decidim/shared/filters/_dropdown_label.html.erb:6

the method filter_text_for(translation, id: nil), is rendering a content_tag with a translation parameter that is automatically escaped by ERB.
in 0.27, we do not have filter_text_for therefore we need to handle by hand ( which i did here )

Looking at the tree helper, there is a test improvement that can be done for main elements + children so that we have everything escaped properly. This would be another PR against the develop, that needs to happen at some point, and i would not keep this PR any longer.

Originally posted by @alecslupu in #12647 (comment)

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

No branches or pull requests

1 participant