Opened 9 years ago

Closed 9 years ago

Last modified 8 years ago

#25135 closed Cleanup/optimization (fixed)

Deprecate admin list_display allow_tags

Reported by: Jaap Roes Owned by: Ola Sitarska
Component: contrib.admin Version: dev
Severity: Normal Keywords:
Cc: 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

I've noticed that setting allow_tags on a list_display function is not necessary if it already returns a safe string (by using mark_safe or format_html).

The docs on allow_tags mention:

If the string given is a method of the model, ModelAdmin or a callable, Django will HTML-escape the output by default. If you’d rather not escape the output of the method, give the method an allow_tags attribute whose value is True. However, to avoid an XSS vulnerability, you should use format_html() to escape user-provided inputs.

To push people to actually do that, deprecating allow_tags and pointing to format_html/mark_safe could be a good thing.

Change History (10)

comment:1 by Tim Graham, 9 years ago

Triage Stage: UnreviewedAccepted

Looking at the code, I think it could be a bit tricky, but the idea sounds good.

comment:2 by Jaap Roes, 9 years ago

Owner: changed from nobody to Jaap Roes
Status: newassigned

Created a pull request with my initial attempt. Django tests all pass, but it might just be that allow_tags is not tested that well.

Having a hard time figuring out where to add tests though...

comment:3 by Ola Sitarska, 9 years ago

Has patch: set
Owner: changed from Jaap Roes to Ola Sitarska

comment:4 by Tim Graham, 9 years ago

Needs tests: set

Waiting on one more test as noted on the pull request.

comment:5 by Ola Sitarska, 9 years ago

Needs tests: unset
Version 0, edited 9 years ago by Ola Sitarska (next)

comment:6 by Tim Graham, 9 years ago

Triage Stage: AcceptedReady for checkin

Pending some cosmetic tweaks.

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

Resolution: fixed
Status: assignedclosed

In f2f8972:

Fixed #25135 -- Deprecated the contrib.admin allow_tags attribute.

Thanks Jaap Roes for the idea and initial patch.

comment:8 by Jaap Roes, 9 years ago

Thank you olasitarska for fixing this up!

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

In 00adec6:

Refs #25135 -- Corrected the timeline section of allow_tags deprecation.

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

In d67a46e1:

Refs #25135 -- Removed support for the contrib.admin allow_tags attribute.

Per deprecation timeline.

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