Allow suppress of form errors (hack to make "more" buttons work properly)

Sorry I wasn't entirely clear, this patch specifically targets poll.module and the problems it experiences with the "more" button. We can distribute this to other #ahah enabled buttons as we convert them to the new FAPI approach.

The idea behind this patch was cooked up after a few hours of brainstorming with quicksketch about one of our fundamental problems: in D6, we can do different validations depending on what we're doing (delete won't enforce business rules about data entry, for example, while submit will). The #required property in particular, though, can't be bypassed during certain kinds of validation, even if we're really only trying to process a small portion of the form and the un-populated #required fields outside that section wouldn't be executed anyways.

This keeps the current behavior as the fallback, and lets buttons/forms supply a small function that determins what part of the form needs to have #required enforced. It also degrades perfectly, since it isn't tied directly to the AHAH handling function. Poll.module's more button, for example, works now even without the Title filled in, with or without JS. Good times.

I'm very curious about chx's thoughts on this approach.

Updating the SimpleTests to avoid notices.

This patch also renames the property from "#validate_portion" to "#validate_section". Here's the complete example from poll.module:

looking over the patch the only thing that jumped out at me was the wording of this comment:

I could only agree to this if you would hand only the validated portion to the submit handler. Alternatively, I also suggested moving AHAH processing into #after_build.

Things we discussed and agreed on

1. #validate_section can only be used on a button
2. If the pressed button has a #validate_section then it's used but then only the button #submit callbacks fire, the form level do not.
3. #validate_section becomes an array of keys. Should you want to do two parts of the form, you put 'em in a wrapper and theme them apart as necessary.
4. The patch does not get in without the explicit consent of Heine and me.

Eaton, I have given up on the "partial section to the form handlers". It'd be great if we can make it work but even if we cant the tight coupling of validate_section with its submit handler will, presumedly, avoid any grave mistakes. We shall see what Nate can up with.

Alright, here's a new approach that makes #validate_section a single array of form keys, rather than as a separate callback. The code is a bit simpler this way, and certainly is much easier for developers to implement since they don't have to make a callback just to return an element.

I also updated the tests to check that both #validate_section and #submit properties are required on the button in order for the partial validation to have any effect. Basically this patch completes the 1-3 requirements of #13, though a review of the implementation would still be welcome.

There is no point IMO on settings #validate => array() on a button, only #element_validate fires there anyways. I also doubt that you need to change how element_validate is called. Having a possibility to peek at the complete form is good thing still. Noone said we want to validate that. This might make it necessary to change the recursive logic a little bit. The recurse logic needs more comments anyways.

Subscribing! Will review ASAP.

No point in enforcing an empty, meaningless empty #validate. I do not know about "Similar to settings #submit on the button prevents form-level submit handlers from firing" I thought that button and form level handlers add up.

Nope, our #submit and #validate handlers override any form level handlers. They do not add up. Otherwise this patch would be a lot bigger to prevent such behavior. This means when we click this "more" button, the only submit handler that gets fired is the one assigned to it. This should reduce accidental execution of submit handlers by quite a bit. However, if no #validate callback is defined on this button, then the default form-level validators fire, which we don't want.

The changes to _form_validate() are for efficiency -- sorry, but google php reference counting. It's actually slower to pass around by reference.

Ah, sure enough. There's a lot of misinformation and assumptions about pass by reference vs. pass by value I suppose. I'll change these back to value.

Finally,

my problem is there, we previously had the complete_form and I would keep that. Or you are saying this is still so just your reorganization made it so that now $form is the complete form?

Yes that's right. $form is now the complete form. There's no point in calling it $complete_form when the other elements are called $elements. The values passed to element level validators is unchanged. This patch should not introduce any API changes in any way, just an API addition.

Ha, my upload patch slipped in there too. You can use #22 if you'd like 2 use-cases of this functionality. :)

Here's the patch with only FAPI and Poll module's changes.

Chx gave me a much, much better way of getting the section of the form to be validated. Rerolling.

