[Django] #22950: Subclassing choice select widgets is a nightmare

Django

unread,

Jul 3, 2014, 8:33:05 AM7/3/14

to django-...@googlegroups.com

#22950: Subclassing choice select widgets is a nightmare
----------------------------------------------+----------------------
Reporter: Patrick Robertson <django@…> | Owner: nobody
Type: Uncategorized | Status: new
Component: Uncategorized | Version: 1.7-rc-1
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------------------+----------------------
So say I want to change the HTML of a RadioSelect widget from using `<ul>`
to something like:

{{{
<div>
<label>label</label>
<input>
</div>
}}}

It sounds pretty simple right? But it looks like I need all of this code,
which still doesn't work:

{{{
class BSChoiceFieldRenderer(widgets.ChoiceFieldRenderer):
def render(self):
"""
Outputs a <ul> for this set of choice fields.
If an id was given to the field, it is applied to the <ul> (each
item in the list will get an id of `$id_$i`).
"""
id_ = self.attrs.get('id', None)
start_tag = format_html('<div id="{0}">', id_) if id_ else '<div>'
output = [start_tag]
for i, choice in enumerate(self.choices):
choice_value, choice_label = choice
if isinstance(choice_label, (tuple, list)):
attrs_plus = self.attrs.copy()
if id_:
attrs_plus['id'] += '_{0}'.format(i)
sub_ul_renderer = ChoiceFieldRenderer(name=self.name,
value=self.value,
attrs=attrs_plus,
choices=choice_label)
sub_ul_renderer.choice_input_class =
self.choice_input_class
output.append(format_html('<div>{0}{1}</div>',
choice_value,
sub_ul_renderer.render()))
else:
w = self.choice_input_class(self.name, self.value,
self.attrs.copy(), choice, i)
output.append(format_html('<div>{0}</div>',
force_text(w)))
output.append('</div>')
return mark_safe('\n'.join(output))

class BSRadioFieldRenderer(BSChoiceFieldRenderer):
choice_input_class = widgets.RadioChoiceInput

# Subclass of the RadioSelect widget
class BSRadioSelect(widgets.RadioSelect):
renderer = BSRadioFieldRenderer

...
forms.py

something = forms.ChoiceField(choices=choices, widget=BSRadioSelect())
}}}

If I try and put the `render()` method in the `BSRadioFieldRenderer`
subclass (so as to avoid having to subclass `ChoiceFieldRenderer`) I get
an error about NoneType not being callable.
With the code as it stands here, the widget isn't actually rendered, just
escaped HTML is printed

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

Django

unread,

Jul 3, 2014, 1:10:28 PM7/3/14

to django-...@googlegroups.com

#22950: Subclassing choice select widgets is a nightmare

Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by timo):

* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0

Comment:

Although I have not looked into the ticket in detail, I guess a solution
might be #15667 - Implement template-based widget rendering.

If not, do you have other ideas? I guess you probably didn't mean it this
way, but your post comes off as a complaint without suggesting any ways to
improve the situation which can be discouraging for people who spend a lot
of time working on Django. Consider avoiding words like "nightmare",
thanks.

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

Django

unread,

Jul 3, 2014, 2:14:04 PM7/3/14

to django-...@googlegroups.com

#22950: Subclassing choice select widgets is a nightmare

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------

Comment (by claudep):

I tried to rewrite the rendering code to factor out the HTML tags. Could
you please see if that patch would allow you to more easily customize the
produced HTML (overwriting only `ChoiceFieldRenderer.outer_html` and
`ChoiceFieldRenderer.inner_html` in your subclass)?

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

Django

unread,

Jul 3, 2014, 8:55:32 PM7/3/14

to django-...@googlegroups.com

#22950: Subclassing choice select widgets is a nightmare

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------

Comment (by Patrick Robertson <django@…>):

Hi claudep,

