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

Fixed an issue where html tags where being displayed on news item titles #4475

Merged
merged 4 commits into from
Nov 5, 2024

Conversation

mauromsl
Copy link
Member

closes #4467

@mauromsl mauromsl requested review from ajrbyers and joemull October 31, 2024 17:34
@mauromsl mauromsl changed the base branch from master to r-v1.7.x October 31, 2024 17:34
@mauromsl mauromsl added this to the v1.7.0 milestone Oct 31, 2024
@@ -16,14 +16,14 @@ <h2 class="title">{% trans 'Latest' %} {{ journal_settings.news.news_title }} {%
<a href="{% url 'core_news_item' item.pk %}"><img
class="news-image"
src="{{ item.best_image_url }}"
alt="{{ item.title }}"/></a>
alt="{{ item.title|safe }}"/></a>
Copy link
Member

Choose a reason for hiding this comment

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

Should we use striptags here? Tags are not valid inside an alt attribute.

@@ -15,7 +15,7 @@
{% if item.large_image_file or request.journal and request.journal.default_large_image or request.press.default_carousel_image %}
<div class="card-image">
<img src="{{ item.best_image_url }}"
alt="{{ item.title }}"/>
alt="{{ item.title|safe }}"/>
Copy link
Member

Choose a reason for hiding this comment

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

Should we use striptags here? Tags are not valid inside an alt attribute.

@ajrbyers ajrbyers assigned mauromsl and unassigned ajrbyers Nov 4, 2024
@joemull joemull removed their request for review November 4, 2024 14:45
@ajrbyers ajrbyers self-requested a review November 5, 2024 09:58
Copy link
Contributor

@StephDriver StephDriver left a comment

Choose a reason for hiding this comment

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

The italics seem fine, but I don't think strip tags is sorting out the ampersand issue within the alt

image image image

@StephDriver StephDriver assigned mauromsl and unassigned StephDriver Nov 5, 2024
@mauromsl mauromsl requested a review from ajrbyers November 5, 2024 16:52
@mauromsl mauromsl assigned ajrbyers and unassigned mauromsl Nov 5, 2024
@mauromsl mauromsl merged commit 7e32d09 into r-v1.7.x Nov 5, 2024
@mauromsl mauromsl deleted the 4467-bugfix branch November 5, 2024 17:00
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.

Italics and ampersand in News title not rendering properly
3 participants