Parts of nested tokens (in fieldsets) printed when left empty

This is a duplicate of #1191984: Webform token values not appearing in email text, but I've closed that issue in favor of this issue because this one is more succinct. This looks like it's a bug in Webform's token handling.

Hi.

I'm experiencing the exact same issue, and I'm happy to help with any testing required.

Any update on what might be causing it?

Thanks
James

Right now we haven't looked into it too much because the new token integration in the 7.x-4.x branch has solved this problem by switching to the Drupal core token system. Still happy to look at patches, but this isn't a high priority for myself.

The problem is in webform.module's _webform_filter_values() function. The str_replace() function strips away fieldsets.

<?php
  $find = array_keys($safe_replacements);
  $replace = array_values($safe_replacements);
  $string = str_replace($find, $replace, $string);
?>

So if %email['camp_weeks'] => fieldset and %email['camp_weeks']['camp_weeks'] => grid, str_replace() will strip out %email['camp_weeks'] from the original $string to be tokenized, and you'll be left with the useless ['camp_weeks']. The cleanup code won't see it since it's missing the token type and fieldset strings. Not sure how to combat at the moment.

csdco

The start of the problem is here. Only the first field set is getting iset to ''

    // Provide blanks for components in the webform but not in the submission.
    $missing_components = array_diff_key($node->webform['components'], $submission->data);
    foreach ($missing_components as $component) {
      $parents = webform_component_parent_keys($node, $component);
      $form_key = implode('][', $parents);
     $replacements['email'][$format]['%email[' . $form_key . ']'] = ''; 
     $replacements['email'][$format]['%value[' . $form_key . ']'] = ''; 
    }

I get this printed as a field's results, which is nested in field sets, on my form if there is no data entered for the field
[ethnic_racial_background_optional][racial_categories]
if I comment out the last two "$replacements" lines above I get
%email[background_information][ethnic_racial_background_optional][racial_categories]

So this part of the function is only setting %email[background_information] to blank and leaving the rest. I don't know how to fix it but this is the start of the issue. Hoping you're a better coder than I and can offer a solution.

I fixed it but forgive me if this is hacky. This worked for me.
The problem was the logic was treating field sets like things that should be getting user entered data. If they didn't get data, which they never would, then they it striped them out and broke other form components for which they were the parents,
So I just skip them.

    $missing_components = array_diff_key($node->webform['components'], $submission->data);
    foreach ($missing_components as $component) {
      if($component[type] != "fieldset"){
        $parents = webform_component_parent_keys($node, $component);
        $form_key = implode('][', $parents);
        $replacements['email'][$format]['%email[' . $form_key . ']'] = ''; 
        $replacements['email'][$format]['%value[' . $form_key . ']'] = ''; 
      }
    }

Thanks gstout, I will give that a shot and report back here.

It works. Patch supplied. But this is probably not the most ideal solution since the real problem is, I think, the fact that we have brackets touching each other in the fieldset-nested tokens. No simple way of combating this pops out at me.

I'm fine deploying this right now on a production site since it solves the problem, but maybe something more long term would be to check for any field type that a token doesn't make sense for, as opposed to just fieldsets? Something like:

<?php
if (!in_array($component['type'], array('fieldset', 'container', 'what-else-is-there?')) {
  ...
}
?>

Thank you very much for this change. I made it to my webform module as well and it does exactly what I want. :)

Attaching a patch that applies to 7.x-3.19.

Rather than hard-coding "fieldset" into the module, it would be better if we checked for the "group" property on the type, as other types of components (i.e. pages) can also be empty.

So instead of:

if ($component['type'] != "fieldset") {

Use:

if (!webform_component_feature($component['type'], 'group')) {

Or maybe this for better readability:

if (webform_component_feature($component['type'], 'group') == FALSE) {

Hm, and we need a patch for the 4.x version of the module, as the _webform_filter_values() function basically does not exist any more.

Yeah, I figured it would need to be updated for 4, I just wanted to share my adaptation for 3. #12 also makes sense.

Patch #12 works for current 3.20 also.
Please commit.

Here is a re-rolled patch against 7.x-3.x, including @quicksketch's recommendations from #12.

I did 3.x because that's what I'm using when noticing the issue. If I have time, I'll see about rolling a 7.x-4.x patch. In the mean time, I'd love to see this applied to 7.x-3.x.

Testing at 7.x-4.x, this does not seem to be an issue. The core Token system seems to take care of it.

Moving it to 7.x-3.x-dev.

Version 7.x-3.x is receiving critical bug fixes only.

That one's critical to me. ;)

@DanChadwick: I think I'm finally stepping up to take maintainership of the 7.x-3.x if no-one else does. At least as long as I am still using webform-7.x-3.x in production sites. And that will be a while.

It seems that this isn't enough after all. For HTML-emails fieldset-components render as an empty fieldset (two div-elements). Their tokens are already populated in the first loop in _webform_filter_values(). In the second loop (that #16) the token values is then replaced with an empty string. I think we have to find a better way to deal with this.

Here is a more comprehensive patch using a different approach:

1. It still includes an empty string for fieldsets as token. This avoids having tokens suddenly appear on sites that "rely" on tokens of fieldsets being replaced.
2. It guarantees a post-order of tokens (nested token always is replaced before it's parent). This is achieved by only looping on $node->webform['components'] (which is guaranteed to be a pre-order traversal) and then reversing the array.

Ah that was actually the interdiff. Attaching a patch that is really based on 7.x-3.x.

Changing to task to reflect the maintenance status of this branch.

I've tested the patch some more and it still looks good. It's now committed to 7.x-3.x.

@Dan: I think this is still a bug - otherwise I wouldn't have the mandate to fix it since 7.x-3.x is "bugfixes only" - isn't it?

Re #28 -- No problem changing back to bug report after its fixed. What I'm trying to avoid is having bugs in the issue queue for a branch for which we have no commitment to fix bugs, other than critical bugs. The same goes for the 8.x branch, for which we have no expectation of being bug-free and the "bugs" are really just things that haven't been ported yet.

The purpose of this is so that the number of bugs accurately reflects the quality of the fully-supported branches, which right now is only 7.x-4.x.