FAPI3 - duplicate id values for form elements [#150851]

Looking at the node form's page source, three different divs have the same ID, which would seem to be a problem for CSS:

<fieldset class=" collapsible collapsed"><legend>Comment settings</legend><div class="form-radios"><div class="form-item" id="edit-comment-wrapper">
 <label class="option"><input type="radio" name="comment" value="0"  checked="checked"  class="form-radio" /> Disabled</label>
</div>
<div class="form-item" id="edit-comment-wrapper">
 <label class="option"><input type="radio" name="comment" value="1"   class="form-radio" /> Read only</label>
</div>
<div class="form-item" id="edit-comment-wrapper">
 <label class="option"><input type="radio" name="comment" value="2"   class="form-radio" /> Read/Write</label>

</div>
</div></fieldset>

suggests a bug in the Forms APi.

Comment	File	Size	Author
#4	form-radio-id-150851-4.patch	2.27 KB	pwolanin
#3	form-radio-id-150851-3.patch	1021 bytes	chx
#2	radio_id_1.patch	1.07 KB	pwolanin

Comments

Comment #1

anders.fajerson commented 8 September 2007 at 15:20

admin/user/settings is another example where radios is affected. The IDs "edit-user-register-wrapper", "edit-user-signatures-wrapper", "edit-user-pictures-wrapper" are defined multiple times.

Probably this code is to blame (line 1906 in form.inc):

function theme_form_element($element, $value) {
  $output  = '<div class="form-item"';
  if (!empty($element['#id'])) {
    $output .= ' id="'. $element['#id'] .'-wrapper"';
  }
...

Comment #2

pwolanin commented 9 September 2007 at 01:47

Title:	FAPI3 - duplicate Id values for form elements	» FAPI3 - duplicate id values for form elements
Status:	Active	» Needs review

Status	File	Size
new	radio_id_1.patch	1.07 KB

hmm, I am having a hard time figuring out why the #id is set correctly for checkbox elements but not radio elements. However, attached patch fixes the bug at least.

Comment #3

chx commented 10 September 2007 at 23:16

Status	File	Size
new	form-radio-id-150851-3.patch	1021 bytes

That's quite a bit sketchy. I would do this instead.

Comment #4

pwolanin commented 11 September 2007 at 02:13

Status	File	Size
new	form-radio-id-150851-4.patch	2.27 KB

fixed parse error and added the same code for filter module's radio elements. Note - we always want to use the parents - otherwise the id may not be unique - essentially this is the same as setting #tree to TRUE, which expand_checkboxes() does in that case.

Seems to fix the problem.