I have a multi-page Webform. I have installed IFE and set-up as per the instructions. IFE doesn't work on some of the fields in my webform. It appears that it doesn't work with Grid fields or Time fields at all. It also doesn't work with radio fields that use the Select or Other module. If it should work with these fields then maybe I'm missing something and any help would be gratefully received.

F

Comments

Status:Active» Needs work

Had a quick look at this but can't get it to work for the moment. The module picks up the errors, but everything stops when it needs to be rendered.

Title:WebForm Validation Errors not displaying in-lineMissing support for complexer field rendering (radios, date, etc...)
Version:6.x-1.1» 7.x-2.x-dev
Priority:Normal» Major
Status:Needs work» Needs review
StatusFileSize
new3.49 KB

I still have this for the 7.x version. Not working fields (and there should be a lot more): radios, date, email.
Since radio buttons are even core fields I go up with the priority. Digging into this I realized that the recursive function call in "ife_element_errors_set()" is not even close to be the right logic. I attach a patch which is at least working but it would be worth to think and discuss this a little but to get a real good solution.

I think this is somewhat the heart of the module...

I'm facing the same problem and patch #3 solved the handling of those elements.
However, in my case in does not pick-up elements like "title" because of the following piece of code that the patch contains:
if (strstr($error_anchor, $field_name . ']', true) === '') {

Because of the . ']', the 'title' field does not get picked-up. Do we need that?
If I remove that concatenation, it is working fine for me.

StatusFileSize
new3.44 KB

Attached to this post: Xamanu's patch (#3) with my suggested change in #4.
This would require some tests/reviews from the community though.

StatusFileSize
new911 bytes

The patches at #3 and #5 caused inline field errors on webforms to disappear entirely, for me anyway.

webform 7.x-3.18
ife 7.x-2.0-alpha2

I solved the "no inline errors for radio buttons" problem with the attached patch.

I suspect this isn't a complete solution, but it works so far for me :)

Status:Needs review» Needs work
StatusFileSize
new877 bytes

+++ b/ife.module
@@ -258,39 +258,71 @@ function ife_errors($op = 'get', $id = NULL, $message = NULL) {
+  // Check for complexer fields, used later for proper rendering
+  $field_types_complex = array('radios' => FALSE,);
+  drupal_alter('ife_complex_types' , $field_types_complex);

This needs a lot more documentation to let the user know about this hook and what it's for.

We should also explain why we set radios to false. It's not immediately obvious at all. And the trailling comma is in violation of Drupal's coding standards.

I'd also change this from a drupal_alter toa module_invoke and move our definition to our own ife hook.

Finally $complex_field_types probably reads better.

+++ b/ife.module
@@ -258,39 +258,71 @@ function ife_errors($op = 'get', $id = NULL, $message = NULL) {
+  // Go through all children of form
+  foreach (element_children($form) as $field_name) {

Now that we've got rid of the recurssion we're not going to be able to deal with deep hierarchies. Take a look at the parents recursion test form in the module I've attached.

+++ b/ife.module
@@ -258,39 +258,71 @@ function ife_errors($op = 'get', $id = NULL, $message = NULL) {
+      // Compare if error for given field has been set.
+      if (strstr($error_anchor, $field_name, true) === '') {

This confuses the form #parents property (which is how we identify form errors) with the form array structure.

https://drupal.org/node/48643

Again see the parent example for where this fails.

strstr is also very slow. If you're checking for the existence of a string in a known location use substr to get that location and compare the result.

+++ b/ife.module
@@ -258,39 +258,71 @@ function ife_errors($op = 'get', $id = NULL, $message = NULL) {
+          if (count($_SESSION['messages']['error']) <= 0) {

Count can't return less than 0 so let's not bother checking that case.

http://php.net/count

+++ b/ife.module
@@ -258,39 +258,71 @@ function ife_errors($op = 'get', $id = NULL, $message = NULL) {
+          // Extract the latest element of field.

latest -> last

+++ b/ife.module
@@ -258,39 +258,71 @@ function ife_errors($op = 'get', $id = NULL, $message = NULL) {
+          while ($element_child = element_children($element)) {
+            $element_child = array_pop($element_child);
+            $element = $element[$element_child];

This is only ever going to get the last child element from a group. Take a look at the tree recursion example for where this fails.

+++ b/ife.module
@@ -258,39 +258,71 @@ function ife_errors($op = 'get', $id = NULL, $message = NULL) {
+            // Check if a complexer type of field is used here

complexer -> more complex

+++ b/ife.module
@@ -258,39 +258,71 @@ function ife_errors($op = 'get', $id = NULL, $message = NULL) {
+              if (function_exists($field_types_complex[$element['#type']])) {

Let's not be defensive. We should probably just try and call the function directly. That way if I do make an error in declaring my identification function I'll get an error rather than it failing silently.

Title:Missing support for complexer field rendering (radios, date, etc...)Nested fields don't render errors correctly
Status:Needs work» Needs review
StatusFileSize
new3.13 KB

Here's another way to go about it which I think works a little better.

StatusFileSize
new3.13 KB

+++ b/ife.module
@@ -258,39 +258,50 @@ function ife_errors($op = 'get', $id = NULL, $message = NULL) {
+      ife_element_errors_set($elements[$key], $elements, FALSE);

Second argument should be $display.

StatusFileSize
new6.21 KB

So I've had some time to think about this more. What the lsat patch basically does is a depth first traversal of the form tree looking for elements with an error. Once it finds one it stops searching that branch. We stop because Drupal will treat each of the child elements as belonging to that same error and we don't want to repeat a load of inline errors.

This might cause problems though where a child of the current branch has a different error message. Are search will never see that node so the different error message won't be printed. What we need to do is continue the search down the child elements but ignore errors which have already been raised against their parents. This patch adopts this solution.

pacproduct also mentioned to me that my patch doesn't work with the date field. That's because date raises the error against a container. The function associates the error with the container element but we never alter the element info because it's not an input element so we don't output it. I'm not sure why only alter certain elements, seems like it would be better to alter everything and push responsibility for raising an error against the correct form elements back on to the implementing modules.

After I made that fix it still didn't work but that's because date actually removes our added theme_wrapper function. That seems to be a bug in the date module so we should have them fix it rather than trying to work around it.

I've opened an issue with the Date module here #2068825: Date form elements clobber #theme_wrappers to deal with the specific problem I mentioned in #10.