A version with less strenuous form checking and so, less code.

Minutes (summary) after a discussion in IRC a while ago.

The patch in #27 contains this gem:

This demonstrates how easy it is to accidentally get unvalidated values at the formbuilders or hook_form_alter; #node will contain non-validated data after node_form_submit_build_node has run.

To prevent such mistakes, $form_state['values'] should only contain validated values (as suggested earlier by chx). Chx suggested that form_set_value was to be used in validation to have an automatic safe $form_state['values'].

Several existing issues cover other aspects of partial and skippable validation. I suspect many could be marked as dupes, but I'm not clear if any existing patch covers these other needs.
#280465: #required (and #element_validate too?) not skippable
#332614: allow buttons like 'reset' to bypass validation

The last submitted patch failed testing.

the summary list of followups:
#370823: Allow Nodes to Be Previewed Without Required Fields
#216064: Delete button triggers form validation
#280465: #required (and #element_validate too?) not skippable
#370819: Upgrade Upload Module to FAPI3

Okay, here's an updated patch that takes extra security precautions per Heine's #29. Now if using partial form validation, the $form_state['values'] array is only populated with the validated portion of the form. After making this change, the dangerous node-building that Heine pointed out in #29 blows up spectacularly (as it should) because $form_state['values'] didn't contain any of the values it needed to build the $node object.

Unfortunately due to the way the node form works (the form actually gets a built $node variable rather than using $form_state), this approach has terrible side-effects on the non-JS behavior. If you disable JavaScript and click the "More choices" button, it wipes out all the values of the form except for the the poll choices. Doh! This is because that node_form_submit_build_node() must be called on every build of the form, otherwise the $node variable passed into hook_form() doesn't get populated.

So, this means that for the node form, non-JS partial validation will probably not be possible in a secure fashion. Because it is impossible to rebuild the pseudo-node variable without a complete $form_state['values'] variable. In the future, if we convert hook_form() to not take a $node object, this could be fixed.

However, the full node object is definitely not necessary when we're only rendering a portion of the form (such as the choices), so we can still render just the choices part of the form with the validated values and update it via JavaScript without any problem.

Note this patch moves the location of drupal_set_value() in form.inc, which could have strange side-effects for form elements that use the $form_state['values'] variable within #process callbacks. However this is never the case in core, and $form_state['values'] just gets replaces with the $element['#value'] later anyway. So this could break some modules, but only if they're using $form_state is a very strange way that could be written differently anyway.

An unrelated issue not caused by this patch #445736: Poll does not respect weight in Preview or More button.

Arg, thwarted by #440894: Poll.module whitespace & coding style cleanup committed this morning. Reroll.

subscribing, will review in, ahem, 2 weeks

subscribing.

Manual re-roll against latest HEAD, accounting for new ajax.inc and plenty of other Form API changes. Just to see what the bot thinks.

Reroll based on sun's #47 that should fix the majority of the notices we're getting (mostly from comment module). We need to maintain NULL values in $form_state['values'], so we had to use array_key_exists('#value', $elements[$key]) instead of isset($elements['key']['#value'])

Still doesn't pass a lot of tests but I'm letting test bot have another look.

The last submitted patch failed testing.

The last submitted patch failed testing.

Hmm, the node_form_submit_build_node() pattern is something we generalized to all entities as they were made fieldable, so that they could support 'Add more' : taxo terms, comments - not users for now, because user form code was a little scary back then, dunno if the recent cleanups made that area cleaner.
#367006: [meta] Field attach API integration for entity forms is ungrokable was open to get feedback on the approach, but I guess the issue title was not really enticing...

/me wonders if he should run and hide :-/

It will do until we clean up node - form relationship in D8 if we do.

I can't stand Eclipse patches because they don't say what function they're from. Here's a straight re-roll of #56 for legibility.

This actually is what I'd consider a critical D7 API clean-up, since the UX for AHAH forms is pretty bad without this. For example, uploading a file when you haven't filled out the node title yet gives you an error at the file upload field telling you that node title is required.

