form_set_errors() flags ALL instances of a form on a page

This looks very promising. I've run it through its paces on number of different forms and seen no ill effects; it also makes certain very impressive hacks like the "editview" module possible.

Long term we want to solve it in a 'proper' way; for now, simply fixing the bug may do. Thoughts? Anyone?

+1 on this

If for no other reason than the making the editview.module work.

Also confirming that I've tested this patch and have not discovered any errors so far.

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.

subscribing

Hi, I'm trying out the Editview, we need this module or something quite the same. How do I apply this patch? Copy & paste on forms.inc ?
A little scary pasting into the core...

There is a page here which details how to apply the patch
http://drupal.org/patch/apply

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.

Happy patching!

Changing the status back to being a bug against 6.x.

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?)

Thanks for any help with this

+1

This bug is really blocking a lot of cool administrative innovation here.

It's a bug, not a feature. While it may not be the perfect long-term fix, is there any argument that it is better than the current behavior?

Tried this on 5.2 but it doesn't patch. Is there an equicvalent for 5.2 as I can't get edit view module to work and it looks as though it's the same problem as with 5.1

Also interested in a 5.x patch/snippet.

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).

You can do this with a form_reset_errors(); anyway though.
Just a thought.

You're right. I didn't recognize this function. Sorry.

But if you decide to extend the patch and handle form_ids, form_reset_errors() should have a signature like form_reset_errors($form_id).

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.

An updated version of patch for Drupal 5.6

I discovered an additional issue. drupal_validate_form() behaves like form_set_error() and caches the form_ids of all forms that have been validated. So every form will only be validated once!

I extended the latest patch in this thread to solve the problem the same way like for form_set_error(). Now it's hopefully possible to really process the same form multiple times in one script.

I also added a function named form_reset_all() to reset errors and validation cache at once.

Anyone know if this works on 5.7?

We currently run drupal 5.7 with form.inc5-6.patch applied.

+10 :-)
subscribing.

Is this still necassery for Editview ? will this work on Drupal 5.10 ?

subscribe

@25: Patch appears to work for 5.10, cannot tell if it is necessary for edit-view (don't see a difference).

This is only necessary for Editview when JavaScript is disabled in the browser.