This issue occurs on default install of Drupal 5.15. Creating a radios form element and assigning it an id will assign the same id to all radios, creating invalid (x)html and creating issues for scripting.

The expected behaviour would be that the radios were enclosed in a container div, with the assigned id, and each child elements would be assigned a unique id based upon parent id and return value.

This has been rolled into D6 and it would be good to get these changes backported, although I notice that in the D6 version of this function the container element would not be assigned any id, making it slightly less useful for creating selectors in css/js.

CommentFileSizeAuthor
#5 forms_inc-367689-5-D5.patch1.54 KBgollyg
#2 forms_inc-367689-2-D5.patch1.53 KBgollyg
#1 forms_inc-367689-1-D5.patch1.34 KBgollyg

Comments

gollyg’s picture

gollyg commented
Status: Active » Needs work
StatusFileSize
new forms_inc-367689-1-D5.patch1.34 KB

Have added a patch against D5.15 based upon changes in D6..

Have tested with clean install by reviewing the code output in garland for a set of radios.

This is my first patch and, whilst having read the patch docs, I have no doubt done something wrong. Be gentle...

gollyg’s picture

gollyg commented
StatusFileSize
new forms_inc-367689-2-D5.patch1.53 KB

Okay, so the first patch created correct wrapper ids for the elements, but left the inputs themselves with duplicate ids.

This next patch checks to see if the id has been set in the #attributes string and, if so, assigns the modified id to the element for further processing.

gollyg’s picture

gollyg commented
Status: Needs work » Needs review

Marking this for review, as it is working on my site without issues.

drumm’s picture

drumm
he/him
Location NY, US
commented
Version: 5.15 » 7.x-dev
Status: Needs review » Needs work

Committed #1 to Drupal 5 because it is a straight backport.

The additional change in #2 needs review against the most recent affected version and then backporting as necessary. The comment does need to be punctuation and capitalization like an English sentence.

gollyg’s picture

gollyg commented
Version: 7.x-dev » 5.16
Status: Needs work » Needs review
StatusFileSize
new forms_inc-367689-5-D5.patch1.54 KB

Changed the capitalisation and punctuation to normal English.

Updated patch to apply to 5.16.

Not sure what backporting to other versions involves?

gollyg’s picture

gollyg commented

Ah, so i should have patched against 7?

drumm’s picture

drumm
he/him
Location NY, US
commented
Version: 5.16 » 7.x-dev

Yes, changes need to go into the development version and then backported as needed.

gollyg’s picture

gollyg commented
Status: Needs review » Needs work

Cool, will do that soon.

effulgentsia’s picture

effulgentsia commented
Status: Needs work » Fixed

This looks fixed to me in D7. Re-open if I'm wrong.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.