Problem/Motivation
hook_webform_component_info() allows developers to define components, including the component's conditional type. See below from webform.api.php:
// Specify the conditional behaviour of this component.
// Examples are 'string', 'date', 'time', 'numeric', 'select'.
// Defaults to 'string'.
'conditional_type' => 'string',
All functions needed for the 'string', 'date', and 'time' conditional types are stored in the webform.conditionals.inc include file; however, in the case of 'numeric' and 'select', the functions needed for their conditional behavior are stored in their respective include files - number.inc and select.inc. As a result, when a module defines a conditional type of either 'numeric' or 'select', then the conditional operator callback will call one or more functions that are not loaded. This will cause a fatal error. (Exception: a 'number' or 'select' component has been added to the form, which would cause the needed include file to be included)
In the case of the 'numeric' conditional type, several conditional operator callback functions call webform_compare_floats(), which is stored in number.inc.
In the case of the 'select' conditional type, several conditional operator callback functions call _webform_select_options(), which is stored in select.inc. It, in turn, calls three additional functions that are also stored in select.inc.
Proposed resolution
There are basically two solutions:
- Move the functions required for conditional behavior to webform.conditionals.inc.
- In WebformConditionals::executeConditionals, check for missing include files and then include them.
Remaining tasks
Maintainer feedback
User interface changes
None anticipated
API changes
None anticipated
Data model changes
None anticipated
| Comment | File | Size | Author |
|---|---|---|---|
| #14 | webform_conditionals_missing_includes-2814119-14.patch | 1.37 KB | chris burge |
Comments
Comment #2
chris burge commentedAttached is a patch that implements solution #2.
Comment #4
chris burge commentedMy client is running Webform 7.x-4.12, which doesn't include #2757751: Sub-conditional and/or expressions. (This is why the patch failed to apply.) I'll retest against DEV and post back.
Comment #5
chris burge commentedThis issue continues to exist in 7.x-4.x-dev. Attached is patch rerolled against HEAD.
Comment #6
mfuggle commentedI have a webform in which I have an date field and an image upload field. I want to display the image upload field based on the date being later than a certain value. The date consists of select boxes for day, month and year. Am I correct in assuming the issue under discussion here will prevent me from succeeding with this action?
Comment #7
chris burge commented@mfuggle - I don't see anything in your use case that would be affected by this issue. This issue is specific to cases where a developer writes a new Webform component. The functions needed for 'date' conditionals are already available in all cases (because webform.conditionals.inc is always loaded).
Comment #8
liam morlandThanks for the patch. The "no additional files" case statements can be combined. module_load_include() has its own built-in check for whether a file has already been included, so an additional check is not needed here. Please check the coding standards: There is supposed to be a space after "if".
Comment #9
chris burge commentedThanks for reviewing the previous patch.
Attached patch 1) removes unnecessary include check (since module_load_include already handles this for us - I see that now that I look at the function), 2) combines the duplicate case statements, and 3) has been run through Coder.
Comment #10
liam morlandThanks. you can do it with:
module_load_include('inc', 'webform', 'components/' . $conditional_type);Perhaps use in_array() or at least don't even have a case for types that require no action.
Comment #11
chris burge commentedUnfortunately, the suggested code in #10 won't work. The conditional type name don't match up with include file names. For the 'numeric' conditional type, the conditional type is 'numeric' and the include file is 'number'. There is no 'string' include file.
The purpose for providing a case for conditional types that require no action is to self-document, to show that we have accounted for all of the conditional types that ship with Webform. This could be moved to a comment. E.g.:
Comment #12
liam morlandIndeed, "numeric" and "number" are different; sorry, I didn't notice.
I would rather go with the comments for documentation.
Comment #13
chris burge commentedOK. I'll submit an updated patch later today.
Comment #14
chris burge commentedAttached patch per #12.
Comment #16
liam morlandThanks!
Comment #18
vasyok commentedpatch not aplied to the current version
Comment #19
chris burge commented@VasyOK. Patch #14 was included in Webform 7.x-4.15. There's no patch to apply. This issue has been resolved.