Opened 6 years ago

Closed 22 months ago

#11725 closed New feature (fixed)

Unable to have label tags without a "for" attribute in form widgets

Reported by: Denis Martinez <deuns.martinez@…> Owned by: sergeykolosov
Component: Forms Version: 1.1
Severity: Normal Keywords: forms label for none dceu13
Cc: eromijn@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

In some cases I want to use a form widget without an input, such as pure javascript or iframe
(recaptcha_django in my case).

form.as_p() generates a label tag with an unmatched id: <label for="id_recaptcha"> .

This makes my page invalid in HTML5.

I tried to override id_for_label like this in my widget's code:

    def id_for_label(self, id_):
        return None

which gives <label for="None"> .

Anyway, I believe that this case should be treated adequately, because the docstring
states that id_for_label can return None values.

Attachments (2)

django-label-for-none.patch (848 bytes) - added by Denis Martinez <deuns.martinez@…> 6 years ago.
handle None values from id_for_label
django_label_for_none_incl_tests.patch (2.6 KB) - added by DenisMartinez 5 years ago.
Same patch applied to svn r13303, including tests

Download all attachments as: .zip

Change History (12)

Changed 6 years ago by Denis Martinez <deuns.martinez@…>

handle None values from id_for_label

comment:1 Changed 6 years ago by SmileyChris

  • Needs documentation unset
  • Needs tests set
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 5 years ago by DenisMartinez

This patch has been accepted since Django 1.0, so can it be included mainline ?

I don't want to have to apply this on every release.

comment:3 Changed 5 years ago by Alex

The patch is clearly missing tests, which are not optional.

Changed 5 years ago by DenisMartinez

Same patch applied to svn r13303, including tests

comment:4 Changed 4 years ago by baumer1122

  • Severity set to Normal
  • Type set to New feature

comment:5 Changed 3 years ago by aaugustin

  • UI/UX unset

Change UI/UX from NULL to False.

comment:6 Changed 3 years ago by aaugustin

  • Easy pickings unset

Change Easy pickings from NULL to False.

comment:7 Changed 22 months ago by sergeykolosov

  • Owner changed from nobody to sergeykolosov
  • Status changed from new to assigned

comment:8 Changed 22 months ago by sergeykolosov

  • Needs tests unset

Pull request: https://github.com/django/django/pull/1179

I had to reject tests from the patch above (those were doctests, and those tested a complete form rather than just a label tag).

comment:9 Changed 22 months ago by erikr

  • Cc eromijn@… added
  • Keywords dceu13 added
  • Triage Stage changed from Accepted to Ready for checkin

Looks fine to me. Tests work. I'd like to see it implemented slightly simpler, but because we also don't want a dangling for="" in the code when there is no ID, I think this is as clean as it gets.

comment:10 Changed 22 months ago by claudep

  • Resolution set to fixed
  • Status changed from assigned to closed

In be0bab1bb8da80402248cd1fa22fd4cc09b34fe7:

Fixed #11725 -- Made possible to create widget label tag without "for"

Thanks Denis Martinez for the report and initial patch, and
Sergey Kolosov for bringing the patch up to date.
Note: See TracTickets for help on using tickets.
Back to Top