I don't understand the added count() check, and the inline documentation wasn't updated to explain it.

We could use some inline documentation here that explains how exactly this recursive #parents skipping check is supposed to work.

This review is powered by Dreditor.

Patch needs at least one test? Might be as easy as adding another file upload and making sure there is no validation error message.

.

Revamped the comments once again, 'cause I didn't like them.

Discussed this some more with chx in IRC.

Also note that I renamed #validate_section to #validate_parents. There's absolutely no need to invent a new term here. The value of #validate_parents is an array of #parents of elements to validate. And #parents on its own is an essential Form API concept that every Drupal developer should know.

This paragraph is confusing to me. How about:
----------
For example, when an "add another item" button belonging to a field is clicked, we don't want a missing required title causing a validation error and preventing a new item from being added to the form. Setting that button's #validate_parents property to the #parents of the field will result in only elements within that field being validated when that button is clicked.
----------

Check whether the pressed button defines #validate_parents to limit validation to elements within those parents. We prevent insecure execution of form-level submit handlers, by not supporting limited validation unless a button-level submit handler is defined.

This is confusing to me, because "contained in" can be interpreted as "this element is a descendant of" as well as "there is an intersection between #parents and #validate_parents". Would it help to call the property "#validation_target", and then explain it as "if the validation target is not an ancestor of the element"?

Why #parents instead of #array_parents? Do we want #tree=FALSE to influence this logic in the way that using #parents suggests? If so, can there be a comment explaining why we want that?

This review is powered by Dreditor.

Thanks! Incorporated those suggestions.

Why #parents instead of #array_parents?

This is all about user input validation, i.e. the now well-known $form_state['input'] and $form_state['values'] ;) (as also the @todo for D8 explains), and #parents define the parents for submitted form values.

Grammar: either remove "By" or rephrase second part of sentence.

Grammar, but also "in reality" seems superfluous. How about: This should live in _form_builder_handle_input_element(), because ideally, we should skip not just validation, but all input processing for unrelated elements.

What's still tripping me up is the use of plurals, because even though #parents and #validate_parents are arrays, they refer to something that is conceptually a singular entity (each array defines a singular point in the $form_state['values'] tree, and what we're trying to determine is if the one point in the tree defined by #validate_parents is an ancestor to the one point defined by $element['#parents']). How about: By setting #validate_parents, the button has indicated that it only needs that portion of $form_state['values'] to be validated. If the element's #parents property is such that the element's value resides in a different part of $form_state['values'], then validation for this element may be skipped.

This review is powered by Dreditor.

Thanks!

Though it does not appear in later comments this approach has my approval.

Had a long conversation with chx in IRC about this patch.

My main concern is that I think module developers are not going to get this right, and accidentally bypass validation when they think they are in fact validating.

#validate is a "developer-facing" property. It's something most developers will end up using quite often through the course of their module building. #parents, on the other hand, is largely an internal, "form.inc-facing" property. I don't think I've ever once encountered a situation where I needed to futz with #parents, and I've written my fair share of forms. :P

#validate_parents has two issues:

1. It's not at all descriptive of what it does, which is "send only this small chunk of the form for validation, bypassing the normal form validation process"
2. It requires specifying the entire parents array in order to get the validation to work as you'd expect. For example, if you had the following form:

You would need to add #validate_parents = array('foo', 'bar') in order for this chunk to be validated. Not the fields themselves -- array('bazzle', 'quxxie') -- and not just the immediate parents of the fields -- array('bar') -- but the entire hierarchy array.

Not having to blank out #validate yourself certainly helps, so #80 is an improvement over #78. But I still think people are going to get this wrong. :( However, I also have no bright ideas on how to address this, and certainly want to see this fixed. I'll leave it until Boxing Day and hope we have a Christmas miracle. ;)

@webchick: I'm sad to hear that the PHPDoc for _form_validate_skip() and the code comments within it fail to explain the concept well enough, but I agree with you that the concept isn't explained so clearly that developers will just "get it" and use it correctly.

