Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#19577 closed Bug (fixed)

admin doc may encourage bad practices

Reported by: foo@… Owned by: nobody
Component: Documentation Version: 1.4
Severity: Normal Keywords:
Cc: Florian Apolloner Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

On https://docs.djangoproject.com/en/1.4/ref/contrib/admin/ two places example code demonstrate allow_tags=True in order to return HTML fragments containing <span> tags. Inside these tags data (self.first_name and self.last_name) is not escaped/quoted.

As such example code often is copy/pasted it probably should reflect "best practices"?

Attachments (2)

19577.diff (2.8 KB ) - added by Tim Graham 12 years ago.
19577.2.diff (5.2 KB ) - added by Tim Graham 12 years ago.

Download all attachments as: .zip

Change History (11)

comment:1 by Florian Apolloner, 12 years ago

Cc: Florian Apolloner added
Triage Stage: UnreviewedAccepted

Yes, this should definitely use format_html (https://docs.djangoproject.com/en/dev/ref/utils/#django.utils.html.format_html). Do you think you can wipe up a patch for that?

by Tim Graham, 12 years ago

Attachment: 19577.diff added

comment:2 by Tim Graham, 12 years ago

Has patch: set

comment:3 by Florian Apolloner, 12 years ago

The indendation is wrong at https://code.djangoproject.com/attachment/ticket/19577/19577.diff#L45 and you need to manually call safe on the arguments of the join https://code.djangoproject.com/attachment/ticket/19577/19577.diff#L63 -- otherwise it looks good to me.

Version 1, edited 12 years ago by Florian Apolloner (previous) (next) (diff)

comment:4 by Florian Apolloner, 12 years ago

Patch needs improvement: set

by Tim Graham, 12 years ago

Attachment: 19577.2.diff added

comment:5 by Tim Graham, 12 years ago

Patch needs improvement: unset

Thanks for taking a look. I don't think the example is meant to assume that get_full_address() returns a safe string, but maybe I interpreted it differently. I added a comment to try to explain what I thought the intent of the example was in addition to the other fixes you mentioned (plus documenting format_html_join).

comment:6 by Florian Apolloner, 12 years ago

Triage Stage: AcceptedReady for checkin

Thanks, looks good now!

comment:7 by Tim Graham <timograham@…>, 12 years ago

Resolution: fixed
Status: newclosed

In eafc0364764ba12babd76194d8e1f78b876471ec:

Fixed #19577 - Added HTML escaping to admin examples.

Thanks foo@ for the report and Florian Apolloner for the review.

comment:8 by Tim Graham <timograham@…>, 12 years ago

In 42fcfcaa529dac1fe3066797d7e4ab7aa6f6cdf3:

[1.5.x] Fixed #19577 - Added HTML escaping to admin examples.

Thanks foo@ for the report and Florian Apolloner for the review.

Backport of eafc036476 from master

comment:9 by Tim Graham <timograham@…>, 12 years ago

In 42fcfcaa529dac1fe3066797d7e4ab7aa6f6cdf3:

[1.5.x] Fixed #19577 - Added HTML escaping to admin examples.

Thanks foo@ for the report and Florian Apolloner for the review.

Backport of eafc036476 from master

Note: See TracTickets for help on using tickets.
Back to Top