Proposal: template-based widget rendering

383 views
Skip to first unread message

Bruno Renié

unread,
Mar 14, 2011, 6:33:16 PM3/14/11
to django-d...@googlegroups.com
Hi django devs,

Although Django 1.3 is not released yet I'd like to take advantage of
the pycon sprints to discuss a proposal for 1.4: render form widgets
using Django templates instead of python code.

This approach is implemented in django-floppyforms [0] (I'm the
author): each widget gets a template_name argument, and the render
method of each widget renders this template. Floppyforms also provides
hooks to inject data in the template context, making it quite easy to
implement custom widgets: add some context data, define a custom
template and you're done.

This approach has a couple of advantages and drawbacks.

Advantages:

- Templates give you escaping for free, giving us better protection against XSS
- Template authors can alter widget rendering more easily
- This would also fix the "how do I specify my own doctype?" issue:
Django can provide XHTML-style <input/> elements and people can switch
to html4-style <input>s by providing their own set of templates.

Drawbacks:

- Template rendering is slower than string interpolation. There is a
benchmark in the floppyforms source tree, for a simple form the
rendering takes 9 milliseconds instead of 1.5 milliseconds. With
template caching enabled, it drops to 3 milliseconds. It still takes
no time but I can see it being an issue.
- We need to add a template loader to provide the default form
templates without asking the users to add 'django.forms' to their
INSTALLED_APPS.

Backwards-compatibility is not going to be a big issue: the default
templates can implement django's current behaviour.

We've been discussing it with Jannis and Carl and I'm trying to start
a broader discussion. A side-effect of such a change would be to make
the widgets API public and provide hooks for customization. In
floppyforms I've been trying to follow the same conventions as the
class-based views, using template_name and get_context_data as
extension points.

I'm happy to work on the patch and convert the admin widgets but I'd
like to hear people's voices about:

1) If such a change is desired, and how we can make it faster,
2) how the API should differ from floppyforms,
3) make sure no significant use case is forgotten.

This could be part of a broader change related to form rendering (e.g.
fieldsets, looping over fields, etc) but I haven't thought about this
enough to have a concrete proposal. For more information, see that
discussion that happened at Djangocon:

https://github.com/SmileyChris/django-forms/wiki/Django-form-API-ideas

Regards,
Bruno

[0] https://github.com/brutasse/django-floppyforms

Carl Meyer

unread,
Mar 15, 2011, 1:59:26 AM3/15/11
to django-d...@googlegroups.com
Hi Bruno,

As we've already discussed here at PyCon, I'm +1 on this change. It
makes forms far more flexible and usable by template authors, and I
think that will benefit almost all Django users. It's more consistent
with Django's philosophy that presentation issues should be accessible
to template designers who are not necessarily Python programmers.

There is the speed tradeoff, and I sure hate to do anything that we know
is slower. But I feel like this is a case of "do it right, then make it
fast" - and I just think this is clearly the right way to do it.
Probably the right way to make it fast is to make the template engine
fast (Alex Gaynor had some interesting proposals for that for last
year's GSoC, though it got dropped in favor of the no-SQL stuff). In
absolute terms, I think the speed difference is already small enough
that only a small fraction of users will be impacted by it, and they
would always have the option to replace the default widgets in their
forms with their own faster versions.

The auto-escaping is another significant advantage - it's quite easy to
forget to escape something that should be escaped in a custom widget (we
had such a bug in Django itself until the latest security release), and
I think it can only be good for security to discourage people ever
building HTML strings in Python code.

I already use floppyforms on all projects, and don't have any issues
with the widget API it provides.

> This could be part of a broader change related to form rendering (e.g.
> fieldsets, looping over fields, etc) but I haven't thought about this
> enough to have a concrete proposal. For more information, see that
> discussion that happened at Djangocon:
>
> https://github.com/SmileyChris/django-forms/wiki/Django-form-API-ideas

Although these proposals are related, and will involve some similar
changes (i.e. the need to be able to load default form templates), I
think the broader questions about form-rendering tags still need more
work, and converting the built-in widgets to use templates doesn't need
to be delayed while we hammer all of that out.

Carl

Bruno Renié

unread,
Mar 23, 2011, 8:56:26 AM3/23/11
to django-d...@googlegroups.com
Hi Carl,

With 1.3 out (yay!), I just created a ticket to track the progress on this:

http://code.djangoproject.com/ticket/15667

I'm working on a patch, I'll attach it to the ticket as soon as I'm
happy with the widgets code and the regression tests pass again.

I'm not sure, however, how django's backwards-compatibility applies in
this case. The widgets API has never been publicly documented but the
official documentation encourages people to look at the code and
create their own widgets based on it. Is it fine to touch some
internal methods like build_attrs()? There will be some minor
differences in the rendered code, with things like the order of
attributes (name='foo' id='bar' versus id='bar' name='foo') and
linebreaks (I have an EOL at the end of every template so each widget
gets a "\n" at its end).

Currently the base `Widget` class has a render() method that raises
NotImplemented. I think the rendering logic should happen here, the
base Widget class should accept a template_name attribute and a
render() method that renders the template. This way there is (almost)
no need to touch any widget's render() method since template_name and
get_context_data provide the needed extension points.

I'll post some updates here and on the ticket, in the meantime I'm
open to comments and suggestions.

Regards,
Bruno

Carl Meyer

unread,
Mar 23, 2011, 3:39:39 PM3/23/11
to django-d...@googlegroups.com
Hi Bruno,

On 03/23/2011 08:56 AM, Bruno Reni� wrote:
> I'm not sure, however, how django's backwards-compatibility applies in
> this case. The widgets API has never been publicly documented but the
> official documentation encourages people to look at the code and
> create their own widgets based on it. Is it fine to touch some
> internal methods like build_attrs()? There will be some minor
> differences in the rendered code, with things like the order of
> attributes (name='foo' id='bar' versus id='bar' name='foo') and
> linebreaks (I have an EOL at the end of every template so each widget
> gets a "\n" at its end).

Aesthetic differences in the rendered HTML (like attribute order or
linebreaks) is not a backwards-compatibility issue, IMO - though the
default templates should still be functionally identical to the current
output.

I think widgets should maintain the same external API (i.e. the way they
are used by Django's forms code should not change), or else follow the
normal deprecation path for any well-justified changes there. As far as
I can tell from a quick scan, the external API consists only of the
signatures of the __init__() and render() methods, and the .attrs
attribute (which is tweaked directly by BoundField). Any methods or
attributes that are a) not documented, and b) not used by Django's form
code outside of widgets.py, are IMO fair game for change (though
obviously still only with good reason).

The area where this is most likely to break user code is if people have
subclassed some of the more complex widgets and are depending on
internal implementation details in their subclass. My feeling here is
that this is just like any other use of an internal API; all bets are
off. If it turns out that this breaks a lot of code, the other option
would be to introduce the new widgets in a new namespace, make them the
default ones, and leave the old ones alone. I'd really like to not do
this unless we absolutely have to.

> Currently the base `Widget` class has a render() method that raises
> NotImplemented. I think the rendering logic should happen here, the
> base Widget class should accept a template_name attribute and a
> render() method that renders the template. This way there is (almost)
> no need to touch any widget's render() method since template_name and
> get_context_data provide the needed extension points.

This sounds reasonable to me.

> I'll post some updates here and on the ticket, in the meantime I'm
> open to comments and suggestions.

Great! Thanks for working on this.

Carl

Reply all
Reply to author
Forward
0 new messages