Here's a stab at explaining it: maybe something here can be useful towards whatever further documentation is needed.

You're building a form, and have a button on it that will fire a button-level submit handler instead of the form-level submit handler. That submit handler does not need the entire form to be valid, and for best UI, specifically does not want the entire form to be validated. The submit handler will be working with some part of $form_state['values'] (in your example, $form_state['values']['foo']['bar']) and needs that part of $form_state['values'] to be valid. So, that button needs to set #validate_parents to array('foo', 'bar'), which means: make sure that everything within $form_state['values']['foo']['bar'] is valid, but don't validate any other part of $form_state['values'], because it's not needed for what the submit handler needs to do.

Does this explanation help, and if so, does it point to needing a better name for #validate_parents, or just better documentation somewhere?

By the way, I'm a little torn on forcing #validate to exist, and therefore always bypassing form-level validation handlers, even without the developer explicitly setting #validate. I understand why you don't want form-level validation handlers running, since they may be validating a part of $form_state['values'] outside of what's specified by #validate_parents, but OTOH, they may also be validating a part of $form_state['values'] that is within #validate_parents, so not having them run without the developer explicitly setting #validate seems to me like being more prone to developers thinking something is being validated when in fact it isn't. Given that you don't know what a form-level validation handler will do, if chx, webchick, and others think it's best to never have them run in partial-validation mode, then I can accept this, but because of this concern, I'll leave it to someone else to RTBC.

Right; I should clarify... the docs for _form_validate_skip() are great. But these are buried in form.inc, in the headers of a private, internal function. I don't know how an average module developer is ever going to find those docs when seeking the solution to this issue.

Your average module developer is going to go, "I need to add an AJAXy button pressy thing and don't want the title field screaming at me to fill it out. I noticed Poll module does this. Let me go see what Poll module does." And that module developer is probably going to be looking around the Poll's "add more" button for a property like #validate_this_section_only = TRUE.

When that is not found, they'll look at the rest of the properties there and by process of elimination, probably figure out that #validate_parents is the magical sauce that makes it work. Then they'll proceed to bash their heads trying to figure out why Poll module only had to pass an array with one index but they have to pass in an array with multiple, since bazzle and quxxie are two levels deep. (Though the upshot is that this head-bashing might cause them to grep the source for #validate_parents and come across the nice docs in _form_validate_skip(). ;))

However, nowhere else in the form API that I'm aware of do we link two dependent properties together in order to achieve a certain behaviour, so I'm not sure how they would also figure out that they need to add #validate = array() as well, without reading this patch. This is why #80 is preferable to me over #78.

But even more preferable to me would be a simple TRUE/FALSE property and have Form API just be smart about it, so that a human being doesn't need to re-iterate to Form API the form structure she has already written elsewhere.

I agree with sun that I'm not the target audience of this patch. But OTOH, I remember rampant mis-use of the #DANGEROUS_SKIP_CHECK property, back when that was around to deal with AJAX validation issues, so I think you are over-estimating peoples' ability to keep themselves out of trouble. ;)

We can do something else: add the name of the button to the section to be validated. This avoids #parents which is good because you do not always have #parents set, what if you have #tree FALSE and just want to bundle a bunch of elements under a wrapper? We can do that. (And I do not think the verbose and obtuse name of the new form_state/element property is good but we can come up with a better name)

Edit: this is a brand new patch using nothing from previous efforts. Some credits go to webchick because she said "What I want is like #validate_this_chunk = TRUE and form API to just figure nit out".

This seems like a much, much simpler approach to me, it's just a few if/else changes in form.inc, and the form definition is really simple.

Hopefully we can come up with something better than #validate_exclusively_on_button. Would just #validate_on_button be enough?

Buttons and the sections are closely interrelated enough that I still think that my patch is better. #92 is 8.x material IMO.

I do not like validate_parents because it depends on parents and what will you do if you just want to wrap things on containers for rendering and not change their parents? If my solution is problematic because of the buttons have same labels -- good catch -- then add more information in there.

