Jump to:

Form errors are set at the field level in form_set_error() and the array is checked for errors for each individual fields when rendering a form. The problem arises when two forms, which exist on the same page, have the same field names. The forms API marks the fields in both forms as errors.

An example of where multiple forms appear with the same fields is the editview module, where node forms exist repeatedly down the page.

The patch I've included moves the static array out of the form_set_error function, into a private function which allows you to set the arrays contents. I've also created a function which allows you to reset the form array, so it can be called between rendering two forms. This is not an ideal solution, as form errors should really be tied to the form_id so that if the same field exists in a number of forms on the same page, it doesn't mark all of them as an error on a page rebuild.

This can probably be integrated withthe new $view_state work that's been done in the FormAPI 3 patch for Drupal 6. If form errors are carried around in the $form_state variable rather than a general array, they can be restricted to just the form that's being rendered.

I don't think the proposed patch solves this problem the right way. I agree that we want to fix this, but we'll want to fix it properly. The solution might be to follow Eaton's advice, or to pass a $form_id into form_set_error(), form_has_errors() and friends.

The latter approach is something chx and I considered for the FAPI3 patch but decided against because of the already-large list of features that were going in. I'd love to see that happen; we already made a similar change to form_set_value(). I don't have time to tackle it myself but I'd be happy to give some guidance to anyone willing to take on the patch.

In linux it's as simple as going to the base installation directory of drupal (the one that has the sites directory in it) and running 'patch -p0 < form_2.patch'
I haven't done this on Windows, but I'm sure that the page above can tell you how to do it in that case.

If you're feeling cautious about patching core, which you have every right to be, I suggest reading patches before applying them.
This patch only effects the file '/includes/form.inc', so you could make a backup of this file and roll back if you're unhappy with the outcome.

I'd still love to see a resolution on this... but I don't think I have the background knowledge of the forms API to be able to do it myself.
I do like the idea of storing the errors in the $form_state variable, but again, not entirely sure what I'd be doing if I were to try and implement it.

I'd be happy to take a look into it if someone can provide me with some direction.

In a nutshell, we currently call form_set_error($name-of-form-element, $error-message). That stores things in a global variable that is shared across all forms. (whoops!) At its simplest, form_set_error and form_get_errors should be keyed by form id THEN form element. Ideally, though, we would instead build up elements in $form_state['errors']. We'd only display them to the screen when the form is rendered, and things like drupal_execute would be able to check the errors collection after execution to find any problems.

This may not be the place for this newbie question but.... I don't have shell access on my shared host, and I don't have any linux box at home (shame on me), so how can I use the patch? Is it possible to manually merge the two files? If not, does "patching" involve any modifications to any files other than form.inc? If its just a modification of this file, could someone provide me with a 'patched' form.inc (or am I being too naive?)

To require a form_id in addition to a form element is a good approach. But it doesn't solve all problems. The second thing is that you need a possibility to reset the error state of a form. For example if you run a script that calls drupal_execute multiple times on the same form_id (like we create nodes from an data file).
This patch solves the second issue. But due to the fact that a reset of the error array must be called from others it should not be private (function _form_set_error_array should not start with an underscore).

Bug fixes should not be put off to the next version when there is a patch available for the current version. Since there have been many changes since Drupal 5, a separate issue should be created for Drupal 6, or the status of this issue should be made “to be ported” after the patch has been applied to Drupal 5.

Applying this patch to Drupal 5 will not affect future versions. From what I can see, it’s ready to be committed. The re-roll below offsets the patch by four lines but does not change any code.

As implemented, this is an API change and should not go into Drupal 5.x. As far as I can tell, this does not actually fix the problem, just provides a new API to reset the form error array.

It is usually best to fix bug in the current development version first. More people will look at 6.x bugs since that is the development focus at the moment. This is an exception since 5.x and 6.x Form API are a bit different.

Here's a patch for drupal 6 that uses a different approach to solve the problem than the patches submitted above. It uses the POST variable to get the form_id. It then uses the form_id to scope the form element name so that when rendering the form element, it will be able to identify exactly which field the error applies to. The patch should be completely backward compatible, and requires no changes to any of the forms to work. This patch can be easily updated to work with Drupal 7 and 8. Please review and provide feedback.

I found a case where the patch in #30 causes problems when certain modules depend on the return value of form_set_error(). I reworked it so that the return value remains the same to allow for better backward compatibility. Added a parameter called $scoped, that allows one to request the errors array with scoped keys. This additional parameter shouldn't cause problems because it has an appropriate default value. I think for Drupal 8, these form_set_error() and form_get_error() functions should not depend on these two modes of operation, but use the scoped mode only.

First, patches must adhere to Drupal coding standards. Specifically, there should be spaces between function arguments, no trailing whitespace on any line, always use braces even for 1-line ifs, comments should be on the line above with a period at the end just to name a few. You can also use the coder module to review your patch and it'll highlight most of those same problems.

Second, since this is a patch for D8 first - let's have $scoped be TRUE by default - that should simplify the function calls somewhat. If/when it gets backported to D7, that's when the maintainer may request that scoped start off as FALSE for consistency.

Third, we need a test. It'll need a page with 2 forms on it, both with form_elements with the same name. Then submit it with the error, and ensure that only the form element with the error is marked.

I've been using the attached patch for drupal 7 that is based on my patch in #31 but with slight enhancements. This version keeps an array of errors indexed by form id. This approach allowed me to develop a special ajax module that displays errors to the correct form.

For drupal 8, I think it's safe to replace the old form_get_error() and form_set_error() with form_get_error_scoped() and form_set_error_scoped() respectively. Any contrib modules that rely on the return value of form_set_error() would need to be updated as it would return an form id indexed array.