Opened 18 years ago
Closed 18 years ago
#3810 closed (fixed)
Widgets should take a copy of the attrs dict
Reported by: | Chris Beaven | Owned by: | Adrian Holovaty |
---|---|---|---|
Component: | Forms | Version: | dev |
Severity: | Keywords: | ||
Cc: | Triage Stage: | Accepted | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Currently, Widget.__init__
looks like:
def __init__(self, attrs=None): self.attrs = attrs or {}
Which is a problem because if you pass an attrs
dict to the Widget it could get modified in place. For example, in Field.__init__
# Hook into self.widget_attrs() for any Field-specific HTML attributes. extra_attrs = self.widget_attrs(widget) if extra_attrs: widget.attrs.update(extra_attrs)
Although a slight change in logic, it would probably make more sense if Widget
copied attrs
(if its a dict)
Change History (5)
comment:1 by , 18 years ago
comment:2 by , 18 years ago
This issue came up when dealing with someone on IRC (if I recall correctly).
They had something like this for their form:
from django.newforms import * extra_attrs = {'class': 'special'} class TestForm(Form): field1 = CharField(max_length=10, widget=TextInput(attrs=extra_attrs)) field2 = CharField(widget=TextInput(attrs=extra_attrs))
Which incorrectly renders (with TestForm(auto_id=False).as_p()
) to:
<p>Field1: <input type="text" class="special" name="field1" maxlength="10"/></p> <p>Field2: <input type="text" class="special" name="field2" maxlength="10" /></p>
(note the second maxlength="10"
, because extra_attrs
was changed in place)
comment:3 by , 18 years ago
Triage Stage: | Unreviewed → Accepted |
---|
Yep. We should be making a copy. That seems to be normal practice in the standard Python libraries: clients pass by reference, consumers copy if they want to change.
comment:5 by , 18 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
I'm not seeing the problem, could you please explain.