Code

Opened 5 years ago

Closed 11 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@…> 5 years ago.
handle None values from id_for_label
django_label_for_none_incl_tests.patch (2.6 KB) - added by DenisMartinez 4 years ago.
Same patch applied to svn r13303, including tests

Download all attachments as: .zip

Change History (12)

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

handle None values from id_for_label

comment:1 Changed 5 years ago by SmileyChris

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

comment:2 Changed 4 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 4 years ago by Alex

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

Changed 4 years ago by DenisMartinez

Same patch applied to svn r13303, including tests

comment:4 Changed 3 years ago by baumer1122

  • Severity set to Normal
  • Type set to New feature

comment:5 Changed 2 years ago by aaugustin

  • UI/UX unset

Change UI/UX from NULL to False.

comment:6 Changed 2 years ago by aaugustin

  • Easy pickings unset

Change Easy pickings from NULL to False.

comment:7 Changed 11 months ago by sergeykolosov

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

comment:8 Changed 11 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 11 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 11 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.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.