Subclassing choice select widgets should be easier

[Patrick Robertson <django@…>]

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

[Tim Graham]

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.

[Claude Paroz]

Proof-of-concept patch

[Claude Paroz]

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)?

[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)

[Patrick Robertson <django@…>]

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

[Claude Paroz]

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.

[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

[Claude Paroz]

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.

[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

[Claude Paroz <claude@…>]

In 9209049211acbe1a53c3dc409dd5fe26edf21634:

Ensured bound field renders as unicode safe data

Refs #22950.

[Claude Paroz]

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.

[django@…]

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

[Aymeric Augustin]

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

[Aymeric Augustin]

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

[Claude Paroz <claude@…>]

In 83a185a3f7d07af295880ed02efba4af4c86a204:

[1.7.x] Ensured bound field renders as unicode safe data

Refs #22950.
Backport of 920904921 from master.

[Claude Paroz <claude@…>]

In dd9a23d5cfb44cb6f150610c267d0de1d963b849:

Fixed #22950 -- Eased markup customization for choice field rendering

Thanks Patrick Robertson for the report.