The Forms API Reference page has as the sample code for #attributes: $form['#attributes'] = array('class' => 'search-form';. The problem with this example is that if you use it from within a form_alter hook, you'll be deleting any previously defined attributes.

A better example might be:

  $form['#attributes']['id'] = 'example-id';
  $form['#attributes']['class'] = isset($form['#attributes']['class']) ? $form['#attributes']['class'] . ' ' . $my_class : $my_class;

The point of the second example is that in specific case of the class attribute, you should normally be appending your class, not overwriting it.

Comments

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

I personally don't see this as a problem. Shouldn't it be fairly obvious to programmers that in form_alter they are altering an existing form? Almost all of the examples in the Reference are setting up forms from scratch, and I don't think we want to change all of them so that they could also be used to alter an existing form.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Version: 6.13 » 7.x-dev

And if we do decide to change this, it should be done in Drupal 7 as well as Drupal 6.

Gary Feldman’s picture

Gary Feldman commented

Let me put it this way: I've found bugs because of this in two separate modules. That, to me, suggests a documentation problem. My guess is that when people first start working with forms, they're more likely to be using form_alter than building a form from scratch. And they're going to be looking at the API docs for snippets on how to achieve things. Combine those two points, and you get this error, which is fairly annoying to track down.

Perhaps a less controversial idea would be to add a section in the Quick Start Guide on Gotchas When Using form_alter?

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented

Does $form['#attributes'][] = array('class' => 'search-form') work? Or maybe array_merge?

ultimateboy’s picture

ultimateboy commented

Alright.. so now that drupal_attributes can accept multi valued attributes thanks to #493746: Enhance drupal_attributes() for multiple valued values, the followup to that patch is #326539: Convert 'class' attribute to use an array, not a string which converts all instances of classes as strings to arrays. With this followup patch, the new standard will be for all classes to be arrays so adding classes will be as simple as $form['#attributes']['class'][] = 'new-class'.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

webchick: Until issues in #5 are resolved for Drupal 7, the answer to your question in #4 would be no, because you would want to concatenate class strings, not merge an array. This will also remain true in Drupal 6. And as noted above, with the changes in #5, in D7 we would want to do
$form['#attributes']['class'][] = 'my_new_class';

Gary: Point taken in #3. So this particular doc issue can be resolved by adding a note in #attributes about form altering possibly wanting to preserve existing class (and other?) attributes by concatenating with the existing value, couldn't it? Are there other spots in the gigantic Form API reference where we might want to put similar notes -- Gary, any other form_alter gotcha's that you've noticed?

Gary Feldman’s picture

Gary Feldman commented

Any of the properties that take a array of items has this particular problem:

  • #after_build
  • #ahah (I think this is unlikely, but I'm not positive.)
  • #attributes
  • #element_validate
  • #options (in theory, but again it's unlikely)
  • #post_render
  • #pre_render
  • #process
  • #submit
  • #validate

For #ahah and #options, they seem so tied to each other or the rest of the form that it feels unlikely anyone would use form_alter to add these to an existing form node. The others seem a bit more likely, but I'm hardly an expert on any of this.

I think #element_validate and #validate illustrate the difference in usage. I don't see adding an #element_validate to someone else's element, but I can envision adding a #validate function to someone else's form.

I don't know if there are other situations in which form_alter needs to be done differently.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

OK, I'll update the D7 and D6 Form API Reference guides to put in notes about form_alter gotcha notes on those elements (I've been editing both of them a lot lately, so it will be easy for me to do this). Thanks for making the list!

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Assigned: Unassigned » jhodgdon
jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Active » Fixed

I have updated the D7 and D6 form reference docs (should be up on api.drupal.org within 24 hours) for these properties. I decided the easiest thing to do was to put the following note in the Usage section of those properties (and I agree, ahah does not apply):

Note: If you are altering an existing form via hook_form_alter() or a similar means, be careful with this property! You will probably want to add to the existing array rather than writing over it, so don't follow this usage example exactly.

If you think more needs to be done, re-open please...

Changes are committed (these docs are in the contrib repository):
http://drupal.org/cvs?commit=247672
http://drupal.org/cvs?commit=247670

Gary Feldman’s picture

Gary Feldman commented

That seems fine, thanks!

Status: Fixed » Closed (fixed)

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