I have multiple instances of the same form on a page (I.e. one in a modal, one in the footer or the actual page, etc). Of course, it's rendering elements with the same exact id's. I could create multiple forms but they're all the same and it would be so much simpler to not have to worry about the same ID appearing everywhere.

One such ID, "block-webform-client-block-4" appears on the same page more than once and that isn't good. I know this sounds like a specific use-case, but I have run across other sites abusing the block functionality of webform and they too are just letting the duplicate IDs pass by their QC. I won't bring up the anti-ID war here but fwiw I am all aboard the no ID train. ;-) (I guess I did bring it up...)

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

quicksketch’s picture

Yeah, totally know where you're coming from. IDs are actually causing us a bit of a headache over here in this issue too: #1738342: Webforms rendered twice on one page do not work with same-page conditional actions (affecting Panels in some configs).

However even though Webform 4.x is an alpha version and we *could* change the markup, I don't think it's a good thing to bring on our users at this point. If you don't want the ID around form elements, you can override theme_webform_element() and change the way the IDs are used.

In your particular example regarding blocks, Webform (and all other modules) don't actually control the markup of the ID on their blocks. Everything Webform provides is *wrapped* by Block module. The HTML ID is derived from the internal identifier of the block (which truly is an ID since there's only ever one). It's a shortcoming of Block module that it assumes a block is only ever going to be displayed once. To fix the ID on the block, you'll need to override block.tpl.php in your theme.

RKS’s picture

Yeah that was a bad example. It was really the first ID I grabbed from the inspector, but things like id="edit-submitted-leave-a-comment" are problems for me too. I'll look into the theme_webform_element() and see if I can't make that work.

I'm glad this isn't just specific to me. In my case I investigated stripping the IDs after they are created, however, that wouldn't solve the problem of conditional actions. I think it would be good to go ID-less, but agree 4x is already in alpha and not a good starting point. Maybe investigating this for 5x would make sense to give the other modules time to get up to speed.

RKS’s picture

Also, if going ID-less is not preferable, you can also look into something like data-id or some other attribute. Switching to data-id would probably be a simple fix for all the 3rd parties to change how they specifying your elements and avoids the problems with duplicate IDs.

quicksketch’s picture

Switching to data-id would probably be a simple fix for all the 3rd parties to change how they specifying your elements and avoids the problems with duplicate IDs.

I'm not sure you can use data-id for CSS selectors reliably. They're also not good for selectors in JS, since there isn't an equivalent to getElementById or getElementByClassName for data attributes. Data attributes aren't really meant for selecting, they're for associated data (usually for use by JS).

