CSRF middleware XHTML conformance

[Patryk Lorenowicz]

HTML code inserted by CSRF middleware should conform XHTML standard. Attributes should be double quoted, which is OK for HTML and XHTML.

[Luke Plant]

Single quotes for attributes are valid XHTML.

[andriijas]

Replying to lukeplant:

Single quotes for attributes are valid XHTML.

Lame excuse for closing. Double quoting should be used since
1) Consistency, Its used everywhere else in django
2) Single quoted html comes from the world of php idiocy ( print "<foo id='$bar'>"; )
3) Why on earth wrap a display none div around an input hidden field? If you are afraid of margins and paddings added by user style sheet, just put display none on the input.
4) Anyone closing this issue again without the patch being apply obviously don't care about consistency and clean markup, which does matter to some people. So lets not make something big of this. It's a quick job to review and apply the patch, though I understand it will take some minutes of someones precious spare time.

It's all about the semantics.

[Luke Plant]

Replying to andriijas:

Lame excuse for closing. Double quoting should be used since

1) Consistency, Its used everywhere else in django

This is not true.

2) Single quoted html comes from the world of php idiocy ( print "<foo id='$bar'>"; )

HTML did not inherit its use of single quotes from PHP practices. Equivalent things are done in the Django code base i.e. single quotes used to avoid problems with nesting quotations. See ​http://code.djangoproject.com/browser/django/tags/releases/1.1/django/views/debug.py#L489 for example.

3) Why on earth wrap a display none div around an input hidden field? If you are afraid of margins and paddings added by user style sheet, just put display none on the input.

Without the div, you have invalid HTML. The div has "display:none" to defensively protect against older browsers and their rendering quirks. (I'm not sure if there is a specific bug with any, but I'm not going to fire up a Windows VM and try X versions of Internet Explorer just to check, and it can't harm, and I've run across related bugs in the past).

4) Anyone closing this issue again without the patch being apply obviously don't care about consistency and clean markup, which does matter to some people. So lets not make something big of this. It's a quick job to review and apply the patch, though I understand it will take some minutes of someones precious spare time.

If I changed the quoting style, I'd could well have another bug filed from someone else who was relying on the previous style for some reason (e.g. if they had a regression test that checked the exact output of a page), and they would actually have a better case — why did I change something that wasn't broken? What am I going to put in the commit message - "Changed some valid HTML to some other valid HTML because andriijas told me so"?

And if I applied your patch as is, I'd have HTML errors immediately. Perhaps you haven't thought this through as well as you thought?

Yes, reviewing this patch did waste some minutes of my precious spare time. I don't begrudge them, but I do object to the attitude that says that you have a right to them, or the idea that if the review does not turn out how you want then obviously I have no valid reasons for doing turning you down.

It's all about the semantics.

The semantics are not affected, I have no idea what you mean here.

The policy in Django is that you don't re-open a bug that is closed by a core committer without discussion on django-devs. Please do not re-open.

[andriijas]

The policy in Django is that you don't re-open a bug that is closed by a core committer without discussion on django-devs. Please do not re-open.

Didnt know about that (the ticket didnt say it was closed by a "core comitter". Sorry.

And about the rest... I don't have the energy to argue about it. Though it's a shame there's no "html guidlines" or something to maintain consistency.

[Jacob]

Milestone 1.2 deleted