Yes, I apologise - my initial submission probably wasn't the most helpful
for you guys. It's amazing to see you've already created a patch, so
quickly!
Your patch is most certainly easier (there is now no need to subclass
`ChoiceFieldRenderer`, I can put the two instance vars in my
`RadioFieldRenderer` subclass.

Perhaps what I wasn't more clear about is that, for some reason - when I
subclass either `ChoiceFieldRenderer` or `RadioFieldRenderer`, then the
outputted HTML ( with `{{ form.field }}` in my template is always escaped.

So even if I just do:

{{{
#renderer subclass
class BSRadioFieldRenderer(widgets.RadioFieldRenderer):
outer_html = '<div{id_attr}>{content}</div>'
inner_html = '<div>{choice_value}{sub_widgets}</div>'

# Subclass of the RadioSelect widget
class BSRadioSelect(widgets.RadioSelect):
renderer = BSRadioFieldRenderer
}}}

the outputted HTML is escaped. If, I now simply replace the `outer_html`
and `inner_html` vars with `pass`, everything works as expected (except
`uls` are used of course)

So perhaps my gripe is that using any non-escaped HTML anywhere in the
subclasses causes rendering to break (although I don't know much about
rendering, so I could be wrong as to the reason why)

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

Django

unread,

Jul 3, 2014, 8:57:04 PM7/3/14

to django-...@googlegroups.com

#22950: Subclassing choice select widgets is a nightmare

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------

Comment (by Patrick Robertson <django@…>):

P.S. using `{{ form.field|safe }}` in my template works, but that just
doesn't seem right.

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

Django

unread,

Jul 4, 2014, 3:06:26 AM7/4/14

to django-...@googlegroups.com

#22950: Subclassing choice select widgets is a nightmare

-------------------------------------+-------------------------------------
Reporter: Patrick Robertson | Owner: nobody
<django@…> | Status: new

* component: Uncategorized => Forms
* needs_tests: 0 => 1
* version: 1.7-rc-1 => master
* has_patch: 0 => 1
* type: Uncategorized => Cleanup/optimization
* stage: Unreviewed => Accepted

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

Django

unread,

Jul 4, 2014, 8:11:41 AM7/4/14

to django-...@googlegroups.com

#22950: Subclassing choice select widgets is a nightmare

Needs tests: 0 |

Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by claudep):

* needs_tests: 1 => 0

Comment:

I've created a pull request including a test:
https://github.com/django/django/pull/2878
However I wasn't able to reproduce your escaping issue. Indeed, you
shouldn't have to append the `|safe` filter. Maybe a sample project might
help debugging your issue.

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

Django

unread,

Jul 4, 2014, 9:07:58 AM7/4/14

to django-...@googlegroups.com

#22950: Subclassing choice select widgets is a nightmare

Comment (by Patrick Robertson <django@…>):

OK - I've narrowed down the issue with rendering escaped HTML. It only
occurs when actually rendering a single form field (with `{{
form.field_name }]` in the template)

See my sample project at:

https://github.com/pjrobertson/django_22950

I think that once this issue is pinpointed, some tests should be created
to ensure that the output rendered from `{{ form }}` and `{% for field in
form %} {{ field }} {% endfor %}` (or whatever) are the same

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

Django

unread,

Jul 4, 2014, 2:20:42 PM7/4/14

to django-...@googlegroups.com

#22950: Subclassing choice select widgets is a nightmare

Comment (by claudep):

Thanks for the sample project, it was useful to debug this weird issue,
being that rendering returned a bytestring which was then converted to
Unicode and therefore lost its safe escaping status.
I've updated the pull request.

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

Django

unread,

Jul 4, 2014, 8:36:05 PM7/4/14

to django-...@googlegroups.com

#22950: Subclassing choice select widgets is a nightmare

Comment (by django@…):

Great, it looks good!

I guess I'm probably asking too much at this point... but is there any
chance the rendering bug can be in Django 1.7rc2? I think it was the main
cause of my 'nightmare' issues, and isn't specific to this pull request (I
originally came across it when subclassing the `render()` method)

Thanks for all your hard work

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

Django

unread,