I was thinking using classes for the IDs, just moving the ID attribute to a class (but sticking in "-id" so people know it's "unique"). i.e. id="webform-component-my-textfield" becomes class="webform-component-id-my-textfield". Of course there are already 4 other classes on that same div, but you get the idea. How's that sound?

but things like id="edit-submitted-leave-a-comment" are problems for me too.

Hmm, well this is a whole different bucket of worms. Those IDs are being added by individual form elements. And unlike the wrappers, the must be TRULY UNIQUE because the "for" attribute on the label is bound to the "id" attribute of the input field. This is required by the HTML spec in order to associate labels and their fields. For screenreaders, it makes the association so the label can be read when the field is given focus. So that particular ID pairing is impossible to avoid. If you've got labels and fields, they MUST have unique IDs. The only thing we have the option to change is the wrappers around the field and labels.

RKS’s picture

Oh, I wasn't meaning you would use the data attribute for CSS. I'd have to look to see if you even can without something extra like sass or something. I just assumed you weren't styling using the IDs and so wouldn't need to continue styling with the data attribute.

As far as jQuery goes, data attributes make for great selectors IMO. For example:

<form data-id="my-new-form">
    <input data-id="my-unique-textarea" />
</form>

$("input[data-id='my-unique-textarea']).each(...)
$("input:not([data-id='my-unique-textarea']).each(...)

But that might be more effort than it's worth to you, especially if you're cool with just adding another class. I'm sure that would be faster and simpler in the long run.

As far as labeling, you could always move to implicit labeling instead of explicit. I.e.:

<label>Your Name: <input type="text" /></label>
<label><input type="checkbox" /> Please check here for terms.</label>

and so on. Wrapping the inputs in labels is valid and works the same as the for/id relationship. You will find some argument against it especially some people with assistive readers, however, I haven't run across a reader with errors since XP and IE6. People would make the argument when those were still a thing and now that we can all forget IE6, it's not much of importance anymore. You also get the overly-semantic argument about it as well being that it is wrapped with a label. Since it isn't a hack, within the spec, it's a good option I use when I need to.

quicksketch’s picture

Title: Is there an option to prevent adding IDs to forms? » Replace use of IDs to classes on Webform wrappers to avoid duplicate IDs
Category: support » task

I'm going to change this to a task. I think we should switch the use of IDs with classes when possible because it will enable fixing of #1738342: Webforms rendered twice on one page do not work with same-page conditional actions (affecting Panels in some configs).

There was a similar issue at #1484946: Duplicated id attributs when webforms are displayed in blocks I marked duplicate.

Regarding the proposal to use implicit labels instead of the "for" attribute: I like the idea of reducing markup but I don't think it's feasible for the extent of labels that Webform uses. We have several situations where we want to have labels but *not* associate them with an individual element: grid, time, date, or any field that has multiple input elements. Since wrapping a label around multiple inputs is invalid HTML, we don't have an option to use the implicit approach everywhere, which would result in different markup for different element types, which would mean different CSS for different types (not ideal). That, plus using the current structure matches Drupal core, so it's less CSS to theme Webform vs. normal Drupal elements.

So let's focus this issue on the wrappers and accept that for now, labels and form elements themselves will have to stick with IDs.

quicksketch’s picture

Status: Active » Needs review
FileSize
12.27 KB

Here's a patch to convert just about all IDs to classes.

Old:

<div class="form-item webform-component webform-component-textfield webform-container-inline" id="webform-component-new-textfield">
  <label for="edit-submitted-new-textfield">New textfield </label>
 <input type="text" class="form-text" maxlength="128" size="60" value="" name="submitted[new_textfield]" id="edit-submitted-new-textfield">
 <div class="description">Description goes here.</div>
</div>

New:

<div class="form-item webform-component webform-component-textfield webform-component--new-textfield webform-container-inline">
  <label for="edit-submitted-new-textfield">New textfield </label>
 <input type="text" class="form-text" maxlength="128" size="60" value="" name="submitted[new_textfield]" id="edit-submitted-new-textfield">
 <div class="description">Description goes here.</div>
</div>

This patch also removes the ID from the <form> tag, the actions wrapper around the buttons, and the next/previous/submit buttons. All together, this makes it so that having the same form multiple times on the same page no longer has any ill effect. Note because Drupal still uses unique IDs for its labels and form elements, there will still be quite a few IDs on the page. But at least Webform can do its part and make all the wrapping DIVs avoid IDs and be consistent for themers.

quicksketch’s picture

Status: Needs review » Fixed

Sorry I should have put this patch up sooner, but I've been testing it locally for the past 3 weeks with success and all conditionals continuing to work. I've pushed this up into the repository, if it causes any issues, please reopen this issue or file a new issue.

quicksketch’s picture

Added an entry for this in our upgrading notes:

https://drupal.org/node/1609324#classes-not-ids

colemanw’s picture

I just updated a bunch of js in the webform_civicrm project to keep up with these changes (which are no doubt an improvement).
Note that ids are still used all over the place, including the form itself which is odd because in your comment @quicksketch you said you were removing the id from the form.
Anyway, this is a step in the right direction.

quicksketch’s picture

you said you were removing the id from the form.

Yeah I had removed it, but then put it back after realizing that it affected both tests and JavaScript. Without an ID, it's difficult to target forms in JS (i.e. document.forms['the-form-id']).

There are still a bunch of IDs in labels and fields themselves, which is needed for accessibility. @RKS provided some ways of fixing that problem above, but as stated, they would require some serious changes to our form structure that would deviate from Drupal's normal forms. If we have any other suggestions that are possible to implement to clean up the markup further, let's make new issues.

I just updated a bunch of js in the webform_civicrm project to keep up with these changes (which are no doubt an improvement).

Yay, excellent. :)

colemanw’s picture

Without an ID, it's difficult to target forms in JS

In anticipation that you might eventually remove it, I went ahead and changed my js to target the form with
$('form.webform-client-form-' + nid) which works fine (the form has a class that is identical to the id) so from my pov it would be safe to remove the id.

Status: Fixed » Closed (fixed)

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