Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
OMG! Why on earth is the function _webform_components_form_rows() declared in theme_webform_components_form()?
Comment | File | Size | Author |
---|---|---|---|
#15 | webform_preprocess_remove_nesting-1162560.patch | 6.66 KB | quicksketch |
#1 | webform-1162560-2.patch | 6.49 KB | skilip |
Comments
Comment #1
skilip CreditAttribution: skilip commentedSorry I don't have GIT installed at my current workspace (government) so I'm stick to SVN. I hope the patch applies either way.
Comment #2
quicksketchIt's declared inside a theme function because it must be a recursive function in order to handle the output and because it's producing themable output it must be in a theme function. If it were outside the theme function, you wouldn't be able to control its output. If you do override this theme function, you're right you need to change the name of the function so that its unique.
Comment #3
skilip CreditAttribution: skilip commentedHey Nathan,
Thanks for your reply. Wouldn't it make sense to at least check whether or not the function already exists or not? Or create a separate theme function for it so it still is themable? Just throwing some thoughts.
Comment #4
quicksketchWe could wrap it in a function_exists() check, but then it'd probably would not help for users overriding it. I'm sure the assumption would be that the theme's version would take precedence, but modules' PHP code is loaded before the theme, so you'd have to rename the function anyway in order to get it to work (otherwise the module code would be called). I think it would actually cause more confusion if we added a function_exists(). At least right now users are forced to rename the function.
I did try to make the function a separate theme function rather than nesting them, but because &$form, &$rows, and &$add_form need to be passed by reference a theme function won't work because the theme() wrapper only works with by-value parameters.
Ultimately I didn't worry too much about it because I didn't think many users would be overriding that theme function. Considering this is the first filed report in almost 2 years since that theme function was written, I think it's been a pretty safe assumption. The code may still be improved though, but I can't think of a better way to handle it.
Comment #5
skilip CreditAttribution: skilip commentedAh, if nobody bumped into this in the last two years, you totally convinced me ;). It's probably not worth the effort. Let's mark it as 'won't fix'. Thanks for your thorough explanation.
Comment #6
MustangGB CreditAttribution: MustangGB commentedFatal error: Cannot redeclare _webform_components_form_rows() (previously declared in \sites\all\modules\webform\includes\webform.components.inc:191) in \sites\all\modules\webform\includes\webform.components.inc on line 191
I've just encountered the same issue also
I'm not doing any theme overriding, just trying to use the module at stock
Related: http://drupal.org/node/588012
Comment #7
quicksketchI doubt the module is "stock", considering this error doesn't come up on new installs (or the 10,000 users on the D7 version). I'll need more information to reproduce.
Comment #8
MustangGB CreditAttribution: MustangGB commentedI spent a good couple of hours trying to track down the issue
As theme_webform_components_form() is only called once I'm struggling to discover how it's even possible to declare _webform_components_form_rows() twice
From my test:
When I say the module is at stock I mean it to be a fresh module checkout
There may however be something interfering from elsewhere in the site
So I shall endeavour to reproducing on a fresh D7 install
Comment #9
MustangGB CreditAttribution: MustangGB commentedGetting this again #1291996: WSOD /node/%/webform/components
Any more info I can provide to track this down?
Comment #10
quicksketchThis may happen if you override the theme_webform_components_form() in your template.php file. In which case, you need to rename the _webform_components_form_rows() function in your template file.
Comment #11
quicksketchClosing after lack of activity.
Comment #12
leanne9 CreditAttribution: leanne9 commentedComment #13
leanne9 CreditAttribution: leanne9 commentedI am actually getting this same error now...
Fatal error: Cannot redeclare _webform_components_form_rows() (previously declared in /home/drydock/public_html/sites/all/modules/webform/includes/webform.components.inc:191) in /home/drydock/public_html/sites/all/modules/webform/includes/webform.components.inc on line 191
It is weird because the error shows it redeclaring on the same line, same file... so could mean the function is somehow getting called twice, but I cannot find where. I also changed the text in template.php according to the upgrading instructions... https://drupal.org/node/1609324
Can anyone point me in the right direction? Thanks!
Comment #14
quicksketchHmm, I think this might be a problem after #1468944: Preprocess variables for components form. It looks like since we maybe should move _webform_components_form_rows() outside of the preprocess function. There's no reason for it to be nested any more. Previously it was nested because I wanted it to be overridable as part of the theme_ function, but now that it's a preprocess function you can't override it anyway.
EDIT: Fixed issue link.
Comment #15
quicksketchThis patch removes the nested function, which should prevent these problems from happening. Committed to the 4.x branch.
Comment #17
MRPRAVIN CreditAttribution: MRPRAVIN commentedI got this error when i using webform_add_existing
Webform module Version: 7.x-4.6
Webform_add_existing module version: 7.x-3.0
When i create new field or using existing field to add it throws this error
Fatal error: Cannot redeclare _webform_components_form_rows() (previously declared in /opt/lampp/htdocs/drupal/sites/all/modules/contrib/webform/includes/webform.components.inc:217) in /opt/lampp/htdocs/drupal/sites/all/modules/contrib/webform_add_existing/webform_add_existing.module on line 224
Help me if anyone know how to solve this issue.
Thanks.
Comment #18
MRPRAVIN CreditAttribution: MRPRAVIN commentedComment #21
MustangGB CreditAttribution: MustangGB commentedThis is a bug in Webform Add Existing rather than Webform itself, you should create a bug report in their issue queue.
Comment #22
MRPRAVIN CreditAttribution: MRPRAVIN commentedya. sry for this. i wrongly posted here...