OMG! Why on earth is the function _webform_components_form_rows() declared in theme_webform_components_form()?

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

skilip’s picture

FileSize
6.49 KB

Sorry I don't have GIT installed at my current workspace (government) so I'm stick to SVN. I hope the patch applies either way.

quicksketch’s picture

Category: bug » support
Priority: Major » Normal

It'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.

skilip’s picture

Hey 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.

quicksketch’s picture

We 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.

skilip’s picture

Status: Active » Closed (won't fix)

Ah, 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.

MustangGB’s picture

Status: Closed (won't fix) » Active

Fatal 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

quicksketch’s picture

I 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.

MustangGB’s picture

I 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:

if (!function_exists(_webform_components_form_rows)) {
  dsm(This text appears)
  function _webform_components_form_rows() {
  }
}
else {
  dsm(This text never appears)
}

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

MustangGB’s picture

Category: support » bug

Getting this again #1291996: WSOD /node/%/webform/components
Any more info I can provide to track this down?

quicksketch’s picture

This 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.

quicksketch’s picture

Category: bug » support
Status: Active » Closed (works as designed)

Closing after lack of activity.

leanne9’s picture

Version: 7.x-3.x-dev » 7.x-4.0-alpha9
leanne9’s picture

Status: Closed (works as designed) » Active

I 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!

quicksketch’s picture

Hmm, 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.

quicksketch’s picture

Status: Active » Fixed
FileSize
6.66 KB

This patch removes the nested function, which should prevent these problems from happening. Committed to the 4.x branch.

Status: Fixed » Closed (fixed)

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

MRPRAVIN’s picture

Issue summary: View changes

I 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.

MRPRAVIN’s picture

Status: Closed (fixed) » Needs review

The last submitted patch, 1: webform-1162560-2.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 15: webform_preprocess_remove_nesting-1162560.patch, failed testing.

MustangGB’s picture

Status: Needs work » Closed (fixed)

This is a bug in Webform Add Existing rather than Webform itself, you should create a bug report in their issue queue.

MRPRAVIN’s picture

ya. sry for this. i wrongly posted here...