I just upgraded my site to 4.7 this afternoon. ( http://malexmedia.net ) I'm a big fan of w3-compliance, so that was the first thing I checked after the site was done.

http://validator.w3.org/check?uri=http%3A%2F%2Fmalexmedia.net%2F

Everything passes except for the following error:

Error! Line 224 column 46: ID "edit-form_id" already defined.

An "id" is a unique identifier. Each time this attribute is used in a document it must have a different value. If you are using this attribute as a hook for style sheets it may be more appropriate to use classes (which group elements) than id (which are used to identify exactly one element).

Thanks to everyone who's worked on 4.7! The results are amazing!

CommentFileSizeAuthor
#8 form.inc_22.patch631 bytesfgm
#1 form_id_2.patch1.05 KBSteven

Comments

Steven’s picture

Steven commented
StatusFileSize
new form_id_2.patch1.05 KB

Essentially, this will occur with any two forms on a page, because they both have a form id, and the id given by form api (edit-form_id) is duplicated. The same goes for two forms which use form tokens: the hidden token field will also collide.

However, only the 'name' attribute matters for form submission and it is not required to be unique. The 'id' has been used for JS/AJAX purposes (and CSS, I guess). We've been giving them different values for ages: name="edit[form_id]" id="edit-form_id", so there is no problem in changing the id to be unique.

Patch attached which modified the #id attribute of the standard form API hidden fields by appending the $form_id (and fixes some bad code style).

I tested it with the login block, search block and theme search form on one page. They all work fine.

Steven’s picture

Steven commented
Status: Active » Needs review
alexmarkley’s picture

alexmarkley commented

For what it's worth, I just patched my site, and it works like a charm! Thanks, Steven! :)

Jaza’s picture

Jaza commented
Status: Needs review » Reviewed & tested by the community

Works fine for me too. Like Steven said, the id isn't part of form submission, and I'm not seeing that the change breaks anything that makes use of the id. +1, and I think it's ready.

dries’s picture

dries commented

Why do we mix - and _? Can't we get rid of the ugly "form_id" to begin with?

archetwist’s picture

archetwist commented

http://drupal.org/node/40486

dries’s picture

dries commented
Status: Reviewed & tested by the community » Needs work
fgm’s picture

fgm
Primary language French
Location Paris, France
commented
StatusFileSize
new form.inc_22.patch631 bytes

Just a reminder: another fix has been discussed on http://drupal.org/node/63516, which is now marked as duplicate of this one to merge both.

Here it is again.

killes@www.drop.org’s picture

killes@www.drop.org commented
Status: Needs work » Needs review

The latest patch makes sense to me. Any other opinions?

drumm’s picture

drumm
he/him
Location NY, US
commented
Status: Needs review » Fixed

Committed to HEAD.

Bèr Kessels’s picture

Bèr Kessels commented

Gerhard asked about some feedback to backport this issue.

The only possible problems Gerhard pointed to, are related to Javascript and CSS. However, both JS and CSS require unique IDS to properly function. So, if a CSS design falls apart because we now have unique IDs, that is only because in the previous state, the CSS "acted wrong" anyway. JS, AFAIK always prompts an error when it tries to use an ID that appears more then once.

So: I don't see a big problem. Since we will only fix the CSS and JS usage, instead of breaking it.

What I cannot really see from the patch, is, if all CSS ids with these two specific ids will change, or if only those that appear twice, are to be changed. If its the first, then we will introduce some problems, but if its the latter, we only fix existing problems, and not introduce any new ones.

killes@www.drop.org’s picture

killes@www.drop.org commented

str_replace('_', '-', "edit-$form_id")

That is the transformation which is executed. That is all _ will be replaced by -. This affects all form IDs for hidden fields.

What now?

killes@www.drop.org’s picture

killes@www.drop.org commented

applied to 4.7.

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)
damaikar-at-sgprojx.com.au’s picture

damaikar-at-sgprojx.com.au commented

Hi folks,

Drupal Newbie (yesterday), CMS Newbie (last week:-P)...I hope this doesn't scare you too much.

I just wish to know WHERE exactly I should place this patch-code....i.e. In which file?

This may be glaringly obvious to you, and I apologise for dumb question, but it is not obvious to me at all...yet:-P

Many thankss

Stephen G

response - mailto:sg@sgprojx.com.au