If I have $form['foo'] and $form['bar'] and want to validate these and these only how do you specify that? They do not have a common parents "root".

I agree with sun that I'm not the target audience of this patch. But OTOH, I remember rampant mis-use of the #DANGEROUS_SKIP_CHECK property, back when that was around to deal with AJAX validation issues, so I think you are over-estimating peoples' ability to keep themselves out of trouble. ;)

At its core, FormAPI was designed for monolithic validation/processing of a single $_POST that maps to a single form, and results in success or failure. Period. End of story. We've grafted a lot of behavior onto it since then, but in this issue we are hitting the wall.

We have three conflicting desires:

• Robust automatic security
• À la carte validation
• Ease of use/discovery for developers

I could be wrong -- someone may have an elegant solution that achieves all three. But I have the strong suspicion after spending some time thinking through the early iterations of the patch, and catching up on the more recent ones, that we're going to need to choose two of those and be happy with them until we can dedicate the time and energy for a deeper rewrite of FAPI's internals. Until we can prioritize those three desires I think we're doomed to keep patching in circles.

Ok, here's an attempt to take everyone's wishes into account. I hope you like it.

While being a nice and easy idea, the approach in #101 does not support #97 at all

Please explain. To my eyes, it's the same as your above code block with #validate_parents renamed to #validate_sections (because #validate_parentses would be kind of weird). It allows a numeric constant for relative-based assigning, because of the desire expressed in #87. But you don't have to use a numeric constant, you can use array('foo') if you want to, and the PHPDoc includes that and the poll use-case uses it. And you can mix-and-match, so for example: #validate_sections = array(VALIDATE_SECTION_PARENT, array('foo'), array('a', 'b', 'c')).

OK, on it.

effulgentsia , could you please move your superb nice comments to this one? a) you are skipping validation and skipping at the wrong place. I allow instead and at a much nicer place b) the tests are seriously ugly c) biggest problem you use parents but array_parents is what you want. I removed grandparent constant too.

We are now covering two cases nicely: one is a node-like form where the buttons are in a buttons contains and they need to validate a bit of this and a bit of that and the field-case where we generate a complex form subtree where we need to validate only ourselves.

So, if #validate_sections = array(array('foo', 'bar')), won't _form_validate_allowed($form['foo'], $form_state) return FALSE, and we won't recurse to even check if it's allowed for $form['foo']['bar']?

I asked in #73 if we wanted to use #array_parents, and sun felt strongly in #75 and later comments that no, we want #parents, and #101 makes a point to comment this.

Nice!

I'm on crack. Are you, too?

So, if #validate_sections = array(array('foo', 'bar')), won't _form_validate_allowed($form['foo'], $form_state) return FALSE, and we won't recurse to even check if it's allowed for $form['foo']['bar']? <= Oh. that's one valid concern! If you can fix it, please, I am on update tests now or I can fix tomorrow.

Let's hope this passes the bot.I made a typo after running tests and that's why it failed. There is no change #114 stil stands etc.

I discussed with sun and he said validate/submit revolves around submitted form values. He is right. Now, despite internally this is handled by using #parents there is no point in exposing this so I renamed the controversial property to #validate_values and gave examples using $form_state['values']. That's nice and easy to explain and no crazy #parents stuff is involved in the explanation. Surely there is in the implementation. I have added lots comments based on previous patches and I think this is a very happy patch now.

So to recap #114: we use $form_state['values'] and everyone agrees here now I think. There is no need or point of having a VALIDATE_SECTION_PARENT constant -- we never used such constants and it does not reflect in $form_state['values'] anyways.

The structure of form_validate is simpler with _form_validate_skip so that's what we use. The code doing that is very simple, too.

Thanks!

Add a short inline comment above this condition, so people trying to understand the flow don't have to look up that function? That comment should also clarify why we recurse into all children, regardless of whether a parent is skipped (#parents).

Stale #validate_parents

