Re: Proposal: template-based widget rendering

[Carl Meyer]

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

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: Proposal: template-based widget rendering

[Bruno Renié]

Hi Carl,

On Tue, Mar 15, 2011 at 6:59 AM, Carl Meyer  wrote:
>
> 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.

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

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: Proposal: template-based widget rendering

[Carl Meyer]

Hi Bruno,

On 03/14/2011 06:33 PM, Bruno Renié wrote:
> 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  elements and people can switch
> to html4-style 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.

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

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Proposal: template-based widget rendering

[Bruno Renié]

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  elements and people can switch
to html4-style 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

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.