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

encode html entities #6

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

encode html entities #6

wants to merge 1 commit into from

Conversation

Tux
Copy link
Contributor

@Tux Tux commented Nov 8, 2018

Dit was een handmatige pick uit de wijzigingen die ik lokaal heb doorgevoerd. Mijn versie is 100% xhtml, maar dat heeft ook veel invloed op <header> die ik in deze PR niet wil aanpassen

Copy link
Owner

@sciurius sciurius left a comment

Choose a reason for hiding this comment

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

Are you sure about the entities encoding in Base.pm? A properly formatted title like "Calvin & Hobbes" will come out as "Calvin &amp; Hobbes".

@Tux
Copy link
Contributor Author

Tux commented Nov 8, 2018

http://tux.nl/Files/20181108095119.png ← do you want to check against my full repo?
FWIW - I run it on openSUSE 15.0 with Opera-developer on KDE/Plasma5

@sciurius
Copy link
Owner

sciurius commented Nov 9, 2018

The point is that the title and alt strings are extracted from HTML source, and should already be properly encoded.
A small survey from the comics titles in my current config (99 out of 128 comics enabled) reveals that 13 comics have properly encoded entities in their titles, while only 2 have an unescaped &. (The rest is plain text.) BTW, the unescaped & is handled graciously by FireFox, don't know about Opera-developer.
So I think it is wrong to apply encode_entities to these strings.

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

Successfully merging this pull request may close these issues.

2 participants