[Django] #25135: Deprecate admin list_display allow_tags

26 views
Skip to first unread message

Django

unread,
Jul 17, 2015, 8:09:32 AM7/17/15
to django-...@googlegroups.com
#25135: Deprecate admin list_display allow_tags
--------------------------------------+--------------------
Reporter: jaap3 | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.admin | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+--------------------
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.

--
Ticket URL: <https://code.djangoproject.com/ticket/25135>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Jul 17, 2015, 9:50:50 AM7/17/15
to django-...@googlegroups.com
#25135: Deprecate admin list_display allow_tags
--------------------------------------+------------------------------------

Reporter: jaap3 | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Changes (by timgraham):

* needs_better_patch: => 0
* needs_docs: => 0
* needs_tests: => 0
* stage: Unreviewed => Accepted


Comment:

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

--
Ticket URL: <https://code.djangoproject.com/ticket/25135#comment:1>

Django

unread,
Jul 20, 2015, 11:06:43 AM7/20/15
to django-...@googlegroups.com
#25135: Deprecate admin list_display allow_tags
--------------------------------------+------------------------------------
Reporter: jaap3 | Owner: jaap3
Type: Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: master

Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Changes (by jaap3):

* owner: nobody => jaap3
* status: new => assigned


Comment:

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...

--
Ticket URL: <https://code.djangoproject.com/ticket/25135#comment:2>

Django

unread,
Aug 31, 2015, 1:42:51 PM8/31/15
to django-...@googlegroups.com
#25135: Deprecate admin list_display allow_tags
-------------------------------------+-------------------------------------
Reporter: jaap3 | Owner:
Type: | olasitarska
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: master

Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by olasitarska):

* owner: jaap3 => olasitarska
* has_patch: 0 => 1


--
Ticket URL: <https://code.djangoproject.com/ticket/25135#comment:3>

Django

unread,
Sep 5, 2015, 9:38:18 AM9/5/15
to django-...@googlegroups.com
#25135: Deprecate admin list_display allow_tags
-------------------------------------+-------------------------------------
Reporter: jaap3 | Owner:
Type: | olasitarska
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: master

Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by timgraham):

* needs_tests: 0 => 1


Comment:

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

--
Ticket URL: <https://code.djangoproject.com/ticket/25135#comment:4>

Django

unread,
Sep 7, 2015, 5:30:39 PM9/7/15
to django-...@googlegroups.com
#25135: Deprecate admin list_display allow_tags
-------------------------------------+-------------------------------------
Reporter: jaap3 | Owner:
Type: | olasitarska
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: master

Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by olasitarska):

* needs_tests: 1 => 0


--
Ticket URL: <https://code.djangoproject.com/ticket/25135#comment:5>

Django

unread,
Sep 7, 2015, 7:38:54 PM9/7/15
to django-...@googlegroups.com
#25135: Deprecate admin list_display allow_tags
-------------------------------------+-------------------------------------
Reporter: jaap3 | Owner:
Type: | olasitarska
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by timgraham):

* stage: Accepted => Ready for checkin


Comment:

Pending some cosmetic tweaks.

--
Ticket URL: <https://code.djangoproject.com/ticket/25135#comment:6>

Django

unread,
Sep 8, 2015, 7:14:22 PM9/8/15
to django-...@googlegroups.com
#25135: Deprecate admin list_display allow_tags
-------------------------------------+-------------------------------------
Reporter: jaap3 | Owner:
Type: | olasitarska
Cleanup/optimization | Status: closed
Component: contrib.admin | Version: master
Severity: Normal | Resolution: fixed

Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Tim Graham <timograham@…>):

* status: assigned => closed
* resolution: => fixed


Comment:

In [changeset:"f2f8972def26cea2b0e8dbe763e11436d194e3d4" f2f8972]:
{{{
#!CommitTicketReference repository=""
revision="f2f8972def26cea2b0e8dbe763e11436d194e3d4"
Fixed #25135 -- Deprecated the contrib.admin allow_tags attribute.

Thanks Jaap Roes for the idea and initial patch.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/25135#comment:7>

Django

unread,
Sep 9, 2015, 3:48:42 PM9/9/15
to django-...@googlegroups.com
#25135: Deprecate admin list_display allow_tags
-------------------------------------+-------------------------------------
Reporter: jaap3 | Owner:
Type: | olasitarska
Cleanup/optimization | Status: closed
Component: contrib.admin | Version: master

Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by jaap3):

Thank you olasitarska for fixing this up!

--
Ticket URL: <https://code.djangoproject.com/ticket/25135#comment:8>

Django

unread,
Sep 19, 2015, 8:37:35 PM9/19/15
to django-...@googlegroups.com
#25135: Deprecate admin list_display allow_tags
-------------------------------------+-------------------------------------
Reporter: jaap3 | Owner:
Type: | olasitarska
Cleanup/optimization | Status: closed
Component: contrib.admin | Version: master

Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Tim Graham <timograham@…>):

In [changeset:"00adec6d5f266469bc62e7351d8e6b641872b47a" 00adec6]:
{{{
#!CommitTicketReference repository=""
revision="00adec6d5f266469bc62e7351d8e6b641872b47a"
Refs #25135 -- Corrected the timeline section of allow_tags deprecation.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/25135#comment:9>

Django

unread,
Jan 17, 2017, 10:09:51 PM1/17/17
to django-...@googlegroups.com
#25135: Deprecate admin list_display allow_tags
-------------------------------------+-------------------------------------
Reporter: Jaap Roes | Owner: Ola
Type: | Sitarska
Cleanup/optimization | Status: closed
Component: contrib.admin | Version: master

Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Tim Graham <timograham@…>):

In [changeset:"d67a46e10459858b681176a3e1f8c6bca39d2ac7" d67a46e1]:
{{{
#!CommitTicketReference repository=""
revision="d67a46e10459858b681176a3e1f8c6bca39d2ac7"
Refs #25135 -- Removed support for the contrib.admin allow_tags attribute.

Per deprecation timeline.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/25135#comment:10>

Reply all
Reply to author
Forward
0 new messages