Opened 11 years ago
Closed 10 years ago
#6529 closed (duplicate)
flatattr() doesn't check if a string is marked as "safe"
Reported by: | Martin Geber | Owned by: | nobody |
---|---|---|---|
Component: | Forms | Version: | master |
Severity: | Keywords: | ||
Cc: | martin@…, antisvin@… | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
In file http://code.djangoproject.com/browser/django/trunk/django/newforms/util.py in line 13:
It might be better to use conditional_escape instead of escape (http://code.djangoproject.com/browser/django/trunk/django/utils/html.py line 30, and 35)
I'm not able to set the attribute onchange in a form widget with JavaScript.
e.g.::
if 1 == '1' {return false;}
becomes::
if 1 == '1' {return false;}
This is kind of annoying.
Hope my explanation is understandable... :)
Cheers,
Martin
Attachments (2)
Change History (9)
comment:1 Changed 11 years ago by
Resolution: | → worksforme |
---|---|
Status: | new → closed |
comment:2 Changed 11 years ago by
(and you can place strings with HTML meaning, such as JS, here, you just have to use mark_safe()
to do so)
comment:3 Changed 11 years ago by
Cc: | antisvin@… added |
---|---|
Has patch: | set |
Resolution: | worksforme |
Status: | closed → reopened |
Original author didn't reopen the ticket, but I'll have to do it as I've just got bitten by this.
Note that what ubernostrum suggested doesn't work because flatatt function makes a call to escape instead of conditional_escape - it's the point of this ticket to make it possible to turn off autoescaping for attrs. I'm not aware of any reason for current behaviour, so likely it's accidental.
I'm also attaching an obvious patch just in case.
Changed 11 years ago by
Attachment: | escape_conditional_for_attrs.patch added |
---|
comment:4 Changed 11 years ago by
Triage Stage: | Unreviewed → Ready for checkin |
---|
I think this patch can be committed to SVN.
comment:5 Changed 11 years ago by
Needs tests: | set |
---|---|
Triage Stage: | Ready for checkin → Accepted |
guettli - thanks for the enthusiasm, but this patch is missing one essential thing - a regression test case. regressiontests/forms
already contains some flatatt tests; they need to be expanded.
Changed 10 years ago by
Attachment: | flatatt_with_conditional_escape.diff added |
---|
Patch file and regression test.
comment:6 Changed 10 years ago by
Needs tests: | unset |
---|
comment:7 Changed 10 years ago by
Resolution: | → duplicate |
---|---|
Status: | reopened → closed |
Yes, your explanation is understandable.
html result
As far as I know this is correct. It worked in IE6.0 and Firefox.
Please provide more information and reopen it, if you still think this is a bug in django.