Skip to content

Conversation

@rsdeus
Copy link
Member

@rsdeus rsdeus commented Nov 26, 2024

🎩 What? Why?

Breadcrumb for mobile is escaped without html_safe, and for desktop, it is displayed without using the escape method.

📌 Related Issues

https://git.octree.ch/decidim/decidim-nightly/-/issues/68#note_46918

Testing

Create an proposal named, for example: "Partage d'idées"

📷 Screenshots

image

♥️ Thank you!

@rsdeus rsdeus requested a review from froger November 26, 2024 11:00
@rsdeus rsdeus self-assigned this Nov 26, 2024
@rsdeus rsdeus changed the title Fix/mobile breadcrumb html safe fix: mobile breadcrumb html safe Nov 26, 2024
@rsdeus rsdeus changed the title fix: mobile breadcrumb html safe Fix mobile breadcrumb html safe Nov 26, 2024
<span>
<% breadcrumb_items.last(2).each_with_index do |item, i| %>
<% item_label = decidim_escape_translated(item[:label]) %>
<% item_label = decidim_escape_translated(item[:label]).html_safe %>
Copy link
Member

@froger froger Nov 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rsdeus decidim_escape_translated, shouldn't always be html_safe? or add a param to the function.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see the function applied in numerous places: https://github.com/search?q=repo%3Adecidim%2Fdecidim+decidim_escape_translated&type=code&p=1

And I see no reference to html_safe... meaning the issue is potentially everywhere?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but the problems do not arise everywhere. However, I suggested returning the output of decidim_html_escape with html_safe to ensure better encoding.
87bd217

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pull request does not contain a valid label. Please add one of the following labels: ['type: feature', 'type: change', 'type: fix', 'type: removal', 'target: developer-experience', 'type: internal']

@rsdeus rsdeus closed this Jan 20, 2025
@rsdeus rsdeus force-pushed the fix/mobile_breadcrumb_html_safe branch from 6e5b605 to f4145fa Compare January 20, 2025 13:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants