twaddington / django-gravatar

Essential Gravatar support for Django. Features helper methods, templatetags and a full test suite!
MIT License
151 stars 33 forks source link

Encode URL to pass XHTML validation tests #4

Closed phrac closed 12 years ago

phrac commented 12 years ago

Two small changes to pass XHTML 1.0 STRICT validation with gravatar URL.

twaddington commented 12 years ago

Hey Derek, thanks for the pull request. Can you provide an example of a URL that is breaking validation for you? Are you having trouble with unicode in the default image url?

phrac commented 12 years ago

If I run it through the W3C validator (http://validator.w3.org/) using an XHTML 1.0 STRICT doc type I get several errors because of the ampersands in the query string are not encoded.

The specific error is: "Line 35, Column 129: cannot generate system identifier for general entity "r" …8cd98f00b204e9800998ecf8427e.jpg?s=40&r=g&d=mm" width="40" height="40" alt="" … ✉ An entity reference was found in the document, but there is no reference by that name defined. Often this is caused by misspelling the reference name, unencoded ampersands, or by leaving off the trailing semicolon (;). The most common cause of this error is unencoded ampersands in URLs as described by the WDG in "Ampersands in URLs".

Entity references start with an ampersand (&) and end with a semicolon (;). If you want to use a literal ampersand in your document you must encode it as "&" (even inside URLs!). Be careful to end entity references with a semicolon or your entity reference may get interpreted in connection with the following text. Also keep in mind that named entity references are case-sensitive; &Aelig; and æ are different characters.

If this error appears in some markup generated by PHP's session handling code, this article has explanations and solutions to your problem.

Note that in most documents, errors related to entity references will trigger up to 5 separate messages from the Validator. Usually these will all disappear when the original problem is fixed."

By encoding the ampersands, it makes a valid URL (per XHTML 1.0 STRICT doctype)

twaddington commented 12 years ago

Good catch, ampersands should definitely be escaped when the Gravatar URL is used in a template. While your solution looks okay, I would rather do the escaping in the template tags in gravatar.py. It's conceivable that someone would want an unescaped version of the URL in case they wanted to make a direct request to get the gravatar.

Please edit gravatar.py and use the escape method: from django.utils.html import escape. If that solves your problem please resubmit the pull request.

Thanks!

phrac commented 12 years ago

I agree, your way is much better. I will close this pull request and re-open a new one.