Wrap "Previous" in quotes. Perhaps also mention AJAX here?

s/form level/form-level/

s/fire/executed/

s/Button level/Only button-level/

Quotes around "Previous" and "Add more".

Not sure whether "do not persist anything" is actually correct.

Missing $ before form_state.

Missing colon (:) after "example".

Missing @code + @endcode. Ideally, embedded code should have an extra indent of 2 spaces.

Extra blank line afterwards can be removed.

I still have issues to understand this comment. What exactly do you want to say? :)

I think we can shorten this to:

"Otherwise, check for a form-level handler, but only if the complete form was validated."

"Tests partial form validation."

Can we squeeze some inline comments in here to explain what is being tested?

Missing PHPDoc.

Form constructor functions should end in "_form".

"Element validation handler for the 'test' element."

I'm on crack. Are you, too?

Calling every form builder _form is stupid and leads to incomprehensible form_alters. But I won't fight that here so I did everything.

Poll module, son of evil: changes $node in the validate handler. We can fix that of course.

The last submitted patch, allow_partial_validation.patch, failed testing.

The last submitted patch, allow_partial_validation.patch, failed testing.

There is no need or point of having a VALIDATE_SECTION_PARENT constant

I'm okay with removing it if webchick is. She's the one who wanted some support for the system just figuring out what to validate based on where the button is. We could rename the constant to something appropriate if that's the issue (given the change from #validate_sections to #validate_values). Or, if she's willing to give this up, then not support relativity. I see no conceptual difference, however, with relativity based on #parents or #array_parents. Buttons have both, and they mean the same thing for a button as they do for every other form element. If we're not gonna support this, there should be a clear reason for why not, such as the desire to have hook_form_alter() implementations move a button to a different part of the form (in a way that changes #parents in addition to #array_parents) without worrying about how that will affect validation. However, I'm not convinced by that argument, because if the argument makes any sense, then it should also apply to other elements, and it's already the case that if hook_form_alter() moves elements in a way that changes #parents, then validation and submit logic is affected due to $form_state['values'] being affected.

#validate_values = array(array('foo'), array('bar', 'baz'))
$element['#parents'] = array('bar', 'baz') should not be skipped.

I'm on crack. Are you, too?

Edit: this is here just to see whether it passes the bot. I will roll in the comments some time later. Or someone else can. I did my share.

The last submitted patch, allow_partial_validation.patch, failed testing.

We're 136 comments into this issue, everyone's burnt out and anxious to get this functionality in, we've been lucky enough to get as much involvement from chx as we've gotten, we've reached agreement on API and desired functionality, and #136 implements it successfully and elegantly.

So, while I could nitpick the code comments and other trivial details, I'd rather see this be RTBC, land, and then set back to needs work for remaining comment improvement that chx wouldn't need to be bothered with, unless he wants to be.

Given everyone who participated here, I'm not ready to set this to RTBC without at least one other person also approving it. But, as far as I'm concerned, it's commit-worthy.

Strange behavior with the patch:
- Edit a node having a multiple field with an 'add more' button
- Empty 'Title' field
- Press 'add more' (-> works as expected, new widget is displayed, no validation error)
- Save the node (-> works as expected, validation error, 'Title field is required')
- Fill the 'Title' back, save the node -> I get redirected to 'system/ajax', showing raw JSON data:

I had this feeling when fixing up poll. This is terrible. But, this is what I am afraid of now.

Edit: we need to change the form workflow or at least change every validate that also changes something in form/form_state into a #process which checks for submitted and works accordingly. This is not 7.x level. We can't go piecemeal, skipping only #required checks in validate but calling user validate routines because those will throw errors...

That idea sounds spooky at first sight, but in the end, it is the same result.

The only (resolvable) problem is that we currently don't have access to $form_state in http://api.drupal.org/api/function/form_set_error/7 and http://api.drupal.org/api/function/form_error/7, and thus, no access to 'clicked_button' -- however, we could certainly add it as argument, which would be a major improvement on its own.

