form validation not called on node forms (as they use button level submit/validation). Attached patch resolves by special casing these forms, and adding the extra submit and validate handlers on the buttons instead of on the form itself.

Comments

mrfelton’s picture

Status: Active » Needs review
StatusFileSize
new1013 bytes
martin_q’s picture

This patch made no difference for me. I still wasn't getting any validation happening on my node form. I couldn't work out whether I needed to use the element ID
field-arrival
or
field-arrival[und][0][value]
but in either case it just didn't do the validation.

jordojuice’s picture

Another good catch. I'll check this one out tomorrow.

@martin_q - We are currently working on a better system for identifying form elements by eliminating the text identifiers (like your example field-arrival[und][0][value]) in favor of a graphical representation of the field. We recognize that text identifiers can be confusing in cases like these and an upcoming patch should clarify that by removing some unnecessary identifiers as well.

jordojuice’s picture

I would hope that there would be a better workaround to this. I hate to provide for a special case specifically because that doesn't really resolve the issue completely in my mind, meaning checking for the node form does not help when this is the case on another form. I still haven't even checked it out but I'm about to right now and I'll have to examine the form array to try to figure out if there's a more abstract way of determining whether validation and submit handlers need to be added to a button directly. Of course, recursively checking the entire form array to determine how it's validated might be costly too.

jordojuice’s picture

Hmm... I can't reproduce this. The validation is working great for me. Of course, I'm using the latest code which has been changed a lot lately but nothing relevant to how the validation is handled has been changed. I'll keep working at it though.

jordojuice’s picture

Status: Needs review » Fixed

I ended up implementing a different version of this patch. Since the module already scans forms to store their elements the first time a form is visited after being activated, the module will now check submit elements during the process to determine whether those elements use button level validation or submission callbacks. Then, in the form_alter the Rules forms support handlers are added to each submit element that uses validation or submission handlers.

jordojuice’s picture

Status: Fixed » Active

This change is not staying in after all. Validating all form buttons can cause major issues with the Rules because it would be hard to target which button's submission the user wants to validate, so I think maybe there needs to be a way to trigger an event for individual buttons within the form. Perhaps events can be added for each button on a form instead of one for the submission of the form. I'll look into it in the next couple days.

jordojuice’s picture

Status: Needs review » Active
StatusFileSize
new7.41 KB

Here's a patch that I'm considering. It adds button level validation and submission handlers to buttons that use them and adds events for those handlers. This allows validation and submission events to be invoked for all buttons of a form. We could probably even add validate and submit events to all submit elements now that I think of it.

jordojuice’s picture

Status: Active » Needs review
jordojuice’s picture

Status: Active » Needs review
StatusFileSize
new9.63 KB

This fixes a few bugs with that last patch and provides an option for enabling button level validation and submission handling.

jordojuice’s picture

Status: Needs review » Fixed

Please try out the latest development snapshot and let me know if providing button level validate and submit events solves this problem.

jordojuice’s picture

Status: Fixed » Needs work

164 passes, 2 fails, 0 exceptions, and 63 debug messages

I'm writing tests for the module now, and all tests have passed except the tests for form and button level validate events (which are the two fails in the events test you see above). It seems that these events are not even being invoked, so this certainly does need some work. I'm not quite understanding it since form and button level submit events are working great. But I'm open to patches again as I don't think I'll have much time to work on it again for a couple days.

jordojuice’s picture

Status: Needs work » Fixed

347 passes, 0 fails, 0 exceptions, and 105 debug messages

All tests pass with the latest commit. I'm still a little concerned with how I've seen the FAPI work, though, that this new feature could have the potential to break validation or submission handlers on some forms. The event tests I've written for Rules forms support can't possibly test all combinations of forms, so it will take some reporting by users to work out the bugs if they do exist. Also, there will be some forms where some events do not even make sense (such as the form validate event on the [node type] node form) so we may want to find a way to filter out the irrelevant events to make it less confusing. Still, I'll document the recommendation that users try button level validate or submit events in the event that form level validate and submit events are not behaving the way they expected.

Status: Fixed » Closed (fixed)

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