Jul 5, 2014, 5:43:18 AM7/5/14

to django-...@googlegroups.com

#22950: Subclassing choice select widgets is a nightmare

Comment (by Claude Paroz <claude@…>):

In [changeset:"9209049211acbe1a53c3dc409dd5fe26edf21634"]:
{{{
#!CommitTicketReference repository=""
revision="9209049211acbe1a53c3dc409dd5fe26edf21634"
Ensured bound field renders as unicode safe data

Refs #22950.
}}}

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

Django

unread,

Jul 5, 2014, 6:02:23 AM7/5/14

to django-...@googlegroups.com

#22950: Subclassing choice select widgets is a nightmare

Comment (by claudep):

I just pushed the rendering issue to master: [9209049211acbe1]
I'm rather confident that it is a harmless change for the 1.7 branch. I'd
just like a second opinion from a core dev.

--
Ticket URL: <https://code.djangoproject.com/ticket/22950#comment:11>

Django

unread,

Jul 5, 2014, 6:47:53 AM7/5/14

to django-...@googlegroups.com

#22950: Subclassing choice select widgets is a nightmare

Comment (by django@…):

Now you have done more than you should have - thanks claudep, the Django
dev team is awesome!

--
Ticket URL: <https://code.djangoproject.com/ticket/22950#comment:12>

Django

unread,

Jul 5, 2014, 7:15:06 AM7/5/14

to django-...@googlegroups.com

#22950: Subclassing choice select widgets should be easier

Comment (by aaugustin):

I've changed the title to something less link-baity, I hope you don't mind
;-)

--
Ticket URL: <https://code.djangoproject.com/ticket/22950#comment:13>

Django

unread,

Jul 5, 2014, 7:16:01 AM7/5/14

to django-...@googlegroups.com

#22950: Subclassing choice select widgets should be easier
-------------------------------------+-------------------------------------
Reporter: Patrick Robertson | Owner: nobody
<django@…> | Status: new
Type: | Version: master
Cleanup/optimization | Resolution:
Component: Forms | Triage Stage: Accepted
Severity: Normal | Needs documentation: 0
Keywords: | Patch needs improvement: 0
Has patch: 1 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------

Comment (by aaugustin):

claudep: this looks safe for backporting even at this stage.

--
Ticket URL: <https://code.djangoproject.com/ticket/22950#comment:14>

Django

unread,

Jul 5, 2014, 7:36:46 AM7/5/14

to django-...@googlegroups.com

#22950: Subclassing choice select widgets should be easier
-------------------------------------+-------------------------------------
Reporter: Patrick Robertson | Owner: nobody
<django@…> | Status: new
Type: | Version: master
Cleanup/optimization | Resolution:
Component: Forms | Triage Stage: Accepted
Severity: Normal | Needs documentation: 0
Keywords: | Patch needs improvement: 0
Has patch: 1 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------

Comment (by Claude Paroz <claude@…>):

In [changeset:"83a185a3f7d07af295880ed02efba4af4c86a204"]:
{{{
#!CommitTicketReference repository=""
revision="83a185a3f7d07af295880ed02efba4af4c86a204"
[1.7.x] Ensured bound field renders as unicode safe data

Refs #22950.
Backport of 920904921 from master.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/22950#comment:15>

Django

unread,

Jul 5, 2014, 7:37:44 AM7/5/14

to django-...@googlegroups.com

#22950: Subclassing choice select widgets should be easier
-------------------------------------+-------------------------------------
Reporter: Patrick Robertson | Owner: nobody

<django@…> | Status: closed
Type: | Version: master
Cleanup/optimization | Resolution: fixed

Changes (by Claude Paroz <claude@…>):

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

Comment:

In [changeset:"dd9a23d5cfb44cb6f150610c267d0de1d963b849"]:
{{{
#!CommitTicketReference repository=""
revision="dd9a23d5cfb44cb6f150610c267d0de1d963b849"
Fixed #22950 -- Eased markup customization for choice field rendering