[Django] #32855: BoundWidget.id_for_label ignores id set by ChoiceWidget.options
Django
-------------------------------------+-------------------------------------
Reporter: Jacob | Owner: nobody
Rief |
Type: Bug | Status: new
Component: Forms | Version: 3.2
Severity: Normal | Keywords: auto_id,
Triage Stage: | id_for_label
Unreviewed | Has patch: 1
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 1
UI/UX: 0 |
-------------------------------------+-------------------------------------
If you look at the implementation of `BoundField.subwidgets`
{{{
class BoundField:
...
def subwidgets(self):
id_ = self.field.widget.attrs.get('id') or self.auto_id
attrs = {'id': id_} if id_ else {}
attrs = self.build_widget_attrs(attrs)
return [
BoundWidget(self.field.widget, widget, self.form.renderer)
for widget in self.field.widget.subwidgets(self.html_name,
self.value(), attrs=attrs)
]
}}}
one sees that `self.field.widget.subwidgets(self.html_name, self.value(),
attrs=attrs)` returns a dict and assigns it to `widget`. Now
`widget['attrs']['id']` contains the "id" we would like to use when
rendering the label of our `CheckboxSelectMultiple`.
However `BoundWidget.id_for_label()` is implemented as
{{{
class BoundWidget:
...
def id_for_label(self):
return 'id_%s_%s' % (self.data['name'], self.data['index'])
}}}
ignoring the `id` available through `self.data['attrs']['id']`. This re-
implementation for rendering the "id" is confusing and presumably not
intended. Nobody has probably realized that so far, because rarely the
`auto_id`-argument is overridden when initializing a form. If however we
do, one would assume that the method `BoundWidget.id_for_label` renders
that string as specified through the `auto_id` format-string.
By changing the code from above to
{{{
class BoundWidget:
...
def id_for_label(self):
return self.data['attrs']['id']
}}}
that function behaves as expected.
Please note that this error only occurs when rendering the subwidgets of a
widget of type `CheckboxSelectMultiple`. This has nothing to do with the
method `BoundField.id_for_label()`.
--
Ticket URL: <https://code.djangoproject.com/ticket/32855>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Forms | Version: 3.2
Keywords: auto_id, | Triage Stage: Accepted
id_for_label |
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_tests: 0 => 1
* stage: Unreviewed => Accepted
Comment:
Hey Jacob — Sounds right: I didn't look in-depth but, if you can put your
example in a test case it will be clear enough in the PR. Thanks.
--
Ticket URL: <https://code.djangoproject.com/ticket/32855#comment:1>
Django
-------------------------------------+-------------------------------------
| Rief
Type: Bug | Status: assigned
Component: Forms | Version: 3.2
Severity: Normal | Resolution:
Keywords: auto_id, | Triage Stage: Accepted
id_for_label |
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: nobody => Jacob Rief
* status: new => assigned
Comment:
Thanks Carlton,
I will create a pull request asap.
--
Ticket URL: <https://code.djangoproject.com/ticket/32855#comment:2>
Django
-------------------------------------+-------------------------------------
| Rief
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: auto_id, | Triage Stage: Accepted
id_for_label |
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Jacob Rief):
Here is a pull request fixing this bug:
https://github.com/django/django/pull/14533
--
Ticket URL: <https://code.djangoproject.com/ticket/32855#comment:3>
Django
-------------------------------------+-------------------------------------
| Rief
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: auto_id, | Triage Stage: Accepted
id_for_label |
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Jacob Rief):
Here is the new pull request https://github.com/django/django/pull/14534
against main
--
Ticket URL: <https://code.djangoproject.com/ticket/32855#comment:4>
Django
-------------------------------------+-------------------------------------
| Rief
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: auto_id, | Triage Stage: Accepted
id_for_label |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_tests: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/32855#comment:5>
Django
-------------------------------------+-------------------------------------
| Rief
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: auto_id, | Triage Stage: Ready for
id_for_label | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Accepted => Ready for checkin
Comment:
The regression test looks good; fails before fix, passes afterward.
I don't think this one
[https://docs.djangoproject.com/en/dev/internals/contributing/writing-code
/submitting-patches/#bugs qualifies for a backport], so I'm changing it to
"Ready for checkin."
--
Ticket URL: <https://code.djangoproject.com/ticket/32855#comment:6>
Django
-------------------------------------+-------------------------------------
| Rief
Component: Forms | Version: 3.2
Severity: Normal | Resolution: fixed
Keywords: auto_id, | Triage Stage: Ready for
id_for_label | checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"db1fc5cd3c5d36cdb5d0fe4404efd6623dd3e8fb" db1fc5c]:
{{{
#!CommitTicketReference repository=""
revision="db1fc5cd3c5d36cdb5d0fe4404efd6623dd3e8fb"
Fixed #32855 -- Corrected BoundWidget.id_for_label() with custom auto_id.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32855#comment:7>
Django
-------------------------------------+-------------------------------------
| Rief
Type: Bug | Status: closed
Severity: Normal | Resolution: fixed
Keywords: auto_id, | Triage Stage: Ready for
id_for_label | checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Dmytro Litvinov (added)
Comment:
Stacked also with that issue and found that ticket. Thanks for providing
fix and merging it 🙏
Any deadline to see it released for 3.2 version? I understand
[https://docs.djangoproject.com/en/dev/internals/contributing/triaging-
tickets/#ready-for-checkin there are a lot of pull requests and it can
take a while for your patch to get reviewed]
--
Ticket URL: <https://code.djangoproject.com/ticket/32855#comment:8>
Django
-------------------------------------+-------------------------------------
| Rief
Type: Bug | Status: closed
Severity: Normal | Resolution: fixed
Keywords: auto_id, | Triage Stage: Ready for
id_for_label | checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Jacob Rief):
> Any deadline to see it released for 3.2 version?
I don't believe it ever will be backported to version 3.2.
--
Ticket URL: <https://code.djangoproject.com/ticket/32855#comment:9>
Django
-------------------------------------+-------------------------------------
| Rief
Type: Bug | Status: closed
Severity: Normal | Resolution: fixed
Keywords: auto_id, | Triage Stage: Ready for
id_for_label | checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak):
Replying to [comment:8 Dmytro Litvinov]:
> Stacked also with that issue and found that ticket. Thanks for providing
fix and merging it 🙏
> Any deadline to see it released for 3.2 version? I understand
[https://docs.djangoproject.com/en/dev/internals/contributing/triaging-
tickets/#ready-for-checkin there are a lot of pull requests and it can
take a while for your patch to get reviewed]
The issue has been present since the feature was introduced. Per our
backporting policy this means it doesn't qualify for a backport to 3.2.x
anymore, see [https://docs.djangoproject.com/en/stable/internals/release-
process/ Django’s release process] for more details. Moreover, Django 3.2
is in extended support so it doesn't receive bugfixes (except security
issues) anymore.
--
Ticket URL: <https://code.djangoproject.com/ticket/32855#comment:10>