Opened 10 years ago

Closed 6 years ago

#3579 closed (invalid)

remove admin's reliance on TEMPLATE_STRING_IF_INVALID set to empty string

Reported by: Jeff Bauer <jbauer@…> Owned by: nobody
Component: Template system Version: master
Severity: Keywords: template admin
Cc: hv@…, Marinho Brandão Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

Refactor the way TEMPLATE_STRING_IF_INVALID works, rather than special-case it for the admin site.

It's often useful to have something like "BADDATA" assigned to TEMPLATE_STRING_IF_INVALID during development and testing, because it's easy to to scan for unforseen side effects, both visually and by a text search.

Attachments (5)

3579.patch.txt (1.0 KB) - added by buriy <burchik@…> 10 years ago.
First piece
all.patch (3.6 KB) - added by buriy <burchik@…> 10 years ago.
All fixes: two for action="{{ form_url }}", two for <th{{some.stuff}}>
all.2.patch (3.6 KB) - added by buriy <burchik@…> 10 years ago.
All fixes: two for action="{{ form_url }}", two for <th{{some.stuff}}>
all.3.patch (4.0 KB) - added by buriy <burchik@…> 10 years ago.
All fixes: two for action="{{ form_url }}", two for <th{{some.stuff}}>, one for <td{{some.stuff}}
3579_4.patch (2.5 KB) - added by Yuri Baburov 8 years ago.
Added template conditionals for sometimes-non-existing variables in admin, r8445.

Download all attachments as: .zip

Change History (27)

comment:1 Changed 10 years ago by Gary Wilson <gary.wilson@…>

Resolution: invalid
Status: newclosed

Not sure what you mean by special casing for the admin, could you please explain in more detail.

comment:2 Changed 10 years ago by Jeff Bauer <jbauer@…>

Resolution: invalid
Status: closedreopened

Sorry, I was referring to this thread: http://tinyurl.com/2nxu98

A better way to phrase the issue?

Refactor the way TEMPLATE_STRING_IF_INVALID works, without special-casing it for the admin site.

comment:3 Changed 10 years ago by Gary Wilson <gary.wilson@…>

Triage Stage: UnreviewedDesign decision needed

Could you please point out where the special-casing exists in the admin.

comment:4 Changed 10 years ago by Jeff Bauer <jbauer@…>

http://www.djangoproject.com/documentation/templates_python/#how-invalid-variables-are-handled

"Many templates, including those in the Admin site, rely upon the silence of the template system when a non-existent variable is encountered."

comment:5 Changed 10 years ago by Gary Wilson <gary.wilson@…>

Component: Core frameworkTemplate system
Triage Stage: Design decision neededAccepted

Ok, this was just pointed out to me. Feel free to provide suggestions and/or patches. Marking accepted as per Adrian's comments in the mailing list post.

comment:6 Changed 10 years ago by Gary Wilson <gary.wilson@…>

comment:7 Changed 10 years ago by Gary Wilson <gary.wilson@…>

Summary: decouple TEMPLATE_STRING_IF_INVALIDremove admin's reliance on TEMPLATE_STRING_IF_INVALID set to empty string

Changed 10 years ago by buriy <burchik@…>

Attachment: 3579.patch.txt added

First piece

Changed 10 years ago by buriy <burchik@…>

Attachment: all.patch added

All fixes: two for action="{{ form_url }}", two for <th{{some.stuff}}>

Changed 10 years ago by buriy <burchik@…>

Attachment: all.2.patch added

All fixes: two for action="{{ form_url }}", two for <th{{some.stuff}}>

Changed 10 years ago by buriy <burchik@…>

Attachment: all.3.patch added

All fixes: two for action="{{ form_url }}", two for <th{{some.stuff}}>, one for <td{{some.stuff}}

comment:8 Changed 10 years ago by buriy <burchik@…>

That's all. All code was looked for similar patterns.

comment:9 Changed 10 years ago by buriy <burchik@…>

related but not closed tickets:

http://code.djangoproject.com/ticket/3073 [hack for output, giving empty form_url]
http://code.djangoproject.com/ticket/2583 [no patch, only long bug description]

comment:10 Changed 9 years ago by buriy <burchik@…>

Has patch: set

committers, anyone? week passed, no reaction.

comment:11 Changed 9 years ago by Malcolm Tredinnick

Please demonstrate some patience. You may have noticed that this isn't the only ticket in Trac and it isn't the highest priority item at the moment. When we have time, it will be reviews and you'll get some feedback on what remains to be done.

comment:12 Changed 9 years ago by James Bennett

#5532 was a duplicate.

comment:13 Changed 9 years ago by Thomas Güttler

Cc: hv@… added

comment:14 Changed 8 years ago by Marinho Brandão

Cc: Marinho Brandão added

Changed 8 years ago by Yuri Baburov

Attachment: 3579_4.patch added

Added template conditionals for sometimes-non-existing variables in admin, r8445.

comment:15 Changed 8 years ago by Yuri Baburov

milestone: 1.0 maybe

comment:16 Changed 8 years ago by Malcolm Tredinnick

milestone: 1.0 maybepost-1.0

comment:17 Changed 8 years ago by Marinho Brandão

milestone: post-1.01.0 maybe

I'm not understanding why this ticket is being considered to post-1.0 if it is (I think) the cause to bugged add-user form and others strange behaviours everytime a variable isn't valid in the template.

For example: a simple /admin/auth/user/add/ is rendering this code:

<form action="&lt;font color=&#39;red&#39;&gt;form_url&lt;/font&gt;" method="post" id="user_form">
<div>

Shouldn't this enought reason to consider this ticket?

Maybe would make it safe string or disabling this "helper" (that isn't helping nothing).

comment:18 Changed 8 years ago by Malcolm Tredinnick

milestone: 1.0 maybepost-1.0

Making a change like this now (in feature freeze) would be destabilising, at a minimum.

Seriously, when a core maintainer triages something as post-1.0, please don't decide that you should change that. we have a mailing list if you want to ask for a reconsideration, but this isn't even borderline. It's an enhancement request that can easily be added afterwards.

comment:19 in reply to:  17 Changed 8 years ago by Yuri Baburov

Replying to marinho:

For example: a simple /admin/auth/user/add/ is rendering this code:

<form action="&lt;font color=&#39;red&#39;&gt;form_url&lt;/font&gt;" method="post" id="user_form">
<div>

When TEMPLATE_STRING_IF_INVALID is empty (by default), there's no such problem.
Also it is said in the docs that

While TEMPLATE_STRING_IF_INVALID can be a useful debugging tool,
it is a bad idea to turn it on as a ‘development default’.

Many templates, including those in the Admin site, rely upon the silence
of the template system when a non-existent variable is encountered.
If you assign a value other than '' to TEMPLATE_STRING_IF_INVALID, you
will experience rendering problems with these templates and sites.

at http://www.djangoproject.com/documentation/templates_python/#how-invalid-variables-are-handled

Admin interface templates are relying on this behavior now in few last places. Patch for them is attached.

comment:20 Changed 8 years ago by Marinho Brandão

Thank you Yuri.

Sorry, Malcolm, you both are right :)

comment:21 Changed 8 years ago by (none)

milestone: post-1.0

Milestone post-1.0 deleted

comment:22 Changed 6 years ago by Luke Plant

Resolution: invalid
Status: reopenedclosed

I think it is now too late to put this genie back in the bottle, and it isn't worth the effort trying to fix the admin or other contrib apps. Given that #3073 was marked as invalid I'm going to do the same here. Please bring to django-devs if you think it is worth the effort to fix this.

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