Opened 9 years ago

Closed 9 years ago

#5730 closed (fixed)

widgets not properly escaping content

Reported by: Densetsu no Ero-sennin <densetsu.no.ero.sennin@…> Owned by: anonymous
Component: Forms Version: master
Severity: Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

RadioSelect widget does not escape label text correctly.

Example:

>>> from django.newforms import RadioSelect
>>> r = RadioSelect()
>>> print r.render('test', 'test', choices=(('test', '<em>must be escaped</em>'),))
<ul>
<li><label><input checked="checked" type="radio" name="test" value="test" /> <em>must be escaped</em</label></li>
</ul>

Attachments (2)

radioselect_escape_label.patch (520 bytes) - added by Densetsu no Ero-sennin <densetsu.no.ero.sennin@…> 9 years ago.
Escape label in RadioSelect widget
widget_escaping.diff (7.9 KB) - added by Chris Beaven 9 years ago.

Download all attachments as: .zip

Change History (7)

Changed 9 years ago by Densetsu no Ero-sennin <densetsu.no.ero.sennin@…>

Escape label in RadioSelect widget

comment:1 Changed 9 years ago by Densetsu no Ero-sennin <densetsu.no.ero.sennin@…>

Owner: changed from nobody to anonymous
Status: newassigned

comment:2 Changed 9 years ago by Chris Beaven

Needs tests: set
Patch needs improvement: set
Triage Stage: UnreviewedAccepted

Bug confirmed.

Current patch incorrectly removes the space between the radio item and the label. Apart from that, just needs a simple test and it's good to go.

comment:3 Changed 9 years ago by Chris Beaven

Needs tests: unset
Patch needs improvement: unset
Summary: [patch] RadioSelect widget does not escape label textwidgets not properly escaping content
Triage Stage: AcceptedReady for checkin

In fact, this is a bigger issue than just RadioSelect. All widgets with choices are displaying the same behaviour.

On top of that, widgets aren't using conditional_escape so safe strings get double-escaped.

Patch with tests incoming...

Changed 9 years ago by Chris Beaven

Attachment: widget_escaping.diff added

comment:4 Changed 9 years ago by Chris Beaven

For this patch, I have left the choice values (and opposed to labels) being hard-escaped because that brings up different issues for selecting the current choice.

comment:5 Changed 9 years ago by Malcolm Tredinnick

Resolution: fixed
Status: assignedclosed

(In [6722]) Fixed #5730: Conditionally escape widget contents in newforms to avoid
inadvertent double-escaping. This still isn't perfect behaviour (since it's
unaware of the current context's auto-escaping setting), but that's a larger
problem that needs fixing and this change at least makes the existing
behaviour consistent. Patch from SmileyChris.

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