Perhaps there's even a way without changing those functions via http://api.drupal.org/api/function/form_get_errors/7.

Poll tests will fail because the form level validate handlers still do not fire. On it.

Funny, I was just finishing up my attempt when chx posted his. I'll post mine anyway, and then we can compare and contrast.

The last submitted patch, drupal.form-validate-values.150.patch, failed testing.

Hey my patch passed. Then something is wrong locally but it passed!

D'oh because I posted the wrong patch. So the patch to consider is #150 and this once I figure out how to make this pass :)

Hey! I got poll working with the new code after doing some cleanup work. It's _incredible_ how poll only worked by accident -- the form level submit handler was doing crucial work for the More choice submit. And yet it did the very same in validate. What a mess!!

effulgentsia won. The problem with mine is that a form level validate handler -- like the node validate and the hooks therein .. -- will set errors on elements. Some of these might be against values we want to have errors some of these can be outside. My patch only works in a cleaner world... we have our work cut for Drupal 8 for sure.

Yep. Setting to "needs review" since this involves a code change, and in IRC, sun indicated he wants a chance to post feedback.

I have restored the security hardening and renamed to #show_errors and renamed the issue too.

Hey, chx, what do you think of this? Very small change to #164: just a difference of from where form_set_error(NULL, '', $show_errors) gets called. Also using drupal_static_reset() to turn off suppression, which handles all the edge cases I can think of. The only edge case remaining is if a #element_validate handler processes another form and afterwards calls form_error() for an element that should be suppressed on the form it's validating. But this is rare, it's not a security risk (suppression is off when it should be on, which isn't so bad), and the validate handler can easily remedy it by calling form_set_error(NULL, '', $form_state['clicked_button']['#show_errors']).

Also, the turning on/off can be moved from _form_validate() to drupal_validate_form() if you think that's better.

This patch does not implement feedback from #165. I just wanted to get this idea in front of folks.

Very nice.

Just fixed PHPDoc formatting and fixed a few comments.

Yeah, I agree that #show_errors is not a good name. That makes it sound like it's a Boolean property, but it appears this is still taking an array of sections to validate when the button is pressed.

Is there a reason we can't go back to #validate_form_sections or similar?

To avoid bikeshedding via issue queue, which is slow and tedious, could you folks just get on IRC when you get a chance and we can go through some names in real time.

Sorry for the re-roll taking so long. I'm slow at reviewing and polishing comments.

In addition to changing the property name and polishing comments related to it, I also updated the PHPDoc for poll and field "add more" functions, because the old docs were simply wrong.

Straight to RTBC, because the code changes were agreed to in IRC, and webchick is in as good a position as anyone to review the PHPDoc.

I created a new issue for the documentation needs: #675206: Document new FAPI property, #limit_validation_errors. This way, patches to change the FAPI doc files in CVS can be submitted on that issue.

Simply glorious, this patch works like a charm. Patch for fixing file upload validation problems is over at #675414: Use #limit_validation_errors on File upload and removal.

Just reviewing this in the light of #173/174, about documenting this new feature (and others). We're going to have to get a grip on FAPI docs and maintaining them.

#100680: Automatically generate Forms API & other big array documentation discusses this basic issue and suggests that we need to get this all into the code, in the same way we manage the hook documentation.

I'm proposing that we have a BoF at Drupalcon SF to brainstorm how to do this.

Is there a version of this for Drupal 5? I've modified the poll module from 5 to allow dynamic creation of new fields but there is a required field that should be filled out just to add new fields, only before submission. This would really be helpful.

Well, that's too bad. I got it working anyway. I set a flag (in the form of a hidden form item) to one state at the beginning of the form function to "invalid". Then I use my own node_form_add_preview function that sets this flag to "valid" before running drupal_validate_form. In the validation function I have a conditional that sets the error message only if this flag = "invalid". Therefore it only runs on submit, not on preview. This does not work for items that have "#required" set but it works for stuff specified in the validate function and that's all I need.

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